Fix disembarking without losing MP through adjacent transport #2025
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Say we have two transports A and B next to the coast C, with a land unit in A. Consider the following orders for the land unit: 1. Unload from A to B->tile 2. Move from B->tile to C This sequence should not succeed because the unit should still be loaded after the first step. However, in practice the unit was not loaded after step 1, resulting in an invalid state that confused ORDER_MOVED. Do the same when bumping units after a transport is killed. The fix to that is obviously to load the unit after step 1. There is a catch though: this is (and has always been) done without considering action enablers. It might be possible to exploit this to bypass loading rules. Spotted by mu in LT78.
It had two branches with identical code.
Since units are (now) automatically loaded back after unloading to a non-native tile, make the goto code aware of this to avoid generating invalid paths. Also unify the handling of ACTION_TRANSPORT_DISEMBARK1 and 2.
|
Does this bug only impact combat units, or would it also work on engineers or other non-combat units? I can't recreate on an unpatched install. |
|
All units are affected as long as they have at least 3 MP (1 for ship hopping, 1 to disembark, 1 to attack). I used Mechs in my tests. |
|
Ok I recreated with a Mech too. |
jwrober
approved these changes
Aug 28, 2023
It might be possible to move from a ship into a transport plane or something like this that would normally be forbidden. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Say we have two transports A and B next to the coast C, with a land
unit in A. Consider the following orders for the land unit:
This sequence should not succeed because the unit should still be loaded
after the first step. However, in practice the unit was not loaded after
step 1, resulting in an invalid state that confused ORDER_MOVED. Do the
same when bumping units after a transport is killed.
The fix to that is obviously to load the unit after step 1. There is a
catch though: this is (and has always been) done without considering
action enablers. It might be possible to exploit this to bypass loading
rules.
Path finding support is also included in this PR.
Backport candidate.