-
Notifications
You must be signed in to change notification settings - Fork 151
feat: Simulation math CRC utility for mismatch prevention #2100
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
base: main
Are you sure you want to change the base?
Conversation
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Include/Common/SimulationMathCrc.h | Simple header file declaring static CRC calculation utility, no issues found |
| Core/GameEngine/Source/Common/SimulationMathCrc.cpp | Implements CRC computation using matrix math operations to detect platform/compiler differences |
| Core/GameEngine/CMakeLists.txt | Added new source and header files to build configuration |
Sequence Diagram
sequenceDiagram
participant Client as Client Code
participant SMC as SimulationMathCrc
participant XferCRC as XferCRC
participant Matrix as Matrix3D
participant Math as Math Functions
Client->>SMC: calculate()
SMC->>XferCRC: open("SimulationMathCrc")
SMC->>SMC: _fpreset()
SMC->>SMC: _controlfp(0x000A001F, flags)
Note over SMC: Set FP rounding mode for vc6
SMC->>SMC: appendMatrixCrc(xfer)
activate SMC
SMC->>Matrix: Set(4.1f, 1.2f, ...)
SMC->>Math: Sin/Cos/tan/asin/etc.
Math-->>SMC: float results
SMC->>Matrix: Set factors_matrix
SMC->>Matrix: Multiply(matrix, factors_matrix)
SMC->>Matrix: Get_Inverse(matrix)
SMC->>XferCRC: xferMatrix3D(&matrix)
XferCRC->>XferCRC: Compute CRC of matrix data
deactivate SMC
SMC->>SMC: _fpreset()
SMC->>XferCRC: close()
SMC->>XferCRC: getCRC()
XferCRC-->>SMC: UnsignedInt CRC value
SMC-->>Client: UnsignedInt CRC value
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
|
While I think the concept is great, I don't think we should pollute the user-facing command-line options even more. |
|
I can agree with that, the command line was useful for testing but maybe not so user friendly, and sharing a CRC with users is not necessarily a goal as the meaning can be misunderstood (a matching CRC does not guarantee no mismatch). We could hold off merging until there is some kind of plan to use it |
|
I like this! I don't mind the commandline option, there are so many already anyway. |
|
I like it :) Agreed maybe we remove the command line option, and use this in the game lobby to warn people of potential compat issues. Also, looking forward, when we eventually go 64 bit + cross platform, this will presumably be removable, just something to plan for. |
|
I'm not sure whether I see the value of the command line option, but perhaps I could use the CRC value for #1404 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 files reviewed, 1 comment
| xfer.open("SimulationMathCrc"); | ||
|
|
||
| _fpreset(); | ||
| // TheSuperHackers @info stm Use the same rounding mode as used in the game. This affects vc6 only. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stm
I think we've used the full Github user name so far.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stm is difficult to decipher.
I agree. The idea for the CRC precheck is great. It would combine well for joining rooms and is another layer of safety to detect mismatch early. |
This PR adds a diagnostic utility to compute a CRC of simulation-relevant math operations which may be useful for predicting and preventing mismatches in multiplayer games.
It's basically a cocktail of math functions that is tweaked to be able to differentiate compilers, known good/bad Linux config, processor architecture. In the case of a non-matching CRC then there would be a high chance of seeing a mismatch at some point.
This provides a similar function to calculating CRC's of a set of replays, with the benefit that it runs very quickly (sub 0.1 ms) and is more stable to changes in the codebase.
Testing results
¹ Good/bad is determined by replay CRC's see #1940
² Known good here based on extensive testing using replays from GeneralsOnline (not the same binary though)
³ I don't have a known bad Linux config for win32
⁴ Apple M1 Mac Mini, Parallels Desktop 26, Windows 11 ARM
⁵ Using Whisky or Crossover
Implementation
A command line argument is added that prints this simulation math CRC:
-printSimMathCrc.It's out of scope but a possible use case for this is to check CRC's between a lobby host and someone who joins so see if their systems are compatible, and perhaps warn or block if the CRC's don't match.
Help with testing
If you have an ARM machine and would like to help test and fill in the TBD items it would be much appreciated.
Instructions
Download and install this version with the CRC print flag (must be logged into GitHub):
vc6: https://github.com/TheSuperHackers/GeneralsGameCode/suites/54311017349/artifacts/5120052850
win32: https://github.com/TheSuperHackers/GeneralsGameCode/suites/54311017349/artifacts/5120052850
Run this command using PowerShell, first
dirinto the directory with the game files:If everything went well this should print the simulation CRC value.