In rP21359 I introduced a new bug while fixing the walking sound animations that makes any serialization test fail in non obvious ways, because the sounds are different from the real game.
Test that it works now, and that I didn't do anything stupid in the previous revision.
- Start a multiplayer match without any patches and task some units
- Replay that file in non-visual replay mode (pyrogenesis -replay=...commands.txt -mod=public)
- If there are some hash mismatches, you reproduced the bug. Otherwise go back to step 1.
- Apply the patch, do the nonvisual replay of the same commands.txt again and check if it the hash mismatch is gone.
Why m_Unit is not initialized in nonvisual mode:
m_Unit is not initialized in non-visual mode because the CGame constructor in Game.cpp has disabledGraphics = true which leaves m_GameView at NULL, thus no GameView creates an ObjectManager and the SetObjectManager function is not called in CGame constructor.
Hence CUnitManager::CreateUnit doesn't call CUnit::Create and returns NULL when called from CCmpVisualActor::InitModel.
Perhaps we can add a // Not initialized in non-visual mode above the variable declaration.
Why nonvisual replaymode doesn't test well:
The visual replay mode tests whatever hash is found in commands.txt whereas the non-visual replaymode compute hashes only every 100th turn (turn % 100 == 0 in Replay.cpp), presumably to save performance.
But sometimes we prefer certainty over performance, so it should become a commandline argument if we want to skip hashing to improve performance for both replaymodes.
I suspect the slower one should be enabled by default as it already is for visual replays and networked games.
Created D1538 to test this diff more properly.
Why it doesn't always mismatch hashes:
Can reproduce (more easily using D1538) by moving some units and it self-corrects once all units are destroyed that ever moved.
I guess that's because the idle animation doesn't have a SoundGroup.
Why it's correct:
The patch only changes the order of the two statements, resulting in the soundgroup being loaded in non-visual mode too, thus always being initialized correctly, thus computing the correct hash.
temple said on irc yesterday that it is correct and must be correct. Maybe he wants to accept too.
I don't know if it's complete, almost never worked with the components and I didn't read all of the concerned diff.
Should be committed now as this bug makes it harder to debug OOS issues and since we don'T want to have to reapply this patch everytime when debugging a23 reports in the future and reduce false positives.