Feature: Tolerant flight checks for docked ships

[tibetiroka]

Feature: There is no PR discussing this issue (that I could find).

Feature Details

This PR makes the takeoff process for docked (carried) ships more realistic by lifting the requirements for power generation and engines on those ships.

Rationalization:

Previously, all ships had to have power generation and engines installed in order to be able to take off from a planet. However, if a ship is inside a carrier, it doesn't actually need any engines to take off - the carrier will just take it up into space. In this PR, the thurster, steering and power generation requirements are lifted from such ships.

We also know that docked ships are connected to the mothership's power systems, since their batteries can be replenished. Since they are inside a dock, they have life support from the mothership, so even if they don't have power the pilot wouldn't suffocate.

Realistically, there is no difference between a cargo container in the cargo bay and between a stripped down Boxwing in a fighter bay. In fact, this is pretty much the essence of a Boxwing.

Implementation Details:

Flight checks are only used for the player's ships, and is done via the PlayerInfo::FlightCheck method. This method ties into the planetary takeoff process and the outfitter/shop tooltips, so changing the behaviour here should be enough to fully support this feature (in theory, anyway).

Ship::FlightCheck now has a parameter for docked ships; if it is false, the previous behaviour is preserved. If it is true, the power, thurster and steering errors are downgraded to warnings.

PlayerInfo::FlightCheck now stores which ships are docked, instead of only counting the number of docks and docked ships. This required some minor code flow changes:

check flight issues and count docks
attempt to dock ships

got changed to

count docks
attempt to dock ships
check flight issues

Notes:

• This feature relies on the predictability of ship autodocking. If that was changed in a nondeterministic way, this feature would become unviable. Such a change could affect gameplay in a major way, and is therefore not probable.
• Reordering ships can make a previously viable fleet unable to take off; in this case, the warnings automatically change to errors in the side panel.
• One could argue that the current balance for Boxwing (and other fighters/drones) is based on the requirements for power and engines, and ships without these could become overly powerful. This is not the case, since those fighters would be stationary, and would get killed almost instantly in a battle.
• It was already possible to use (dock and deploy) such fighters in the game, however they couldn't depart from a planet. One can create such fighters by taking all their power and engines after they have been disabled.
• These fighters can be deployed by themselves and do receive the standard launch velocity, however they cannot dock unless the mothership manually manouvers to them.
• Feel free to edit the new tooltips, I'm sure someone more professional can make them a bit more natural and understandable.

UI Screenshots

Energy warning for docked ships: Energy warning for non-docked ships (same as existing behaviour):

Testing Done

Playtesting has been done with multiple fighters and a carrier where some of the fighters had missing components.

Performance Impact

This feature shouldn't have a significant performance impact, considering that the original bay counting already ran in n^2 time, and the extra iteration through ships in the new version only adds n (where n is the number of ships in the player's fleet).

[tibetiroka]

All done. I playtested most things so it should be fine. As soon as the new tooltips are reviewed this should be ready for merging.

[tibetiroka]

Yeah, that and the use of dockedShips is a bit wonky, but I don't think there is a better way without refactoring the whole thing.

[Hurleveur]

@roadrunner56 could you take a look a those new tooltip descriptions when you have the time? they seem fine to me but that doesn't mean much

[tibetiroka]

Thanks! Apparently I wasn't paying much attention to the details there.

[petervdmeer]

I'm not sure if this change improves the game. Some players will appreciate being able to fly around with incomplete, broken, fighters, but this seems to be a bit of niche functionality.

Please first check with @Amazinite and @Zitchas if this change is desired before I generate a lot of additional work for this PR.

And if they agree that this is a good addition to the game, then I will ask for extensive test-coverage for this feature (integration tests with things like "fighters that have fuel and hyperdrives, but no energy", in various orders with other complete and incomplete fighters). Not wanting to discourage anyone, but niche functionality needs quite extensive (automated) testing, especially since it will not be used often (and typically forgotten by manual tests in future PRs).

I will then also request that we make sure that such "incomplete, broken," fighters will not be deployed by the deploy command (but that a message is given instead that says something like "[nr] ships were not deployed because they lacked the means to function independently."). Such an additional change will help with minimizing the support requests that we can expect from players that are unaware of the functionality in this PR and that accidentally take off with an incomplete fighter.

[tibetiroka]

the support requests that we can expect from players that are unaware of the functionality in this PR and that accidentally take off with an incomplete fighter.

I'm not sure why this feature would cause trouble by itself; the behaviour of AI fleets have not changed and players get warnings in the outfitter and in the shipyard about having such ships. The only way you could avoid those warnings is if you captured these ships, in which case the existing behaviour would be even worse (since it is inconsistent).

this seems to be a bit of niche functionality

At first glance, maybe. However, this PR enables a lot more than an extra cargo expansion on your Boxwings. Consider this: stripped down ships are cheap, customizable and expendable.

They are cheap, because depending on the type of ship you buy, the outfits might be 14-88% of the cost. (The most extreme examples are the Boxwing, Black Diamond and Violin Spider; other ships fall closer to the 30-60% mark.) This allows you to easily assemble huge (although severely handicapped) fleets.

They are customizable, since you can put just about anything in them, unless you want to use them as fighters - in which case, this feature is indeed irrelevant. You can use them to store ammo, which could be useful if the mothership uses missile launchers.

Also, don't forget that you can remove any of the previously required generators / engies, but yon don't need to remove all of them. You could make ships that have power and reverse thrusters, but no steering - unless they use weapons, they will be able to stop, and even return to a stationary mothership (assuming the AI can figure out reverse-only thrushers). And if you install long-range missile launchers on those ships they become deployable super-mines that are relatively cheap and can beat anything that doesn't shoot back. They can be especially useful against single targets.

Due to their low cost, these ships are also expendable. I haven't playtested this yet, but I can imagine that a fleeing ship could release these fighters to distract its pursuers, presenting an alternative to dumping cargo. Okay, this is a niche use case, but I could totally see this being used merchant fleets.

And these are only the things off the top of my head. Considering that @Hurleveur wanted similar changes for a while there will be even more potential uses and demand for this feature.

fighters that have fuel and hyperdrives, but no energy

Hyperdrives, Scram Drives and Jump Drives don't actually consume energy when used. And, again, any "new" edge cases have already been present in the game, even if they don't have bug reports / tests associated with them. If you want these tests, they have to be added even if this PR never gets merged.

I will then also request that we make sure that such "incomplete, broken," fighters will not be deployed by the deploy command

That could be discussed; I see your point here but deploying these ships could be useful. I just don't see a way where we could allow deploying these ships without including them in the default command. If that were possible, sure thing. Maybe we could change how the deploy command selects docked ships (using the existing flight checks), I'll have to look into that.

TL;DR: I don't think this is a niche use case, and any issues this PR allegedly introduces are already existing edge cases in the game.

[Hurleveur]

That could be discussed; I see your point here but deploying these ships could be useful. I just don't see a way where we could allow deploying these ships without including them in the default command. If that were possible, sure thing. Maybe we could change how the deploy command selects docked ships (using the existing flight checks), I'll have to look into that.

We could make it a preference, or a warning when you deploy ships. (you have unequipped fighters, do you want to launch them as well? ok/cancel dialog)

---

Also there is no need to be worried by integration tests, they don't require too much work to understand/do, and they assure everyone that this thing works. In this case, 4 simple tests with save files that contain the different kind of previously prevented from launching fighter ships, as well as one with these limitations on normal ships. You just make a new procedure to attempt launching, that will press "D" then "Enter", and then "L" and check the date. If the date changed then you indeed launched, otherwise you had a warning.

But maybe you should not get too much work going before Amazinite approves this behaviour.

[petervdmeer]

I will then also request that we make sure that such "incomplete, broken," fighters will not be deployed by the deploy command (but that a message is given instead that says something like "[nr] ships were not deployed because they lacked the means to function independently.").

@tibetiroka Please consider to create this other PR first, because this other PR will also be useful for many other situations (and I don't expect many objections to this other PR). (Never mind, I see in your response that you already planned to do so. Sorry for not reading all of the comments properly before responding.)

[Pilcrow182]

We could make it a preference, or a warning when you deploy ships. (you have unequipped fighters, do you want to launch them as well? ok/cancel dialog)

Might I suggest both of these, actually? A warning dialogue with a yes/no prompt, but also a preference that, when enabled, skips that dialogue and proceeds as if the player had selected 'yes'.

[tibetiroka]

Alright, here are the requested options. I added two of them, which is probably too much, so please tell me your opinion below.

The first option controls how the takeoff is handled from planets; it either prompts you about the incomplete ships (default) or skips it and takes off straight away.

The second option handles how the fighters are deployed; they are either deployed with the rest of the fighters or not. This only affects the automatic deploy-everything kind of command, manual deployments are always allowed. There is also a new message about ships that were not deployed due to player preferences.

All messages and texts are subject to change, and if somebody more talented comes along then I'd be happy to use whatever they suggest. If we do end up using these messages then the preferences menu will need to become wider, because the current option is so long.

Tests are coming as soon as Derpy or Zitchas approves the idea (or sooner if I get bored). I don't want to separate them into another PR because all of the actual logic is here and I don't want to deal with any merge conflicts if they can be avoided.

[Hurleveur]

There's never too many options. That said, we could have both of those in one, named Incomplete fighters behaviour, the default option would be "ask", then "launch", and finally, "deploy". If you want we can even add a final "deploy and launch" option, if you want to keep both separate, otherwise "deploy" could mean we "launch" as well. (though may as well add that fourth option).

However, currently I don't think this works, since nothing in Preferences has been modified. In order to make that kind of 3-4 choice preferences I'm talking about, you can look at https://github.com/endless-sky/endless-sky/pull/6346 and inspire yourself from the changes made there in Preferences and PreferencesPanel. ~~don't look at the lambdas within lambdas I don't want to scare you~~

Also would be neat to see what the new preference panel looks like with the extra option, once you're done.

[tibetiroka]

Preferences stores true/false options in a map, so the class doesn't need to be changed; PreferencesPanel has the entry and it handles the possible options. You only need to edit Preferences if the new preference isn't a simple on-off option (like the one you proposed).

A view of the panel: (The text is currently a placeholder)

The new options are saved automatically in preferences.txt:

volume 0
(...)
"Damaged fighters retreat" 1
"Deploy incomplete fighters" 0
"Draw background haze" 1
(...)
"System map sends move orders" 1
"Take off with incomplete fighters" 1
"Turrets focus fire" 1
(...)
maximized 1

[Hurleveur]

Nice, you even happen to add it in the only category that still has space! Still I think one option for all of this behavior is best, as you can see we kinda lack space in that panel, and the less options there are the clearer it is. (one option with more toggle is thus best if you ask me)

[tibetiroka]

Okay, so just to clarify: you want a 3-state toggle with these options:

• Ask before launch, don't deploy (ask)
• Don't ask before launch, don't deploy (launch)
• Don't ask before launch, deploy (deploy)

(Launch and deploy sound a bit similar, but that is the content reviewer's problem.)

[Hurleveur]

Yeah exactly though the names may change, I think depart would be clearer, instead of launch? I'm not too good with names so that can change, but you got what I mean indeed.

[tibetiroka]

Once again, I'm asking for your review.

[tibetiroka]

Okay, then I'm moving onto tests and have roadrunner re-review the text for the option when I'm done, since that is their area.

[Amazinite]

I'm in agreement with Peter on this one.