[CMAKE][GEN][ZH] Make all builds compatible with retail #985
Conversation
- This fix makes all builds—including modern C++20 builds and debug builds—compatible with the retail game and with each other. - Fixes a floating point determinism bug in Thing::setOrientation that caused CRC mismatches when built with different compilers or optimization flags. - The cross product-based transformation code has been replaced with an equivalent, algebraically simplified implementation. - This new implementation is not only more efficient, but also eliminates intermediate floating point steps that were prone to subtle inaccuracies. - Also added the IGNORE_COMPATIBILITY macro (settable via CMake) for enabling future fixes that would break compatibility, rather than applying them unconditionally to modern builds now that they are no longer inherently incompatible.
|
Converted to Draft because this likely will still mismatch with most replays. |
It does mismatch on longer replays, the initial analysis of this was incomplete. But the optimised version of the function could be worthwhile using still, as long as it does not cause any mismatching when compiled on a VC6 build. |
| Matrix3D mtx; | ||
| const Bool stickToGround = true; // yes, set the "z" pos | ||
| TheTerrainLogic->alignOnTerrain(angle, pos, stickToGround, m_transform ); | ||
| TheTerrainLogic->alignOnTerrain(angle, m_cachedPos, true, m_transform); |
There was a problem hiding this comment.
if down the alignOnTerrain call chain it uses m_cachedPos or m_cachedAngle it will cause a mismatch since you moved the assignment
There was a problem hiding this comment.
The values are not mutated. Also the section under the if block is never actually run because the functionality was never implemented.
| @@ -208,40 +208,26 @@ void Thing::setOrientation( Real angle ) | |||
| Coord3D oldPos = m_cachedPos; | |||
| Matrix3D oldMtx = m_transform; | |||
|
|
|||
| pos.x = m_transform.Get_X_Translation(); | |||
There was a problem hiding this comment.
Vector3 vec = Get_Translation(); could had been used
| { | ||
| Matrix3D mtx; | ||
| const Bool stickToGround = true; // yes, set the "z" pos |
There was a problem hiding this comment.
removing bool is just change for change sake, that said matrix removal is useful as it avoids unnecessary codegen
| m_cacheFlags &= ~VALID_DIRVECTOR; // but don't clear the altitude flags. | ||
|
|
||
| reactToTransformChange(&oldMtx, &oldPos, oldAngle); | ||
| DEBUG_ASSERTCRASH(!(_isnan(getPosition()->x) || _isnan(getPosition()->y) || _isnan(getPosition()->z)), ("Drawable/Object position NAN! '%s'\n", m_template->getName().str() )); |
| x.z, y.z, z.z, pos.z ); | ||
| Real a = Sin(angle); | ||
| Real b = Cos(angle); | ||
| m_transform[0][0] = b; m_transform[0][1] = -a; m_transform[0][2] = 0; |
There was a problem hiding this comment.
Why?
if you thought this is cheaper, nope, m_transform[][] actually makes extra calls, Matrix3D::operator[] Vector4::operator[]
|
I'm not convinced this is enough to make 2022 builds and Retail match. For a start intermediate floating point maths is done at variable precision for 2022 (so floats are handled at float precision, doubles at double) while VC6 does everything at double... unless the code forces a spill to stack when it might use float. Also VC6 uses fastmath by default which does some unsafe compile time optimisation which can change the binary level outcome of a calculation which isn't enabled by default on 2022. |
Yeah it was a bit optimistic. Hadn't tested it with long replays. |
This fix makes all builds including modern C++20 builds and debug builds compatible with the retail game and with each other.
Fixes a floating point determinism bug in Thing::setOrientation that caused CRC mismatches when built with different compilers or optimization flags.
The cross product-based transformation code has been replaced with an equivalent, algebraically simplified implementation.
This new implementation is not only more efficient, but also eliminates intermediate floating point steps that were prone to subtle inaccuracies.
Also added the IGNORE_COMPATIBILITY macro (settable via CMake) for enabling future fixes that would break compatibility, rather than applying them unconditionally on modern builds.