Skip to content

refactor: Fix unnecessary copy initializations - #3202

Merged
xezon merged 7 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:fix/clang-tidy-unnecessary-copies
Aug 30, 2026
Merged

refactor: Fix unnecessary copy initializations#3202
xezon merged 7 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:fix/clang-tidy-unnecessary-copies

Conversation

@CryoTheRenegade

@CryoTheRenegade CryoTheRenegade commented Aug 26, 2026

Copy link
Copy Markdown
  • Fix all 73 performance-unnecessary-copy-initialization findings across 29 files from clang-tidy.
  • Bind const-only locals to const references.
  • Remove unused copies and one-use aliases.

@Caball009 Added their fixes as well, now up to 121 findings across 61 files

@CryoTheRenegade
CryoTheRenegade marked this pull request as ready for review August 26, 2026 00:28
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Eliminate unnecessary local copy initialization

✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Eliminates 73 clang-tidy unnecessary-copy findings across shared and game-specific C++ code.
• Reuses immutable values through const references in runtime, rendering, and tooling paths.
• Removes unused or single-use aliases while preserving existing behavior.
Diagram

graph TD
  A["clang-tidy findings"] --> B["Copy cleanup"] --> C["Shared Core"] --> D["Render and audio"]
  B --> E["Generals"] --> F["Game logic"]
  B --> G["Zero Hour"] --> H["WorldBuilder tools"]
Loading
High-Level Assessment

The targeted const-reference and direct-argument approach is appropriate because it removes flagged copies without changing APIs or control flow. Broader iterator or range-loop modernization was considered but would add unrelated churn and complicate verification across mirrored product sources.

Files changed (29) +60 / -79

Refactor (29) +60 / -79
ReplaySimulation.cppReference replay filenames during simulation +1/-1

Reference replay filenames during simulation

• Binds each replay filename to the existing vector element instead of copying it before logging and simulation.

Core/GameEngine/Source/Common/ReplaySimulation.cpp

ControlBar.cppReference command science requirements +1/-1

Reference command science requirements

• Reads the command button's science vector through a const reference when selecting the first required science.

Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp

LadderDefs.cppReference ladder factions and map names +2/-2

Reference ladder factions and map names

• Avoids copying faction and quick-match map strings while validating ladder configuration and map metadata.

Core/GameEngine/Source/GameNetwork/GameSpy/LadderDefs.cpp

W3DTerrainVisual.cppReference building geometry for faction bibs +1/-1

Reference building geometry for faction bibs

• Uses the building's existing geometry information directly when calculating faction bib dimensions.

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DTerrainVisual.cpp

W3DTreeBuffer.cppReference the camera transform during culling +1/-1

Reference the camera transform during culling

• Reads the camera transform through a const reference while deriving the tree-culling view direction.

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DTreeBuffer.cpp

rendobj.cppReference camera viewport for screen sizing +1/-1

Reference camera viewport for screen sizing

• Avoids copying the viewport while calculating a render object's projected screen size.

Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp

texproject.cppRemove projection transform aliases +2/-4

Remove projection transform aliases

• Passes the input transform directly into perspective and orthographic texture projection multiplication, removing one-use matrix copies.

Core/Libraries/Source/WWVegas/WW3D2/texproject.cpp

Sound3D.cppTransform sound positions without a local copy +1/-2

Transform sound positions without a local copy

• Passes the supplied position directly into listener-space transformation instead of creating a redundant vector alias.

Core/Libraries/Source/WWVegas/WWAudio/Sound3D.cpp

W3DViewDoc.cppReference asset dependency filenames +1/-1

Reference asset dependency filenames

• Uses dependency-list filenames by const reference while constructing source and destination asset paths.

Core/Tools/W3DView/W3DViewDoc.cpp

textureCompress.cppReference matched texture file metadata +2/-2

Reference matched texture file metadata

• Avoids copying source and cache file records when comparing modification times during directory scans.

Core/Tools/textureCompress/textureCompress.cpp

WOLBuddyOverlay.cppReference Generals buddy messages +1/-1

Reference Generals buddy messages

• Passes each stored buddy message to chat insertion without copying the list element.

Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLBuddyOverlay.cpp

WOLQuickMatchMenu.cppReference Generals quick-match selections +2/-2

Reference Generals quick-match selections

• Avoids copying map and faction strings while populating quick-match maps and resolving the selected side.

Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLQuickMatchMenu.cpp

ScriptConditions.cppEliminate redundant Generals condition strings +7/-13

Eliminate redundant Generals condition strings

• Binds immutable parameter strings by const reference and removes unused trigger-name copies across script condition evaluation.

Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptConditions.cpp

ScriptEngine.cppReference Generals script lookup values +5/-5

Reference Generals script lookup values

• Reads special-power list pairs and object names by const reference during completion checks and topple adjustment.

Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptEngine.cpp

meshmdl.cppUse bone transforms directly for deformation +1/-4

Use bone transforms directly for deformation

• Removes an unchanged matrix copy and uses the bone transform directly when composing the deformed vertex buffer.

Generals/Code/Libraries/Source/WWVegas/WW3D2/meshmdl.cpp

part_buf.cppReference Generals particle viewport +1/-1

Reference Generals particle viewport

• Avoids copying the camera viewport while preparing particle level-of-detail calculations.

Generals/Code/Libraries/Source/WWVegas/WW3D2/part_buf.cpp

EditObjectParameter.cppReference Generals object side names +1/-1

Reference Generals object side names

• Uses the template's default owning side by const reference while organizing WorldBuilder objects.

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

EditParameter.cppReference Generals upgrade names +1/-1

Reference Generals upgrade names

• Reads upgrade names directly from the collection while populating the WorldBuilder combo box.

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

ScriptDialog.cppReference Generals script parameter names +4/-4

Reference Generals script parameter names

• Avoids copying waypoint, path, team, and unit parameter strings during WorldBuilder script dependency scanning.

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

Module.cppReference Zero Hour trigger upgrades +1/-1

Reference Zero Hour trigger upgrades

• Compares trigger upgrade names directly from the module's vector instead of copying each string.

GeneralsMD/Code/GameEngine/Source/Common/Thing/Module.cpp

WOLBuddyOverlay.cppReference Zero Hour buddy messages +1/-1

Reference Zero Hour buddy messages

• Passes each stored buddy message to chat insertion without copying the list element.

GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLBuddyOverlay.cpp

WOLQuickMatchMenu.cppReference Zero Hour quick-match selections +2/-2

Reference Zero Hour quick-match selections

• Avoids copying map and faction strings while populating quick-match maps and resolving the selected side.

GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLQuickMatchMenu.cpp

OCLUpdate.cppReference faction OCL entries +1/-1

Reference faction OCL entries

• Reads faction-specific object creation data by const reference while selecting the matching faction entry.

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/OCLUpdate.cpp

ScriptConditions.cppEliminate redundant Zero Hour condition strings +7/-14

Eliminate redundant Zero Hour condition strings

• Binds immutable parameter strings by const reference and removes unused trigger-name copies across script condition evaluation.

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptConditions.cpp

ScriptEngine.cppReference Zero Hour script lookup values +5/-5

Reference Zero Hour script lookup values

• Reads special-power list pairs and object names by const reference during completion checks and topple adjustment.

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptEngine.cpp

part_buf.cppReference Zero Hour particle viewport +1/-1

Reference Zero Hour particle viewport

• Avoids copying the camera viewport while preparing particle level-of-detail calculations.

GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/part_buf.cpp

EditObjectParameter.cppReference Zero Hour object side names +1/-1

Reference Zero Hour object side names

• Uses the template's default owning side by const reference while organizing WorldBuilder objects.

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

EditParameter.cppReference Zero Hour upgrade names +1/-1

Reference Zero Hour upgrade names

• Reads upgrade names directly from the collection while populating the WorldBuilder combo box.

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

ScriptDialog.cppReference Zero Hour script parameter names +4/-4

Reference Zero Hour script parameter names

• Avoids copying waypoint, path, team, and unit parameter strings during WorldBuilder script dependency scanning.

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

@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 start a comment with 'qodo' or '@qodo' to chat about any finding

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@Caball009

Copy link
Copy Markdown

Are you also going to include the ones from #3157 in this pr? I think I could push what I have if you like.

@CryoTheRenegade

Copy link
Copy Markdown
Author

Are you also going to include the ones from #3157 in this pr? I think I could push what I have if you like.

Go for it

@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown

Greptile Summary

This PR removes unnecessary C++ object copies by binding read-only locals to const references, eliminating unused aliases, and passing existing values directly.

  • Updates shared engine, rendering, audio, networking, and tooling code.
  • Applies equivalent cleanup across the Generals and GeneralsMD variants.
  • Preserves copies where local mutation is required.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
Core/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DModelDraw.cpp Replaces temporary transform copies with const references in client-side rendering and particle-system paths.
Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptConditions.cpp Removes unused aliases and binds read-only script parameter strings to const references.
GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptConditions.cpp Mirrors the script-condition copy cleanup for the Zero Hour variant.
Generals/Code/Tools/WorldBuilder/src/DrawObject.cpp Simplifies transform construction and removes redundant reset copies while retaining locally mutable transforms.
GeneralsMD/Code/Tools/WorldBuilder/src/DrawObject.cpp Applies the equivalent WorldBuilder transform cleanup to the Zero Hour tool.

Reviews (8): Last reviewed commit: "fix: address follow-up Matrix3D review f..." | Re-trigger Greptile

Comment thread Core/GameEngine/Source/Common/Audio/GameAudio.cpp Outdated
@Caball009

Caball009 commented Aug 27, 2026

Copy link
Copy Markdown

Are you also going to include the ones from #3157 in this pr? I think I could push what I have if you like.

Go for it

I've pushed what I have. I've tried to be conservative with the Matrix3D changes, but it's still quite a number.

There are a lot of Matrix3D copies and not all are desirable to be changed to const&.

Here's an example of a pattern that comes up a lot:

// ugh... kill the mtx so we get it in modelspace, not world space
Matrix3D originalTransform = m_renderObject->Get_Transform(); // save the transform
Matrix3D tmp(true);
tmp.Scale(getDrawable()->getScale());
m_renderObject->Set_Transform(tmp); // set to identity transform
const Matrix3D boneTransform = m_renderObject->Get_Bone_Transform(boneIndex);
Vector3 vpos = boneTransform.Get_Translation();
rotation = boneTransform.Get_Z_Rotation();
m_renderObject->Set_Transform(originalTransform); // restore it

Changing originalTransform to const& would compile but change behavior.

@CryoTheRenegade

Copy link
Copy Markdown
Author

This should probably be good for now and we can do some more through static analysis later on

@Caball009

Caball009 commented Aug 27, 2026

Copy link
Copy Markdown

I'd like to see commit 4 and 7 gone ideally. Is that something that'd be trivial for you? Otherwise I'll have to wrestle a bit with Git to get it done.

EDIT: these commits:
e2d3915
742f755

@CryoTheRenegade

Copy link
Copy Markdown
Author

Can do, can you specify which commits or files? 4 and 7 can be ambiguous as sometimes it sorted differently

@Caball009

Copy link
Copy Markdown

I thought I'd edit my previous comment with the commits to avoid another comment, but perhaps you missed it. The PR description also needs updating.

@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown

Want your agent to iterate on Greptile's feedback? Try greploops.

@Caball009

Copy link
Copy Markdown

I don't understand what the point of the last 3 commits is. It certainly doesn't do what I requested.

@CryoTheRenegade

Copy link
Copy Markdown
Author

I reverted the commits as you asked, but that broke CI for some reason so the last two were to fix that. Did i misunderstand what you asked for?

@Caball009
Caball009 force-pushed the fix/clang-tidy-unnecessary-copies branch from 5f5cde6 to 9a03b9d Compare August 28, 2026 16:26
@Caball009

Copy link
Copy Markdown

I reverted the commits as you asked, but that broke CI for some reason so the last two were to fix that. Did i misunderstand what you asked for?

(I didn't ask for the commits to be reverted but to be removed). It's done now.

@CryoTheRenegade

Copy link
Copy Markdown
Author

Ok, my bad

Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/FlatHeightMap.cpp Outdated
Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/HeightMap.cpp Outdated
Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/HeightMap.cpp Outdated
Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/FlatHeightMap.cpp Outdated
Comment thread Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDebugIcons.cpp Outdated
Comment thread Generals/Code/Tools/WorldBuilder/src/DrawObject.cpp Outdated
Comment thread Generals/Code/Tools/WorldBuilder/src/DrawObject.cpp Outdated
Comment thread Generals/Code/Tools/WorldBuilder/src/DrawObject.cpp Outdated
Comment thread Generals/Code/Tools/WorldBuilder/src/DrawObject.cpp Outdated
Comment thread GeneralsMD/Code/Tools/WorldBuilder/src/DrawObject.cpp Outdated
Comment thread GeneralsMD/Code/Tools/WorldBuilder/src/DrawObject.cpp Outdated
Comment thread GeneralsMD/Code/Tools/WorldBuilder/src/DrawObject.cpp Outdated
@xezon xezon changed the title Fix unnecessary copy initialization findings refactor: Fix unnecessary copy initializations Aug 29, 2026
@xezon xezon added Performance Is a performance concern Refactor Edits the code with insignificant behavior changes, is never user facing labels Aug 29, 2026

@xezon xezon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

@xezon
xezon merged commit ccff285 into TheSuperHackers:main Aug 30, 2026
23 checks passed
@CryoTheRenegade
CryoTheRenegade deleted the fix/clang-tidy-unnecessary-copies branch August 30, 2026 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Performance Is a performance concern Refactor Edits the code with insignificant behavior changes, is never user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants