bugfix(logic): Restore retail compatibility after change to frozen time check#2803
bugfix(logic): Restore retail compatibility after change to frozen time check#2803Caball009 wants to merge 2 commits into
Conversation
|
| Filename | Overview |
|---|---|
| Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp | Adds a post-script-engine frozen-time refresh and early return inside GameLogic::update(), correctly gating all downstream updates on the state set by the just-run scripts. |
| GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp | Identical fix as the Generals counterpart; symmetric and correct for the Zero Hour expansion. |
| Generals/Code/GameEngine/Source/Common/GameEngine.cpp | Simplifies the update dispatch by removing the separate script-only branch; IgnoreFrozenTime flag ensures timing still fires when frozen, while a post-UPDATE() freeze check gates the client step. |
| GeneralsMD/Code/GameEngine/Source/Common/GameEngine.cpp | Mirror of the Generals fix; no divergence between the two codebases. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant GE as GameEngine::update()
participant GL as GameLogic::UPDATE()
participant SE as ScriptEngine::UPDATE()
participant FP as FramePacer
participant GC as GameClient
GE->>FP: "canUpdateGameLogic(IgnoreFrozenTime)<br/>sets initial timeFrozen state"
alt timing gate passes
GE->>GL: "TheGameLogic->UPDATE()"
GL->>SE: "TheScriptEngine->UPDATE()<br/>(may toggle frozen time)"
GL->>FP: "setTimeFrozen(TheGameEngine->isTimeFrozen())<br/>re-evaluates AFTER script ran"
alt time is now frozen
GL-->>GE: return early (skip terrain, objects, etc.)
else not frozen
GL->>GL: continue terrain/object/CRC updates
GL-->>GE: return normally
end
GE->>FP: isTimeFrozen()?
alt not frozen
GE->>GC: "TheGameClient->step()"
end
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant GE as GameEngine::update()
participant GL as GameLogic::UPDATE()
participant SE as ScriptEngine::UPDATE()
participant FP as FramePacer
participant GC as GameClient
GE->>FP: "canUpdateGameLogic(IgnoreFrozenTime)<br/>sets initial timeFrozen state"
alt timing gate passes
GE->>GL: "TheGameLogic->UPDATE()"
GL->>SE: "TheScriptEngine->UPDATE()<br/>(may toggle frozen time)"
GL->>FP: "setTimeFrozen(TheGameEngine->isTimeFrozen())<br/>re-evaluates AFTER script ran"
alt time is now frozen
GL-->>GE: return early (skip terrain, objects, etc.)
else not frozen
GL->>GL: continue terrain/object/CRC updates
GL-->>GE: return normally
end
GE->>FP: isTimeFrozen()?
alt not frozen
GE->>GC: "TheGameClient->step()"
end
end
Reviews (3): Last reviewed commit: "Replicated in Generals." | Re-trigger Greptile
xezon
left a comment
There was a problem hiding this comment.
This is unfortunate. Can we fix this somehow without moving the early return back into the Game Logic Update function? It makes the logic more complicated to understand again. It would be good to implement it in simple terms.
I don't see how to avoid the early return in a way that doesn't have major downsides. |
|
What downsides do you see? |
Either code duplication, or a potential risk of game logic changes. If you want to keep the exact same logic, you'd need to keep these in place:
Only solution I see at the moment to keep the changes as is, or perhaps put the code up to the script engine update in a separate function, although I'm not entirely sure if that's going to work. |
|
Unless you have a better suggestion, can we just merge this? I can create an issue / task to refactor this so that we can perhaps find a better way to implement this in the future. |
|
As last resort, maybe only do the hack in RETAIL_COMPATIBLE_CRC builds? |
The non-retail compatible code would always be one frame behind with freezing and unfreezing time; e.g. if a script is supposed to freeze the game at frame 1000 and unfreeze at frame 1100, it actually does that at frames 1001 and 1101. It probably is rarely used so we didn't notice it before, but I don't like the idea of keeping that unexpected behavior. |
|
I took the liberty and polished the code a bit, but fundamentally I have no better idea than what you already had. Can you test it again? |
TheGameLogic->UPDATE();
if (!TheFramePacer->isTimeFrozen())
{
TheGameClient->step();
}
|
5888813 to
cc04254
Compare
|
I found no mismatches with cc04254. |
|
I do not expect that the camera can shake during frozen time. The 3D World is supposed to be frozen. Camera shakes typically happens because of battles. Do you see a use case for camera shake during frozen time? |
No, I don't see any at the moment. |
xezon
left a comment
There was a problem hiding this comment.
I tested game pause and frozen time and it worked.
#1528 introduced a small change to the game logic as is shown below.
Implementation before approximately:
Implementation after approximately:
Replay vc6_replay_scripted_frozen_time.zip on map "[Mod] Football v3" mismatches because of that change. This PR fixes the mismatch.