Retreat from pre-Amphibious Assault sea battle incorrectly implemented

[panther2]

Engine version

Experienced with 1.9.0.0.12322
Implemented at any point during 1.9.0.0.x development.

Map name

noted with wwII_Global_1940

Can you describe how to trigger the error? (eg: what sequence of actions will recreate it?)

When the attacker retreats from the amphibious assault-preparing sea battle the land units incorrectly are kept in the enemy territory and are subject to a land battle.

Instead of this error, what should have happened?

In case the attacker retreats from the sea battle the land units can't be unloaded and stay on the transport.

Any additional information that may help

In the attached savegame Germany wants to amphibiously assault Scotland (2 Subs, 1 Transport, 2 Inf). There is a French fighter scrambling from England. Luckily the fighter misses, so the attacker is enabled to retreat.
The German infantries incorrectly are still placed in Scotland and take Scotland during a land battle.

By the rules the Land Battle can only take place when the sea zone has been cleared of enemy units.
So the German Infantry units have to be kept on the transport when the sea units retreat.

retreat.zip

In 1.8.0.9 the land units are correctly back on the transport when the sea units retreat from the sea battle.

[Cernelius]

Also I think it would be consistent the most if units that submerge cannot offload, either (of course, submarines are not supposed to be able to transport units, in the basic games).

[simon33-2]

Far out. That's a bad bug. It can't be in every version of 1.9 I don't think.

[simon33-2]

Build 11599 works correctly.

I recommend downgrading the stable release to that.
@ron-murhammer @DanVanAtta

Here's a test case from before the battle
4162.zip

[ssoloff]

Using @simon33-2's save, I bisected the first failure to 1.9.0.0.11746 (#3931). It last passed in 1.9.0.0.11739.

[ssoloff]

It appears the root cause is due to the removal of the TransportTracker#transporting(Collection<Unit>, Collection<Unit>) overload. What threw me off when I was comparing that overload to the transporting(Collection<Unit>) overload is that I confused the call to transporting(Unit, Collection<Unit>) as a recursive call to transporting(Collection<Unit>, Collection<Unit>). So, there was indeed a one-line difference between transporting(Collection<Unit>) and transporting(Collection<Unit>, Collection<Unit>).

The question for everyone now is the following: Is this issue such a significant regression that we need to push a hotfix for the current stable? Or can a fix wait until the incompatible release is complete?

[Cernelius]

I'm using 1.9.0.0.12421 since a while, for a bunch of games, and never had this problem. I never play the referenced game, tho. I tested it just now, with "World War II v3 1941", trying to invade the United Kingdom with Germans and retreating from 6 Sea Zone and didn't notice any issues. I wonder if it might be related to the mentioned scrambling.

[panther2]

Might be related to scrambling, indeed.

The question for everyone now is the following: Is this issue such a significant regression that we need to push a hotfix for the current stable? Or can a fix wait until the incompatible release is complete?

@ssoloff When do you expect the incompatible release to be "ready"? At least on A&A.org the affected games are most popular. Usually I encourage people there to update to the latest stable ... but this issue would surely be an argument not to do so, IMHO.

[ssoloff]

When do you expect the incompatible release to be "ready"?

@panther2 Based on the current tasks in #2827 and the rate we've been addressing them, my best estimate would be that it's still months away.

[panther2]

@ssoloff In that case I would courteously ask for fixing it 😄
(not being able to estimate how much work that might be).

[ssoloff]

(not being able to estimate how much work that might be)

@panther2 It's a pretty simple fix. We just need to work through some logistical issues in order to release a new stable 1.9. Hopefully, we can knock that out in a few days or a week. 😄

[ssoloff]

@panther2 This should be fixed in 1.9.0.0.12640. Please give it a drive when you get a chance.

@ron-murhammer Leaving it up to you if you want to proceed to promote this release to stable. It probably wouldn't hurt to ask the QA team to test it for a few days simply to verify there's nothing weird about the new release branch.

[simon33-2]

Works for me.

[panther2]

@ssoloff I can confirm it works correctly now. Thank you.

I would recommend to release a new stable containing this fix.

[ron-murhammer]

@ssoloff Yeah, we should probably release a new stable with this fix since it is a regression and impacts popular maps.

[simon33-2]

Can we also get #4255 fixed? Unless it isn't a global problem like it appears to be.