Fix visual-replays not processing turn 0 commands.
rP24280
Actions

Description

Fix visual-replays not processing turn 0 commands.

Use FinishedOwnCommands, which is intented to plan commands for the N+2 turn, but can be used to run commands immediately by the replay manager, and unlike FinishedUpdate is called before the first update.

Reported by: Freagarach

Fixes #5546

Differential Revision: https://code.wildfiregames.com/D3067

Details

Committed

wraitii

Nov 28 2020, 10:14 AM

Differential Revision

D3067: Fix visual-replays not processing turn 0 commands.

Parents

rP24279: Fix VisualActor tech changes for mirages

Branches

Unknown

Tags

Unknown

Reverted By

rP24764: Revert rP24280, causing false OOS in visual replays.

Build Status

Buildable 13997
Build 29096: Trigger Windows Autobuild
Build 29095: Post-Commit Build	Jenkins
Build 29094: Post-Commit Build (macOS)	Jenkins

Event Timeline

wraitii committed rP24280: Fix visual-replays not processing turn 0 commands..Nov 28 2020, 10:14 AM

wraitii added an edge: D3067: Fix visual-replays not processing turn 0 commands..

Harbormaster completed building B13997: rP24280: Fix visual-replays not processing turn 0 commands..Nov 28 2020, 10:38 AM

vladislavbelov added a subscriber: vladislavbelov.Dec 1 2020, 12:11 PM

vladislavbelov added inline comments.

/ps/trunk/source/simulation2/system/ReplayTurnManager.cpp
63	Is it guaranteed that `turn` is greater or equal to `COMMAND_DELAY`?

wraitii added inline comments.Dec 1 2020, 12:14 PM

/ps/trunk/source/simulation2/system/ReplayTurnManager.cpp
63	Yes, it's actually called with "m_CurrentTurn + COMMAND_DELAY"

vladislavbelov added inline comments.Dec 1 2020, 12:22 PM

/ps/trunk/source/simulation2/system/ReplayTurnManager.cpp
63	I see no guarantees (comments or additional checks) that it can't be called with just a `m_CurrentTurn`.

wraitii added inline comments.Dec 1 2020, 12:25 PM

/ps/trunk/source/simulation2/system/ReplayTurnManager.cpp
63	Well, there are none, but doing that would completely break the MP implementation, so I don't think it's happening soon. That said, adding an ENSURE probably wouldn't hurt here, if we had contracts, it'd certainly be part of it.