bugfix(smudge): Decouple smudge time step from render update

[xezon]

• Merge after fix(smudge): Fix minor errors in W3DSmudge #2483
• Fixes USA Microwave Heat Haze is not halting on game pause #1694

This change decouples the smudge time step from the render update. Smudge refers to the heat haze of the USA Microwave Tank. When the game is paused, the heat effect will now pause as well.

[greptile-apps]

Greptile Summary

This PR decouples the smudge (heat-haze) time-step from the render update so that the effect correctly pauses when the game is paused (fix for issue #1694). The approach moves smudge creation/reset into ParticleSystemManager::update() (game-logic tick) and reduces the render path to a frustum-cull + draw-flag step.

Key changes:

• Smudge gains an Identifier (void*) field and a m_draw flag; SmudgeSet adds a hash_map<Identifier, Smudge*> for O(1) lookup.
• SmudgeManager / SmudgeSet get resetDraw() and findSmudge() helpers; removal signatures now null out the caller's pointer.
• ParticleSystemManager::update() fully rebuilds the smudge list each game tick; W3DParticleSystemManager::doParticles() only marks visible smudges and triggers render.
• W3DSmudgeManager::render() switches to a local offset copy (important correctness fix — prevents permanently zeroing smudge->m_offset across frames now that smudges persist).
• W3DSmudgeManager is marked final.

Concerns:

• addSmudgeToSet has no guard against duplicate identifiers; a double-insert would corrupt the map while leaving a stale node in the linked list. The current call-site is safe (always preceded by reset()), but there is no assertion to enforce this contract.
• m_offset is now randomised once per logic tick rather than once per render frame. At high frame rates the shimmer will visually step at logic-tick frequency, which may be a perceptible regression.
• findSmudge returning nullptr in the render path is silently swallowed; a debug-only log or assert would make out-of-sync states visible earlier.

Confidence Score: 4/5

• Safe to merge with one minor defensive hardening remaining in addSmudgeToSet.
• The core decoupling logic is sound and the critical render-side m_offset side-effect bug is correctly fixed with the local copy. The duplicate-identifier risk in addSmudgeToSet is not reachable with today's call-sites but lacks a contract guard. The m_offset tick-rate change and silent findSmudge miss are low-severity. No data-loss, security, or crash risk in the normal game path.
• Core/GameEngine/Source/GameClient/System/Smudge.cpp — addSmudgeToSet has no duplicate-key guard; Core/GameEngine/Source/GameClient/System/ParticleSys.cpp — m_offset randomisation frequency change.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/GameClient/Smudge.h	Adds Identifier (void*) and m_draw to Smudge; adds hash_map-backed lookup and resetDraw() to SmudgeSet/SmudgeManager; changes set/smudge removal signatures to null-out via reference. Changes are clean; #pragma once and nullptr rules respected.
Core/GameEngine/Source/GameClient/System/Smudge.cpp	Implements new addSmudgeToSet(identifier), findSmudge, resetDraw, and updated reset() (now also clears map and resets counter). No duplicate-key guard in addSmudgeToSet; see inline comment.
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp	Adds smudge creation logic to the game-logic-tick update(), decoupling it from the render path. Smudges are rebuilt on every tick. m_offset random value is now tick-rate, not frame-rate (visual trade-off). Second full particle list scan per tick is a minor perf cost.
Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DSmudge.h	Minimal change: adds final specifier to W3DSmudgeManager. Correct and safe.
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DSmudge.cpp	Render pass now skips smudges where m_draw == false, uses a local offset copy (important correctness fix since smudges now persist across frames), and sets m_smudgeCountLastFrame in-place. Logic is correct; batch-overflow path correctly handles non-drawable smudges.
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp	Render path simplified: now calls resetDraw() then marks visible smudges via findSmudge. Removes old reset()/setSmudgeCountLastFrame calls. Silent miss on findSmudge null-return is acceptable but lacks debug visibility.

Sequence Diagram

sequenceDiagram
    participant GL as Game Logic Tick<br/>(ParticleSystemManager::update)
    participant SM as SmudgeManager
    participant SS as SmudgeSet
    participant RP as Render Pass<br/>(W3DParticleSystemManager::doParticles)
    participant W3D as W3DSmudgeManager::render

    GL->>SM: reset()
    SM->>SS: reset() — clears list & map
    GL->>SM: addSmudgeSet()
    SM-->>GL: SmudgeSet*
    loop For every smudge particle
        GL->>SS: addSmudgeToSet(particle*)
        SS-->>GL: Smudge* (m_draw=false)
        GL->>GL: set pos/size/opacity/offset
    end

    Note over GL,W3D: --- Render Frame ---

    RP->>SM: resetDraw()
    SM->>SS: resetDraw() — all m_draw = false
    loop For every visible smudge particle (frustum-culled)
        RP->>SM: findSmudge(particle*)
        SM-->>RP: Smudge*
        RP->>RP: smudge->m_draw = true
    end

    RP->>W3D: render(rinfo)
    W3D->>SS: getUsedSmudgeList()
    loop Pass 1 — compute UV / count
        W3D->>W3D: skip if !m_draw
        W3D->>W3D: project verts, clamp offset (local copy)
    end
    W3D->>W3D: m_smudgeCountLastFrame = count
    loop Pass 2 — fill VB & draw
        W3D->>W3D: skip if !m_draw
        W3D->>W3D: DrawTriangles batch
    end

Loading

Comments Outside Diff (1)

1. Core/GameEngine/Source/GameClient/System/Smudge.cpp, line 150-163 (link)

   Duplicate identifier overwrites map entry but leaves stale smudge in list

   addSmudgeToSet does not check whether identifier already exists in m_usedSmudgeMap. If the same identifier is inserted twice (without a preceding reset()), the map is silently overwritten but both Smudge nodes remain in m_usedSmudgeList. The first node becomes unreachable via the map, which means:

   • findSmudge can no longer retrieve it.
   • removeSmudgeFromSet on the first node calls m_usedSmudgeMap.erase on an identifier that now maps to the second node, corrupting the map and leaking the first node.

   The current call-site (ParticleSys.cpp::update) always calls reset() before iterating, so in practice no duplicate is introduced today. However, the function makes no contract-level guarantee, and future callers could easily trigger this. Consider adding an ASSERT or early-return:

   Smudge *SmudgeSet::addSmudgeToSet(Smudge::Identifier identifier)
   {
       DEBUG_ASSERTCRASH(m_usedSmudgeMap.find(identifier) == m_usedSmudgeMap.end(),
           ("addSmudgeToSet: identifier already present – call reset() first"));
       ...
   }

Prompt To Fix All With AI

This is a comment left during a code review.
Path: Core/GameEngine/Source/GameClient/System/ParticleSys.cpp
Line: 3036

Comment:
**`m_offset` randomised at logic-tick rate, not render rate**

`m_offset` is now assigned a random value once per game-logic tick (inside `ParticleSystemManager::update()`). Previously it was assigned every render frame in `doParticles()`, so the heat-haze shimmer animated at full frame rate.

With this change, when the game runs at e.g. 60 fps but the logic ticks at 30 Hz, the shimmer will visually "step" at 30 Hz instead of updating smoothly. This is likely an acceptable trade-off for the pause fix, but it is a perceptible visual regression at high frame rates and may be worth documenting or revisiting (e.g. by keeping a per-render random offset in the render path while still using the stable `m_offset` for state persistence).

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DParticleSys.cpp
Line: 179-183

Comment:
**Silent miss when particle was never registered**

If `TheSmudgeManager->findSmudge(p)` returns `nullptr` the particle is silently skipped. This can happen legitimately (e.g. if `update()` hasn't been called yet at startup, or if the particle list is out of sync), but it can also mask real bugs where the game-logic and render paths differ (e.g. different particle list snapshots). Consider adding a debug assertion or a counter so misses are detectable during development:

```cpp
if (Smudge *smudge = TheSmudgeManager->findSmudge(p))
{
    // The particle is in view. Draw the smudge!
    smudge->m_draw = true;
}
#ifdef _DEBUG
else
{
    DEBUG_LOG(("W3DParticleSys: smudge particle %p not found in manager\n", p));
}
#endif
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: Core/GameEngine/Source/GameClient/System/Smudge.cpp
Line: 150-163

Comment:
**Duplicate identifier overwrites map entry but leaves stale smudge in list**

`addSmudgeToSet` does not check whether `identifier` already exists in `m_usedSmudgeMap`. If the same identifier is inserted twice (without a preceding `reset()`), the map is silently overwritten but both `Smudge` nodes remain in `m_usedSmudgeList`. The first node becomes unreachable via the map, which means:

- `findSmudge` can no longer retrieve it.
- `removeSmudgeFromSet` on the first node calls `m_usedSmudgeMap.erase` on an identifier that now maps to the *second* node, corrupting the map and leaking the first node.

The current call-site (`ParticleSys.cpp::update`) always calls `reset()` before iterating, so in practice no duplicate is introduced today. However, the function makes no contract-level guarantee, and future callers could easily trigger this. Consider adding an `ASSERT` or early-return:

```cpp
Smudge *SmudgeSet::addSmudgeToSet(Smudge::Identifier identifier)
{
    DEBUG_ASSERTCRASH(m_usedSmudgeMap.find(identifier) == m_usedSmudgeMap.end(),
        ("addSmudgeToSet: identifier already present – call reset() first"));
    ...
}
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (5): Last reviewed commit: "Initialize m_draw" | Re-trigger Greptile}