Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix movement bugs that break integration tests #32

Draft
wants to merge 3 commits into
base: experimental
Choose a base branch
from

Conversation

UnorderedSigh
Copy link

@UnorderedSigh UnorderedSigh commented Jul 6, 2023

Bugfix: Fixes #29 and some related problems mentioned on Discord

Fix Details

  1. Different drag logic to fix allow stopping to work reliably, while also allowing lateral thrusters to work as intended.
  2. AI.cpp should use Command::Thrust(), not FORWARD and BACK.

Testing Done

Ran the integration tests. Flew around a bit.

I ran the failing integration test ten times on an Ubuntu 22 GL build, and it passed.

@UnorderedSigh
Copy link
Author

No, no, apparently another test is failing now.

@UnorderedSigh
Copy link
Author

Test #67: Test-Framework - Navigate is failing with a watchdog timeout.

@TheGiraffe3
Copy link

Try re-running checks. I had that problem and when I re-ran the checks there was no more trouble.

@Zitchas
Copy link
Member

Zitchas commented Sep 30, 2024

I apparently do not have the brain power to follow this today. Will try again later. But I'd like to get this and your other PR tested and merged in the very near future.

@UnorderedSigh
Copy link
Author

Try re-running checks. I had that problem and when I re-ran the checks there was no more trouble.

It is a sporadic failure; about one in three times, it'll fail.

Presently, github says it has failed, and doesn't give any logs. I think we can safely say the fix doesn't work. That's probably why I changed it to a draft.

@Zitchas Zitchas added bug Something isn't working code Things that focus on the mechanics and internals of how they have engine works. labels Oct 3, 2024
@Zitchas
Copy link
Member

Zitchas commented Oct 6, 2024

I'm hoping that <24h I will have the brain power to analyze this, resolve the conflicts, test, and see how things go. No guarantees, though.

@Zitchas
Copy link
Member

Zitchas commented Oct 31, 2024

I haven't seen an integration test fail in the last month or two; unless those wouldn't show up in PR checks. Would they? If a failing integration test would show up as a failed check, then I think we can close this as it hasn't happened in a while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working code Things that focus on the mechanics and internals of how they have engine works.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect logic in Ship::Move breaks Command::STOP
3 participants