Rewrite frame CRC generation#1369
Conversation
|
Nightly build for this pull request:
This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build. |
|
What kind of tests should be done here if you want feedback? |
chaserli
left a comment
There was a problem hiding this comment.
If you're rewriting these completely, it would be better to hash these in a more modern way for better readability and extensibility
6584f4c to
f517354
Compare
f517354 to
e4c80d2
Compare
|
The wrapper may be good idea in theory but that implementation mostly just obfuscates what gets hashed and for what type of object (it also changes the functionality from how game does it slightly, although likely to little to no tangible effect). The benefits would be more obvious if there was a realistic chance for new properties that need hashing to be added for, say, TechnoClass and derivatives in the future but as it stands this is very unlikely. If you can think of a way to do it without extra overhead or the obfuscating nature while reducing repeated code be my guest, but the previous implementation was not it so I've elected to revert it. I do hold a local copy of it for now still just in case though. I'm not even going to repeat the point about the rules hash check in Phobos anymore, should be obvious by now that it does not belong here. I did do some cleanup to the first iteration too. |
It only wrapped a DWORD and serves for hash the object on creation and do the hash combine operation. I bet it makes less overhead than your current method. It also allows you to know the "sum" at compile time. But anyway these are small things |
e4c80d2 to
4ca9bb5
Compare
4ca9bb5 to
f1ebedc
Compare
f1ebedc to
596c76e
Compare
596c76e to
2b49718
Compare
2b49718 to
0710f77
Compare
2128644 to
799ea22
Compare
7b7655c to
9c16e3c
Compare
|
Rebased on develop and applied some of the feedback. This is ready for further testing and another review pass. |
9c16e3c to
d7f11dd
Compare
|
no desync or other issue occured in our testing so far |
d7f11dd to
8941456
Compare
9fb53b2 to
fdb27bd
Compare
7a87b77 to
c9bc0f2
Compare
b429215 to
280b1c8
Compare
c9bc0f2 to
ffbda85
Compare
ffbda85 to
91df63b
Compare
91df63b to
8dd586f
Compare
b4473e6 to
c89b048
Compare
c89b048 to
02cde34
Compare
02cde34 to
177e06c
Compare
177e06c to
818c9de
Compare
ZivDero
left a comment
There was a problem hiding this comment.
A couple nitpicks but looks good, see no issues.
- Use DEFINE_JUMP to replace calls to ComputeGameCRC instead of hooking in it - Separate the CRC calculations into inlined functions - Do not inline the hash eligibility check function due to its size, rename it for clarity - Misc. minor improvements
818c9de to
231fced
Compare
Reimplements game's frame CRC generation function, only change logic-wise is that it now excludes
AnimClassandParticleClassobjects that are purely visual.