Skip to content

fix: compiler warnings with potential side effects#2934

Merged
bruno-dasilva merged 4 commits into
masterfrom
bruno/fix-warnings-4
Jun 2, 2026
Merged

fix: compiler warnings with potential side effects#2934
bruno-dasilva merged 4 commits into
masterfrom
bruno/fix-warnings-4

Conversation

@bruno-dasilva

@bruno-dasilva bruno-dasilva commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

Purpose

Fix the warning spam when compiling with clang, gcc, or msvc (mostly clang). This PR focuses on the more "potenitally breaking" fixes. Some of these are false positives (as in the underlying code has no bug) but changing the code avoids the warning. There are a few legit bugs that have slight behaviour changes.

I've gone through and successfully compiled every non-trivial fix, pulling the actual warning from the compiler into a review comment. I've added some explanations on sections where I wasn't sure whether the change was safe or why the change was made, to try and add detail for other reviewers.

AI Disclosure

This PR was generated through running claude code in a loop to fix every warning. However, every non-trivial change was inspected and several "fixes" were dropped in favor of different solutions to try and keep backwards compatibility in mind. I've called out the couple places where I don't understand the fix or the implication of the fix to other reviewers.

@bruno-dasilva

bruno-dasilva commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator Author

Comment thread rts/Lua/LuaOpenGLUtils.cpp
Comment thread rts/Map/SMF/SMFGroundTextures.cpp
Comment thread rts/Rendering/GL/glHelpers.h
Comment thread rts/Rendering/Textures/Bitmap.cpp
Comment thread rts/Rendering/DepthBufferCopy.cpp
Comment thread rts/Sim/MoveTypes/MoveDefHandler.cpp Outdated
Comment thread rts/Sim/Units/Scripts/CobDeferredCallin.cpp
Comment thread rts/Sim/Path/HAPFS/PathingState.cpp
@sprunk

sprunk commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Looks good apart from the waterline one. I'd keep the value within integer range there

@bruno-dasilva bruno-dasilva force-pushed the bruno/fix-warnings-3 branch from a634fdc to 6ddbfda Compare May 28, 2026 11:33
@bruno-dasilva bruno-dasilva force-pushed the bruno/fix-warnings-4 branch from 95ae94c to b0d3a81 Compare June 1, 2026 09:25
@bruno-dasilva bruno-dasilva requested a review from sprunk June 1, 2026 09:39
@bruno-dasilva bruno-dasilva changed the base branch from bruno/fix-warnings-3 to master June 1, 2026 09:41
@bruno-dasilva bruno-dasilva merged commit d6a481d into master Jun 2, 2026
4 checks passed
@bruno-dasilva bruno-dasilva deleted the bruno/fix-warnings-4 branch June 2, 2026 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants