Game crashes when disabling rotation while exiting a station

[mwerle]

Observed behaviour

When launching from a station using flight-assist mode, killing the rotation causes the game to crash.

Expected behaviour

Game does not crash.

Steps to reproduce

• Dock at an orbital station (eg, Mars High)
• Request launch clearance and launch
• Retract landing gear, increase forward speed to exit station
• Press the "Kill Rotation" key
• GAME CRASH

Patch

Game crashes on an assert in PlayerShipController::FlightAssist(). The assert is partially invalid because immediately prior to calling FlightAssist the game calls PollControls which can set the flight mode to CONTROL_FIXHEADING_KILLROT

The following patch fixes the issue:

@@ -296,11 +296,16 @@ void PlayerShipController::PostLoadFixup(Space *space)
 	SetFollowTarget(space->GetBodyByIndex(m_followTargetIndex));
 }
 
 void PlayerShipController::FlightAssist(const float timeStep, TotalDesiredAction &outParams)
 {
-
+	// MKW : This is called immediately after PollControls, which can set the
+	// flight mode to CONTROL_FIXHEADING_KILLROT. If it does, this function
+	// should presumably do nothing.
+	if (m_flightControlState == CONTROL_FIXHEADING_KILLROT) {

+		return;
+	}
 	// in other modes it doesn't make sense (yet?)
 	assert(m_flightControlState == CONTROL_FIXSPEED);
 
 	assert(!outParams.desireLinVel && "assumed that no one has touched the linear speed yet");
 	outParams.desireLinVel = true;

My pioneer version (and OS):
Pioneer "trunk"; Debian GNU/Linux 12

My output.txt (required) and game save (optional, but recommended)

output.txt

FWIW, the output.txt does not include the output from assertion failures - it probably should:

Info: Color Fractal name: GGSaturn
Info: Color Fractal name: GGJupiter
pioneer: /home/micha/Source/Pioneer/src/ship/PlayerShipController.cpp:310: void PlayerShipController::FlightAssist(const float, PlayerShipController::TotalDesiredAction &): Assertion `m_flightControlState == CONTROL_FIXSPEED' failed.
Aborted (core dumped)

Scan_Test.zip

[Gliese852]

It seems to me that pushing (most likely accidentally) on "Kill rotation" when cruise mode is on should be ignored. That is, a patch like this:

--- a/src/ship/PlayerShipController.cpp
+++ b/src/ship/PlayerShipController.cpp
@@ -708,7 +708,7 @@ void PlayerShipController::PollControls(const float timeStep, int *mouseMotion,
 		wantAngVel += Util::NeededAngVelocityToFaceDirection(*this, timeStep, GetMouseDir(), outParams.angPower);
 	}
 
-	if (InputBindings.killRot->IsActive()) SetFlightControlState(CONTROL_FIXHEADING_KILLROT);
+	if (InputBindings.killRot->IsActive() && GetFlightControlState() != CONTROL_FIXSPEED) SetFlightControlState(CONTROL_FIXHEADING_KILLROT);
 
 	outParams.desiredAngular = wantAngVel;

Because if we go to CONTROL_FIXHEADING_KILLROT, we don’t go back to CONTROL_FIXSPEED, which is most likely unexpected.

[mwerle]

Hmm, looks like my last comment got lost in the aether..

In my case it was definitely not an accident that I pushed the kill-rotation button. I very specifically wanted my ship to stop following the station rotation so that I could manoeuvre properly and set course to my next destination without the station rotation interfering.

The patch I proposed appeared to result in the expected flight behaviour.. but might need more testing in different circumstances.

[Gliese852]

The fact is that the ship itself always kills rotation by default. The "kill rotation" button is used when the "rotation damping" is disabled and the ship is freely dangling in space. In your case, the ship rotates because it follows the station, repeating its movement, this is not free rotation. What you really want is to disable cruise mode, which is achieved by clicking on the "Flight Assist" button (I guess that's what it should be called) and selecting the appropriate mode.

[Zireael07]

1. This is not clear to the player
2. Game shouldn't crash because player clicked wrong button

[Gliese852]

@Zireael07
Yes, I agree with both points. Do you use the "kill rotation" button? What do you think it should do in "follow orientation" mode?

[Zireael07]

Either 1) be grayed out and do nothing or 2) do what turning off flight assist does (disable cruise)

[mwerle]

I don't want to disable cruise, I just want to disable the "follow rotation" and remain in cruise mode.

There should never be a "wrong" button for the player to press..

[Gliese852]

@Zireael07

Either 1) be grayed out and do nothing or 2) do what turning off flight assist does (disable cruise)

It's not clear what "grayed out" should be, we don't have such a button in the gui, as far as I know, only binding. And I am inclined to think that it should not do anything, since other elements are responsible for changing flight modes.

@mwerle

I don't want to disable cruise, I just want to disable the "follow rotation" and remain in cruise mode.

Okay, you can do this too via the "Flight Assist" button.

There should never be a "wrong" button for the player to press..

Yes, this is also a good declaration, which is hard to disagree with.

[Zireael07]

Ah, I thought those were on-screen HUD buttons, that's why I said "greyed out".

In which case it shouldn't do anything

[bszlrd]

Not sure how intuitive it would be, but Kill Rot could be used to quicky turn off Follow Orientation, without affecting cruise control.

The radial menu can be a bit fiddly anyway, it would be useful to have a keybind for toggling it anyway, and the Kill Rot button might be a good candindate, since it is only useful if Rot Damping is off. So it could double as that.

At least I think that having the Rot Damping off wouldn't make sense anyway when you have Follow Orientation on. Since your engines are already actively working on keeping you synced.

[mwerle]

So I just tested this and there's no (apparent) way to disable "Follow Rotation" mode when exiting a space-station other than the button (kill-rot) which causes a crash in the current implementation. (Actually there is - see below; leaving this for posterity since i already typed it up)

Steps to repro:

1. Request launch from station services
2. Ship launches, in "up-cruise" mode with "follow-orientation" on.
3. Rise vertically from station axis until aligned with centre axis of station
4. Raise landing gear. -> ship comes to a stop and flight-mode changes to "forward-cruise" mode with "follow-orientation" still on.
5. Increase speed and exit station.

The mouse context-menu on the cruise-control indicator just lets me select between different orientations (prograde, retrograde, etc).

The mouse context-menu on the station just lets me select auto-pilot modes.

Clicking on the "rotation damping" button does nothing (even though the indicator changes)

The cruise-control button (F5) just toggles between free-flight and forward-cruise mode. It does not disable "follow-orientation".

Leaving the stations' frame of reference does not disable "follow-orientation".

Leaving the stations controlled space does not disable "follow-orientation".

.....

Actually there is - I just found that LEFT-clicking on the cruise-mode indicator brings up a different context menu (I was only right-clicking up to now..), and this does provide a way to toggle "follow-orientation" mode. But while this works, the icon is actually greyed-out so it appears as if it's disabled.

BUG: if "follow-orientation" is ON, and you go too far from the station, then the icon really does become disabled and from this point onwards there is no way to disable it.

---

Proposal:

1. There should be button mappings for all UI elements; having to switch between keyboard and mouse to fly is not ergonomic.
2. The LEFT-CLICK action menu on the flight-mode icon has greyed-out "X" buttons to cancel modes. This is not intuitive (they looked like disabled buttons to me). Possibly we could overlay the "X" over the original icon (no idea if the UI allows compositing icons or whether we'd need a new icon set)?
3. Leaving the station should automatically disable "follow-orientation" mode once a certain distance is reached. At the very latest when leaving the station's frame-of-reference.
4. "follow-orientation" mode only makes sense within a cone along the station's axis of rotation; leaving that cone should disable "follow-orientation" mode as well. It results in incredibly bizarre flight behaviour if "follow-orientation"mode is on and one is far from the axis of rotation.

[Gliese852]

@mwerle

1. There should be button mappings for all UI elements; having to switch between keyboard and mouse to fly is not ergonomic.

Yes, this is also a correct statement, but this may be a separate, rather large project. By the way, you can configure the binding to open the round menu “Flight Assist” and “Attitude Hold”, and then select the desired one using the arrows.

2. The LEFT-CLICK action menu on the flight-mode icon has greyed-out "X" buttons to cancel modes. This is not intuitive (they looked like disabled buttons to me). Possibly we could overlay the "X" over the original icon (no idea if the UI allows compositing icons or whether we'd need a new icon set)?

Yes, it’s probably worth at least making the cross brighter.

BUG: if "follow-orientation" is ON, and you go too far from the station, then the icon really does become disabled and from this point onwards there is no way to disable it.

3. Leaving the station should automatically disable "follow-orientation" mode once a certain distance is reached. At the very latest when leaving the station's frame-of-reference.

I think these are related statements and are a topic for a separate issue. There is a certain distance specified at which follow orientation works, but this requires additional refinement.

4. "follow-orientation" mode only makes sense within a cone along the station's axis of rotation; leaving that cone should disable "follow-orientation" mode as well. It results in incredibly bizarre flight behaviour if "follow-orientation"mode is on and one is far from the axis of rotation.

Please note that this mode works not only in relation to a rotating station, but also in relation to any ship, not only rotating but also maneuvering in any way. This is not used in the game yet, but perhaps in the future it will be useful, for example, for docking with a rotating ship. In addition, the possibility of landing directly on the station's residential ring is being considered.

[Gliese852]

@bszlrd
But what to call such a binding? "Kill rotation in manual mode with rotation damping off or toggle follow orientation in follow mode"?

[mwerle]

So after binding the pop-up menus to hot-keys, it's possible to invoke the functionality to stop follow-mode.

It might still be useful to eventually have specific hot-keys for this, but currently not so important.

What might be more important is providing default bindings for bringing up those pop-up menus... (I bound them to Z and X)

I'll give the patch proposed here : #5933 (comment) a playtest; sounds like it will have less side-effects than the one I proposed.

[impaktor]

There should be button mappings for all UI elements; having to switch between keyboard and mouse to fly is not ergonomic

yes please

[bszlrd]

@Gliese852 Maybe it is two separate binding, but mapped to the same key?

[Gliese852]

@bszlrd Yes, we can probably create 2 actions, but strictly separate their areas of application so that we can bind the same button, and this is an interesting case!

[mwerle]

I'd be hesitant to bind the same button to two separate actions; at least by default. It'll most likely cause no end of confusion amongst users.

[mwerle]

FWIW, the patch proposed by @Gliese852 seems to work fine. I'd still propose adding some distances at which it auto-disables though..

[bszlrd]

I think in this case sharing a binding shouldn't be problematic, since you are either in one or the other situation, they don't overlap. At least they shouldn't.

Auto-off of Orientation follow, might need some thought. Especially if we want to allow ring landing eventually (I do want to make some new orbitals, even larger than the current ones). But yeah, a frame change would be a good point to switch it off, since it would avoid the flight computer seemingly going haywire when it would start to sync up with a planet for example.

On a half-unrelated note, I think I'd raise the orbital station frame radius at least to 50km. Maybe it could even be set by the station itself (for eventual larger stations. I'm eyeing an O'neill's clinder, and the trimsheet workflow. Won't happen fast though)

[sturnclaw]

From a diegetic perspective, I could see ships having an easily accessible "kill rotational velocity" button on the flight controls which would disable follow-orientation mode and also immediately attempt to stop all angular velocity. This would be a point in favor of a single binding.

From a user-accessibility perspective, it might be better to have a "disable follow mode" binding separate from "disable autopilot" and "kill rotation". The first binding would go back to normal cruise mode from any of the follow modes, the second would cancel any kind of fixspeed or fixheading mode, and the last would attempt to cancel angular velocity so long as the button is held down.