Skip to content

[GEN][ZH] Fix replay header mismatch between builds using 32 bit or 64 bit wide time_t #765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 28, 2025

Conversation

helmutbuhler
Copy link

Another day goes by, and another bug is discovered. But this time, it's not a bug in the retail version, but in our build with VS22.
The replay header contains (among other) these elements:

  • starttime
  • endtime
  • number of frames
  • mismatch flag
  • quitEarly flag
  • disconnect flag (for each player)
  • replayName (string)

start and endtime are of type time_t. On VC6 this was 32-bit, but on newer compilers it's 64-bit. In order to remain compatible, we need to load and save them as 32-bit. This PR fixes that.

Now you may ask: Wait, why did we not notice until now? Why are replays working?
Well, the replayName syncs everything up. It's serialized with a special character to indicate its end, and since all other header elements very likely don't contain that character, the serialization is automatically fixed there. But all the replay header elements listed above are indeed parsed and written out corrupted. We also didn't notice that, because, as it turns out, all those elements are invisible to the user.

We should still fix it though, so we don't spread the internet with corrupted replays. I'd also like to use the mismatch flag to harvest some non-mismatched replays for automatic testing in the near future...

@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Fix Is fixing something, but is not user facing labels Apr 26, 2025
Copy link

@xezon xezon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Needs to be replicated in Generals.

@xezon
Copy link

xezon commented Apr 28, 2025

This 64 bit time_t breaks GenTool replay uploads in VS 2022 builds, because it cannot read the replay header.

@xezon xezon changed the title [ZH] Fix replay header memory corruption on VS22 [ZH] Fix replay header mismatch between builds using 32 bit or 64 bit wide time_t Apr 28, 2025
@xezon xezon added the Stability Concerns stability of the runtime label Apr 28, 2025
@xezon
Copy link

xezon commented Apr 28, 2025

I locally replicated this change to Generals but I am unable to push this to the remote branch

"git" push --recurse-submodules=check --progress "helmutbuhler" refs/heads/fix_replay_header_sh:refs/heads/fix_replay_header_sh
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
ERROR: Permission to helmutbuhler/CnC_Generals_Zero_Hour.git denied to xezon.

@xezon xezon force-pushed the fix_replay_header_sh branch from d1ab2ba to c340968 Compare April 28, 2025 21:53
@xezon xezon added this to the Stability fixes milestone Apr 28, 2025
Copy link

@xezon xezon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replicated in Generals. Looks good.

@xezon xezon merged commit 59e5b4e into TheSuperHackers:main Apr 28, 2025
18 checks passed
@xezon xezon deleted the fix_replay_header_sh branch April 28, 2025 22:37
xezon pushed a commit that referenced this pull request Apr 28, 2025
@xezon xezon changed the title [ZH] Fix replay header mismatch between builds using 32 bit or 64 bit wide time_t [GEN][ZH] Fix replay header mismatch between builds using 32 bit or 64 bit wide time_t Apr 28, 2025
@xezon
Copy link

xezon commented Apr 29, 2025

tomsons26

This was wrong
It should had been made to use time32_t
startTime global remains 64 bit
Doing replay_time_t tmp = (replay_time_t)startTime;
You just end up writing invalid time since the 32 bit and 64 bit time format differs

@xezon xezon added Gen Relates to Generals ZH Relates to Zero Hour labels May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Is fixing something, but is not user facing Gen Relates to Generals Minor Severity: Minor < Major < Critical < Blocker Stability Concerns stability of the runtime ZH Relates to Zero Hour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants