performance(Drawable): Adjusts Turret Positioning, Recoil and Muzzle for Model Draw to Update Only when Necessary by IamInnocent3X · Pull Request #2046 · TheSuperHackers/GeneralsGameCode

IamInnocent3X · 2026-01-01T16:42:57Z

Modifies the GameLogic to tell handleClientTurretPositioning() and handleClientRecoil() function in W3DModelDraw to only update when Necessary.

Skyaero42

Any indication on the performance gain?

This may need extensive testing for mismatching.

Skyaero42 · 2026-01-02T18:25:06Z

+//-------------------------------------------------------------------------------------------------
+void W3DModelDraw::setNeedUpdateTurretPositioning(Bool set)
+{
+	m_needUpdateTurretPosition = set; // A simple function with the dangers of an atomic bomb, misuse and it'll cause desync


Comment not necessary.

Skyaero42 · 2026-01-02T18:26:23Z

 	if( xfer->getXferMode() == XFER_LOAD && m_subObjectVec.empty() == FALSE )
 		updateSubObjects();

+#if !RETAIL_COMPATIBLE_CRC


Does this require a xfer version change?

IamInnocent3X · 2026-01-02T23:58:50Z

Any indication on the performance gain?

I am not sure, but the devs does mention it being not a major problem.

Line 2494 @todo fix me someday (srj)

handleClientRecoil function are majority just For Loop Checks, each time checks with for loops based on how many Maximum Weapons are in game, (In this case, 3), and checks for each Weapon Slot whether there are any recoil to handle. If it does, it handles the Model Bone Recoil and handles their Muzzle from showing. Every Barrel Bone present will require checks.

handleClientTurretPositioning however, is O(n) based. It calls for all Turrets for each Weapon Slot to update Every Frame regardless if there are any changes. For each turret in the runtime it calls for AIUpdateInterface to get their Turret Pitch and Angle, then Transforms the Matrix up to 2 Times (One for Angle, One for Pitch). This is called every frame.

The changes just add a go/stop variable at the beginning of both functions, (m_doHandleRecoil, and m_needUpdateTurretPosition) to inform W3DModelDraw to allow the calling of the respective functions.

stephanmeesters · 2026-01-03T12:04:34Z

I think this can be simplified by removing the changes to xfer and instead forcing a recompute right after you load the savegame, like this (do test it):

diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp
  @@ void Object::loadPostProcess()
   	if( m_xferContainedByID != INVALID_ID )
   		m_containedBy = TheGameLogic->findObjectByID(m_xferContainedByID);
   	else
   		m_containedBy = NULL;
  +
  +	setNeedUpdateTurretPositioning(TRUE);

diff --git a/GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DModelDraw.cpp b/GeneralsMD/Code/GameEngineDevice/
  Source/W3DDevice/GameClient/Drawable/Draw/W3DModelDraw.cpp
  @@ void W3DModelDraw::loadPostProcess( void )
   	// extend base class
   	DrawModule::loadPostProcess();
  +
  +	m_needUpdateTurretPosition = TRUE;
  +	m_doHandleRecoil = TRUE;

This may change the CRC right after loading a savegame but I don't see how that would be a problem

IamInnocent3X · 2026-01-04T12:16:30Z

I think this can be simplified by removing the changes to xfer and instead forcing a recompute right after you load the savegame, like this (do test it):

diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp
  @@ void Object::loadPostProcess()
   	if( m_xferContainedByID != INVALID_ID )
   		m_containedBy = TheGameLogic->findObjectByID(m_xferContainedByID);
   	else
   		m_containedBy = NULL;
  +
  +	setNeedUpdateTurretPositioning(TRUE);

diff --git a/GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DModelDraw.cpp b/GeneralsMD/Code/GameEngineDevice/
  Source/W3DDevice/GameClient/Drawable/Draw/W3DModelDraw.cpp
  @@ void W3DModelDraw::loadPostProcess( void )
   	// extend base class
   	DrawModule::loadPostProcess();
  +
  +	m_needUpdateTurretPosition = TRUE;
  +	m_doHandleRecoil = TRUE;

This may change the CRC right after loading a savegame but I don't see how that would be a problem

This change seems logical, tested on my end from loading a retail compatible save and it seems fine. I'll adjust and update my commit.

Caball009

I don't think the turret positioning implementation is right.

I built a couple of firebases. I moved the camera somewhere else and then back to the firebases and the turrets snap to a certain angle. It looks like a purely visual thing, though, because it didn't cause a mismatch with a client that didn't have PR.

IamInnocent3X · 2026-06-30T09:15:52Z

Alright, thanks for reporting. Pushed an update, it should fix it.

Edit: Nevermind, the issue still persist, just less noticable. I'm looking into it.

greptile-apps · 2026-06-30T09:30:21Z

Greptile Summary

This PR introduces a performance optimization that skips handleClientTurretPositioning and handleClientRecoil in W3DModelDraw when no work is needed, reducing per-frame CPU cost for units with idle or static turrets. Two new boolean flags (m_needUpdateTurretPosition, m_doHandleRecoil) gate the render-side updates, driven by a lightweight deduplication layer in Object → Drawable → W3DModelDraw.

TurretAI state transitions (AimTurretState, RecenterTurretState, IdleScanState, setTurretTargetObject/Position, recenterTurret, setTurretEnabled) now set setNeedUpdateTurretPositioning appropriately so the draw module only updates bones while the turret is actually rotating.
m_doHandleRecoil is set to TRUE when a weapon fires (handleWeaponFireFX, rebuildWeaponRecoilInfo) and self-extinguishes once all barrels return to IDLE, ensuring recoil/muzzle-flash animation runs exactly as long as needed.
Object::setNeedUpdateTurretPositioning deduplicates redundant calls with a cached m_turretNeedPositioning field, preventing unnecessary traversal of the draw module list each frame.

Confidence Score: 4/5

Mostly safe to merge; the single-flag-per-object design means one turret's quiescence can suppress bone updates for sibling turrets on multi-turret units (acknowledged in a prior review thread).

The optimization logic is sound for single-turret units and the recoil path is well-guarded. The known multi-turret flag sharing issue is the main open risk. The new code is otherwise correct and the deduplication layer in Object is implemented cleanly.

TurretAI.cpp and W3DModelDraw.cpp are the most behaviorally significant files; the multi-turret interaction through the shared Object-level flag warrants extra attention for units with more than one turret slot.

Important Files Changed

Filename	Overview
GeneralsMD/Code/GameEngine/Source/GameLogic/AI/TurretAI.cpp	Adds setNeedUpdateTurretPositioning calls in all turret state transitions. One if-body-on-same-line style violation at the new setTurretEnabled call.
Core/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DModelDraw.cpp	Gates handleClientTurretPositioning and handleClientRecoil behind new flags; resets them in constructor, loadPostProcess, handleWeaponFireFX, and rebuildWeaponRecoilInfo.
GeneralsMD/Code/GameEngine/Include/Common/DrawModule.h	Adds setNeedUpdateTurretPositioning as a pure virtual to ObjectDrawInterface, correctly scoped so only ObjectDrawInterface implementors must provide it.
GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp	Implements Drawable::setNeedUpdateTurretPositioning, forwarding only to draw modules with an ObjectDrawInterface.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp	Implements Object::setNeedUpdateTurretPositioning with deduplication guard; initialises flag to FALSE in constructor and forces TRUE in loadPostProcess.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant TAI as TurretAI (Logic)
    participant OBJ as Object
    participant DRW as Drawable
    participant MDL as W3DModelDraw

    Note over TAI: Turret starts rotating
    TAI->>OBJ: setNeedUpdateTurretPositioning(TRUE)
    OBJ->>OBJ: cache check
    OBJ->>DRW: setNeedUpdateTurretPositioning(TRUE)
    DRW->>MDL: setNeedUpdateTurretPositioning(TRUE)
    loop Every render frame (rotating)
        MDL->>MDL: handleClientTurretPositioning() runs
    end
    Note over TAI: Turret aligns
    TAI->>OBJ: setNeedUpdateTurretPositioning(FALSE)
    OBJ->>DRW: setNeedUpdateTurretPositioning(FALSE)
    DRW->>MDL: setNeedUpdateTurretPositioning(FALSE)
    loop Every render frame (idle)
        MDL->>MDL: handleClientTurretPositioning() SKIPPED
    end
    Note over MDL: Weapon fires
    MDL->>MDL: "handleWeaponFireFX sets m_doHandleRecoil=TRUE"
    loop Every render frame (RECOIL/SETTLE)
        MDL->>MDL: handleClientRecoil() runs
    end
    Note over MDL: All barrels reach IDLE, m_doHandleRecoil=FALSE

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant TAI as TurretAI (Logic)
    participant OBJ as Object
    participant DRW as Drawable
    participant MDL as W3DModelDraw

    Note over TAI: Turret starts rotating
    TAI->>OBJ: setNeedUpdateTurretPositioning(TRUE)
    OBJ->>OBJ: cache check
    OBJ->>DRW: setNeedUpdateTurretPositioning(TRUE)
    DRW->>MDL: setNeedUpdateTurretPositioning(TRUE)
    loop Every render frame (rotating)
        MDL->>MDL: handleClientTurretPositioning() runs
    end
    Note over TAI: Turret aligns
    TAI->>OBJ: setNeedUpdateTurretPositioning(FALSE)
    OBJ->>DRW: setNeedUpdateTurretPositioning(FALSE)
    DRW->>MDL: setNeedUpdateTurretPositioning(FALSE)
    loop Every render frame (idle)
        MDL->>MDL: handleClientTurretPositioning() SKIPPED
    end
    Note over MDL: Weapon fires
    MDL->>MDL: "handleWeaponFireFX sets m_doHandleRecoil=TRUE"
    loop Every render frame (RECOIL/SETTLE)
        MDL->>MDL: handleClientRecoil() runs
    end
    Note over MDL: All barrels reach IDLE, m_doHandleRecoil=FALSE

_{Reviews (3): Last reviewed commit: "Redo Fix for Turret Snapping" | Re-trigger Greptile}

greptile-apps · 2026-06-30T09:30:25Z

+	if(!m_needUpdateTurretPosition && !m_lastNeedUpdateTurretPosition)
+		return;
+
+	m_lastNeedUpdateTurretPosition = m_needUpdateTurretPosition;


Single Object flag spans all turret slots

m_needUpdateTurretPosition is one Bool for the entire object, but handleClientTurretPositioning loops over MAX_TURRETS. Any path that clears the flag (e.g. a two-turret unit where RecenterTurretState finishes for slot 1) suppresses updates for every other turret slot. A per-slot flag array, or a reference-counted active-turret counter, would prevent one turret's quiescence from silencing a sibling that is still rotating.

Prompt To Fix With AI

This is a comment left during a code review. Path: GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DModelDraw.cpp Line: 2426-2429 Comment: **Single Object flag spans all turret slots** `m_needUpdateTurretPosition` is one `Bool` for the entire object, but `handleClientTurretPositioning` loops over `MAX_TURRETS`. Any path that clears the flag (e.g. a two-turret unit where `RecenterTurretState` finishes for slot 1) suppresses updates for every other turret slot. A per-slot flag array, or a reference-counted active-turret counter, would prevent one turret's quiescence from silencing a sibling that is still rotating. How can I resolve this? If you propose a fix, please make it concise.

IamInnocent3X · 2026-06-30T12:07:21Z

Alright, thanks for reporting. Pushed an update, it should fix it.

Edit: Nevermind, the issue still persist, just less noticable. I'm looking into it.

Ignore the above, the issue is linked towards my GameClient Drawable Iteration Issue in another PR. That being said, without that Iteration feature even before the fix attempt, I cannot replicate the bug by moving the camera either by scrolling or clicking from the minimap. @Caball009 , able to send a video on replicating it?

Caball009 · 2026-06-30T15:01:47Z

Here you go.

firebase_turret_angle_snap.mp4

IamInnocent3X · 2026-07-01T10:40:25Z

Thank you, I have reattempted the fix as TurretAI's function expands beyond AIUpdate. I have also tested the fix on the current updated version of the SH Repo. Should be fine now.

Caball009 · 2026-07-01T14:58:02Z

Nice, I'll try to make some time to test it later.

Can you rebase this branch?

IamInnocent3X · 2026-07-06T07:35:58Z

Nice, I'll try to make some time to test it later.

Can you rebase this branch?

It's done. Sorry for the wait.

…Update when Necessary

…a on Client's side

Skyaero42 requested changes Jan 2, 2026

View reviewed changes

IamInnocent3X requested a review from Skyaero42 January 4, 2026 12:51

Caball009 requested changes Jun 29, 2026

View reviewed changes

IamInnocent3X force-pushed the W3DModelDrawPerfImprovements branch from 3ea78f3 to 3e96ab1 Compare June 30, 2026 09:24

greptile-apps Bot reviewed Jun 30, 2026

View reviewed changes

IamInnocent3X force-pushed the W3DModelDrawPerfImprovements branch from 955c9e1 to b402a9b Compare July 6, 2026 07:34

IamInnocent3X added 5 commits July 6, 2026 15:38

Adjusted handleClientTurretPositioning() and handleClientRecoil() to …

2e73a71

…Update when Necessary

Adjusted according to Changes Proposed

957cc83

Changes retail xfer to loadPostProcess

032da9b

Fixed Turret snapping into other Positions when moved away from Camer…

d9014c8

…a on Client's side

Redo Fix for Turret Snapping

ab42a7e

IamInnocent3X force-pushed the W3DModelDrawPerfImprovements branch from b402a9b to ab42a7e Compare July 6, 2026 07:38

Uh oh!

Conversation

IamInnocent3X commented Jan 1, 2026

Uh oh!

Skyaero42 left a comment

Choose a reason for hiding this comment

Uh oh!

Skyaero42 Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Skyaero42 Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

IamInnocent3X commented Jan 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stephanmeesters commented Jan 3, 2026

Uh oh!

IamInnocent3X commented Jan 4, 2026

Uh oh!

Caball009 left a comment

Choose a reason for hiding this comment

Uh oh!

IamInnocent3X commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps Bot Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

IamInnocent3X commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Caball009 commented Jun 30, 2026

Uh oh!

IamInnocent3X commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Caball009 commented Jul 1, 2026

Uh oh!

IamInnocent3X commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

IamInnocent3X commented Jan 2, 2026 •

edited

Loading

IamInnocent3X commented Jun 30, 2026 •

edited

Loading

greptile-apps Bot commented Jun 30, 2026 •

edited

Loading

IamInnocent3X commented Jun 30, 2026 •

edited

Loading

IamInnocent3X commented Jul 1, 2026 •

edited

Loading