[ZH] Fix level 1 compiler warnings for implicit integer type conversions and uninitialized variables in copy constructors #534

I am a bit skeptical about integer to float changes. What if it changes a math operation somewhere unexpectedly? According to a chatgpt inquiry it could change floating point precision in regards to optimizing more complex statements.

Maybe it would be better to

Disable warning C5055 in cmake and revert all changes that addressed it

or

Test that integer to float conversions introduce no game state changes. This would require feature implementation Implement feature to verify replay playback correctness #566

GeneralsMD/Code/GameEngine/Source/Common/PartitionSolver.cpp

GeneralsMD/Code/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarScheme.cpp

GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp

GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GameWindowTransitionsStyles.cpp

GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp

GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DModelDraw.cpp

helmutbuhler · 2025-04-02T19:14:30Z

I am a bit skeptical about integer to float changes. What if it changes a math operation somewhere unexpectedly? According to a chatgpt inquiry it could change floating point precision in regards to optimizing more complex statements.

Maybe it would be better to
* Disable warning C5055 in cmake and revert all changes that addressed it
or
* Test that integer to float conversions introduce no game state changes. This would require feature implementation [Implement feature to verify replay playback correctness #566](https://github.com/TheSuperHackers/GeneralsGameCode/issues/566)

Regarding replay checking: I will probably need some time to implement this, but you can already check this manually:

Merge: [ZH] Fix Replay CRC Checking #545
Create a replay in the retail version or get an existing one
Load the replay in the VC6 release build with the PR above merge and check that no mismatch message appears during playback.

But I also agree, one tiny mistake and we introduce a possible mismatch that might be hard to find. Also these types of warnings rarely indicate real flaws in my experience. So just supressing that warning doesn't seem like such a bad idea to me.

roossienb · 2025-04-03T06:23:10Z

@xezon, @Mauller, @helmutbuhler There seems to be some concern about floats and after careful consideration, I have to agree with those. Therefore, my proposal is to pick the following.

signed/unsigned conversion (Trig and random - we have discussed this on discord)
casting to HRESULT
Fixing string formatting
Use unsigned int in for-loops rather than signed int.
Fix compiler warnings for uninitialised variables in copy constructors

and discard the others. This still resolves about 2000 warnings.
The branch consists of multiple commits, so I can easily cherry-pick.

…e c++17 (TheSuperHackers#534)

…heSuperHackers#534)

…teger (TheSuperHackers#534)

…uctors (TheSuperHackers#534)

…rHackers#534)

roossienb · 2025-04-03T19:08:27Z

I've undone the float castings.

…teger (TheSuperHackers#534)

…uctors (TheSuperHackers#534)

Mauller

This looks good to go for me, if we get a quick VC6 check with the golden sample replay then it should help identify that the Trig removal is fine and the retail game does not use it either.

roossienb · 2025-04-08T07:16:05Z

VC6 replay tested and verified

xezon · 2025-04-09T07:21:58Z

GeneralsMD/Code/Libraries/Source/WWVegas/WWLib/random.cpp

-	static_cast<int>(0x7439a9d4), static_cast<int>(0x9cea8ac5), static_cast<int>(0x89537c5c), static_cast<int>(0x2588f55d),
-	static_cast<int>(0x415b5e1d), static_cast<int>(0x216e3d95), static_cast<int>(0x85c662e7), static_cast<int>(0x5e8ab368),
-	static_cast<int>(0x3ea5cc8c), static_cast<int>(0xd26a0f74), static_cast<int>(0xf3a9222b), static_cast<int>(0x48aad7e4)
+	(int)0xbaa96887, (int)0x1e17d32c, (int)0x03bcdc3c, (int)0x0f33d1b2,


The second commit should be merged into the first commit, because they touch the exact same code.

xezon · 2025-04-09T07:24:47Z

GeneralsMD/Code/Libraries/Source/WWVegas/WWLib/random.cpp

-	0x07439a9d4, 0x09cea8ac5, 0x089537c5c, 0x02588f55d,
-	0x0415b5e1d, 0x0216e3d95, 0x085c662e7, 0x05e8ab368,
-	0x03ea5cc8c, 0x0d26a0f74, 0x0f3a9222b, 0x048aad7e4
+	static_cast<int>(0xbaa96887), static_cast<int>(0x1e17d32c), static_cast<int>(0x03bcdc3c), static_cast<int>(0x0f33d1b2),


The first commit uses different wording style than the other commits.

It should say

"[ZH] Fix compiler warnings for [...]"

xezon · 2025-04-09T07:26:34Z

GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h

@@ -531,9 +531,13 @@ class GlobalData : public SubsystemInterface
 	GlobalData *newOverride( void );		/** create a new override, copy data from previous
 																			override, and return it */

-
+#if defined(_MSC_VER) && _MSC_VER < 1300


"uninitialised" is misspelled in third commit title.

xezon · 2025-04-09T07:33:40Z

Perhaps replicate the fixes straight into Generals as well and have 2 commits for [GEN][ZH] ? Would be clean. You could also open 2 Pull Requests for each of the commits, because this pull request has already undergone massive transformation.

… to signed integer [GEN] Changing cast to c-type (TheSuperHackers#549) [ZH] Fixed warnings of implicit conversion from unsigned to signed integer (TheSuperHackers#534) [ZH] Changing cast to c-type (TheSuperHackers#534)

roossienb · 2025-04-09T17:50:45Z

Closing this in favour of two new PR's as discussed above

#639
#640

…e c++17 (TheSuperHackers#534)

DevGeniusCode added Build Anything related to building, compiling ZeroHour Relates to Zero Hour labels Mar 28, 2025

Mauller changed the title ~~Fixes Level 1 warnings for Zero hour~~ [ZH] Fixes Level 1 warnings for Zero hour Mar 28, 2025

xezon reviewed Mar 28, 2025

View reviewed changes

xezon reviewed Mar 29, 2025

View reviewed changes

GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h Outdated Show resolved Hide resolved

GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Replacing 'register' keyword by a macro as it is deprecated sinc…

3411aac

…e c++17 (TheSuperHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fixing warnings related to converting between long and HRESULT (T…

6c7966a

…heSuperHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fix compiler warnings for uninitialised variables in copy constr…

bd7de7f

…uctors (TheSuperHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fixing warnings related to narrowing conversions (TheSuperHacker…

e154955

…s#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fixed warnings about signed/unsigned integer comparison (TheSupe…

a5d56b7

…rHackers#534)

roossienb force-pushed the fix/zh/warnings-level-1 branch from 916e6d6 to f05537e Compare March 29, 2025 21:33

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fix warnings about operators between enumerations and floating p…

f05537e

…oint types (TheSuperHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fix warnings about operators between enumerations and floating p…

add6c02

…oint types (TheSuperHackers#534)

roossienb force-pushed the fix/zh/warnings-level-1 branch from f05537e to add6c02 Compare March 29, 2025 21:47

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fixing warnings related to narrowing conversions (TheSuperHacker…

bfd60fc

…s#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fixed warnings about signed/unsigned integer comparison (TheSupe…

637cb17

…rHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fix warnings about operators between enumerations and floating p…

5e4ede3

…oint types (TheSuperHackers#534)

roossienb force-pushed the fix/zh/warnings-level-1 branch from add6c02 to 5e4ede3 Compare March 29, 2025 22:00

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fixed warning messages related to incorrect string formatting (T…

7f0d74b

…heSuperHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fixed warnings of implicit conversion from unsigned to signed in…

44abb48

…teger (TheSuperHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fix compiler warnings for uninitialised variables in copy constr…

18d4660

…uctors (TheSuperHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fixing warnings related to narrowing conversions (TheSuperHacker…

e8b3083

…s#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fixed warnings about signed/unsigned integer comparison (TheSupe…

31802f3

…rHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fix warnings about operators between enumerations and floating p…

5da039b

…oint types (TheSuperHackers#534)

roossienb force-pushed the fix/zh/warnings-level-1 branch from 5e4ede3 to 5da039b Compare March 29, 2025 22:03

This comment was marked as outdated.

Sign in to view

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fix compiler warnings for uninitialised variables in copy constr…

118a0d5

…uctors (TheSuperHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fixing warnings related to narrowing conversions (TheSuperHacker…

3046c42

…s#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Mar 29, 2025

[ZH] Fixed warnings about signed/unsigned integer comparison (TheSupe…

16b9de1

…rHackers#534)

roossienb force-pushed the fix/zh/warnings-level-1 branch from 6cc3b43 to d21a323 Compare April 2, 2025 07:04

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Apr 2, 2025

[ZH] Changing cast to c-type (TheSuperHackers#534)

bbf389e

xezon reviewed Apr 2, 2025

View reviewed changes

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Apr 3, 2025

[ZH] Replacing 'register' keyword by a macro as it is deprecated sinc…

4bef11f

…e c++17 (TheSuperHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Apr 3, 2025

[ZH] Fixing warnings related to converting between long and HRESULT (T…

cf496e6

…heSuperHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Apr 3, 2025

[ZH] Fixed warning messages related to incorrect string formatting (T…

ab7de24

…heSuperHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Apr 3, 2025

[ZH] Fixed warnings of implicit conversion from unsigned to signed in…

809a2d7

…teger (TheSuperHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Apr 3, 2025

[ZH] Fix compiler warnings for uninitialised variables in copy constr…

070199f

…uctors (TheSuperHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Apr 3, 2025

[ZH] Fixed warnings about signed/unsigned integer comparison (TheSupe…

adba37f

…rHackers#534)

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Apr 3, 2025

[ZH] Changing cast to c-type (TheSuperHackers#534)

17c4842

roossienb force-pushed the fix/zh/warnings-level-1 branch from bbf389e to 4a9e82b Compare April 3, 2025 19:02

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Apr 3, 2025

[ZH] Fixing incorrect cast (TheSuperHackers#534)

4a9e82b

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request Apr 3, 2025

[ZH] Removing blank line (TheSuperHackers#534)

dda2218

Bart Roossien added 3 commits April 7, 2025 15:18

[ZH] Fixed warnings of implicit conversion from unsigned to signed in…

ac30b7a

…teger (TheSuperHackers#534)

[ZH] Changing cast to c-type (TheSuperHackers#534)

58ce47c

[ZH] Fix compiler warnings for uninitialised variables in copy constr…

037483a

…uctors (TheSuperHackers#534)

roossienb force-pushed the fix/zh/warnings-level-1 branch from dda2218 to 037483a Compare April 7, 2025 13:27

Mauller approved these changes Apr 8, 2025

View reviewed changes

xezon changed the title ~~[ZH] Fixes Level 1 warnings for Zero hour~~ [ZH] Fix level 1 compiler warnings for implicit integer type conversions and uninitialized variables in copy constructors Apr 9, 2025

xezon reviewed Apr 9, 2025

View reviewed changes

roossienb closed this Apr 9, 2025

roossienb deleted the fix/zh/warnings-level-1 branch April 10, 2025 06:29

roossienb pushed a commit to roossienb/GeneralsGameCode that referenced this pull request May 3, 2025

[ZH] Replacing 'register' keyword by a macro as it is deprecated sinc…

4173839

…e c++17 (TheSuperHackers#534)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ZH] Fix level 1 compiler warnings for implicit integer type conversions and uninitialized variables in copy constructors #534

[ZH] Fix level 1 compiler warnings for implicit integer type conversions and uninitialized variables in copy constructors #534

roossienb commented Mar 28, 2025

xezon left a comment

This comment was marked as resolved.

This comment was marked as outdated.

xezon left a comment

helmutbuhler commented Apr 2, 2025

roossienb commented Apr 3, 2025 •

edited

Loading

roossienb commented Apr 3, 2025

Mauller left a comment

roossienb commented Apr 8, 2025

xezon Apr 9, 2025

xezon Apr 9, 2025

xezon Apr 9, 2025

xezon commented Apr 9, 2025 •

edited

Loading

roossienb commented Apr 9, 2025

[ZH] Fix level 1 compiler warnings for implicit integer type conversions and uninitialized variables in copy constructors #534

[ZH] Fix level 1 compiler warnings for implicit integer type conversions and uninitialized variables in copy constructors #534

Conversation

roossienb commented Mar 28, 2025

xezon left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as outdated.

xezon left a comment

Choose a reason for hiding this comment

helmutbuhler commented Apr 2, 2025

roossienb commented Apr 3, 2025 • edited Loading

roossienb commented Apr 3, 2025

Mauller left a comment

Choose a reason for hiding this comment

roossienb commented Apr 8, 2025

xezon Apr 9, 2025

Choose a reason for hiding this comment

xezon Apr 9, 2025

Choose a reason for hiding this comment

xezon Apr 9, 2025

Choose a reason for hiding this comment

xezon commented Apr 9, 2025 • edited Loading

roossienb commented Apr 9, 2025

roossienb commented Apr 3, 2025 •

edited

Loading

xezon commented Apr 9, 2025 •

edited

Loading