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

[UnorderedSigh]

Is there an existing issue for this?

• [X] I have searched the existing issues

Describe the bug

When the AI decides to stop the ship, sometimes it can't. This happens due to an error in the new thrusting logic. Sometimes, the code will ignore dragAcceleration, which is the variable Ship::Move uses to implement Command::STOP when the ship is almost stopped. This prevents ships from boarding, landing, assisting, and a variety of other things, if they happen to be moving at just the right speed. That's why the integration tests are failing.

Steps to Reproduce

Run the integration tests a few times on Ubuntu 20 or 22 using GL. Watch Capture Uncapturable With Capturable Override fail.

Expected Behavior

Ships can stop. All integration tests should pass.

Screenshots

No response

Link to save file

No response

Operating System

Ubuntu 22

Game Source

Built from source

Game Version

e1004d144

Additional Information

This is the fix:

diff --git a/source/Ship.cpp b/source/Ship.cpp
index 61d4b668c..47fa1f485 100644
--- a/source/Ship.cpp
+++ b/source/Ship.cpp
@@ -2249,7 +2249,7 @@ void Ship::Move(vector<Visual> &visuals, list<shared_ptr<Flotsam>> &flotsam)
                                if((aNormal > 0.) != (vNormal > 0.) && fabs(aNormal) > fabs(vNormal))
                                        dragAcceleration = -vNormal * angle.Unit();
                        }
-                       if(velocity.Length() > MaxVelocity() || velocity.Length() < 0.1)
+                       if(commands.Has(Command::STOP) || velocity.Length() > MaxVelocity() || velocity.Length() < 0.1)
                                velocity += dragAcceleration;
                        else
                                velocity += acceleration;

I don't want to do a PR until the conflicts between master and experimental are resolved.

[UnorderedSigh]

I have retested experimental with this fix, and it doesn't fix the problem. I'm still investigating.

[UnorderedSigh]

I have a fix for this, but a later test fails now.

[Zitchas]

It's been a while and I'm just getting back up to speed. Is this bug still a thing?

[xobes]

1. I came to this thread to ask if the down-arrow to turn around and allow me to press forward to slow down is intentionally broken in this fork/branch (which is it, anyway)
   • also -- actually, when i press the down arrow at a low enough speed i start going backwards in my star barge -- no reverse thrusters, so that's not right.
2. I see up there something about integration tests, but on my linux box I just pulled this down and built the Debug build but could not do ctest --preset linux-integration, as it claimed No tests were found!!!

[TheGiraffe3]

1. I came to this thread to ask if the down-arrow to turn around and allow me to press forward to slow down is intentionally broken in this fork/branch (which is it, anyway)

• also -- actually, when i press the down arrow at a low enough speed i start going backwards in my star barge -- no reverse thrusters, so that's not right.

In Delta, all thrusters also have some reverse thrust. So down to turn around intentionally doesn't do anything, and reverse thrusters are just an extra bonus - you don't actually need them to go backwards.

[Zitchas]

Thanks for the feedback!

1. By default, the down arrow switches to reverse thrust when the ship has some reverse capability; and currently all ships have limited amounts of reverse and lateral thrust. That being said, having a control to tell your ship to flip around to exactly counter your current movement is a useful command, so we should probably figure out a way to force that behavior. Something tickles the back of my mind that it might already exist, but I'm not sure about that.
2. Unfortunately, my knowledge of the tests or how to invoke them is pretty limited. On Visual Studio on Windows the tests don't run on a normal or debug build; I have to trigger them by telling VS to do the tests. I would suspect that this is also the case on Linux.