Skip to content

fix(heightmapedit): Fix undefined behaviors in WorldHeightMapEdit::remapTextures, WorldHeightMapEdit::findBoundaryNear - #3201

Merged
xezon merged 1 commit into
TheSuperHackers:mainfrom
CryoTheRenegade:fix/worldbuilder-heightmap-uninitialized-null
Aug 29, 2026
Merged

fix(heightmapedit): Fix undefined behaviors in WorldHeightMapEdit::remapTextures, WorldHeightMapEdit::findBoundaryNear#3201
xezon merged 1 commit into
TheSuperHackers:mainfrom
CryoTheRenegade:fix/worldbuilder-heightmap-uninitialized-null

Conversation

@CryoTheRenegade

Copy link
Copy Markdown

Initialize anyChanges to false so remapTextures() returns a defined value when no texture classes change.
Return from findBoundaryNear() after handling a null point to prevent a null dereference.
Found with clang-tidy

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Fix undefined height map editor behavior

🐞 Bug fix 🕐 Less than 10 minutes

Grey Divider

AI Description

• Initialize texture remapping state to return false when no classes change.
• Stop boundary searches after null input handling to prevent dereferencing null pointers.
• Apply both safety fixes across the Generals and GeneralsMD editors.
Diagram

graph TD
  A["Height map edit"] --> B{"Requested action"}
  B -->|"Remap textures"| C["False baseline"] --> D["Defined result"]
  B -->|"Find boundary"| E{"Point available"}
  E -->|"No"| F["Set index -1"] --> G["Return safely"]
  E -->|"Yes"| H["Search boundaries"]
Loading
High-Level Assessment

The PR uses the appropriate minimal defensive fixes: initialize the accumulator at declaration and return immediately after handling invalid input. Broader API changes, such as replacing pointer outputs with value-returning types, would create unnecessary scope for these mirrored legacy implementations.

Files changed (2) +4 / -2

Bug fix (2) +4 / -2
WHeightMapEdit.cppMake Generals height map operations deterministic and null-safe +2/-1

Make Generals height map operations deterministic and null-safe

• Initializes the texture-remapping change flag to false so the function always returns a defined value. Returns immediately after reporting a null boundary point, preventing coordinate access through a null pointer.

Generals/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp

WHeightMapEdit.cppMirror height map safety fixes in GeneralsMD +2/-1

Mirror height map safety fixes in GeneralsMD

• Applies the same deterministic texture-remapping state and null-point early return to the GeneralsMD WorldBuilder implementation, keeping both variants behaviorally aligned.

GeneralsMD/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp

@Skyaero42

Copy link
Copy Markdown

How would this undefined behaviour show in the game?

@Caball009 Caball009 added Minor Severity: Minor < Major < Critical < Blocker WorldBuilder Relates to World Builder labels Aug 27, 2026
@Caball009

Caball009 commented Aug 27, 2026

Copy link
Copy Markdown

Function: WorldHeightMapEdit::remapTextures
User action: World Builder -> Texture Sizing -> Remap Textures -> Cancel

Looks like it just does unnecessary work.


Function: WorldHeightMapEdit::findBoundaryNear
User action: World Builder -> Border Tool -> Click on map border

Cannot be nullptr because it's initialized in BorderTool::mouseDown as Coord3D groundPt and only has one call site currently.

Ideally, all parameters for that function are passed by reference, but that's a larger change as there are many functions in that class that pass by pointer.

@xezon xezon changed the title fix(worldbuilder): Fix height map undefined behavior fix(heightmapedit): Fix undefined behaviors in WorldHeightMapEdit::remapTextures, WorldHeightMapEdit::findBoundaryNear Aug 29, 2026
@xezon xezon added the Fix Is fixing something, but is not user facing label Aug 29, 2026
@xezon
xezon merged commit 21aa508 into TheSuperHackers:main Aug 29, 2026
16 checks passed
@CryoTheRenegade
CryoTheRenegade deleted the fix/worldbuilder-heightmap-uninitialized-null branch August 29, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Fix Is fixing something, but is not user facing Minor Severity: Minor < Major < Critical < Blocker WorldBuilder Relates to World Builder

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants