bugfix: Restore Generals retail compatibility after change to ParticleUplinkCannonUpdate::update reordered floating-point operations

[Caball009]

• Closes Changes to Generals ParticleUplinkCannonUpdate::update broke retail compatibility #3364
• Follow up for [GEN][ZH] Separate ParticleUplinkCannonUpdate's damage radius calculation from LaserUpdate's client update #874

It appears that the VC6 compiler is extremely sensitive to changing around floating point operations in ParticleUplinkCannonUpdate::update.

Currently the instructions for Real targetDistance = buildingToInitialTargetVector.length() are equal to sqrt(z*z + x*x + y*y) left to right. The Steam binary does sqrt(x*x + y*y + z*z) left to right, and the change in this PR restores the exact code that the Steam binary uses. See issue for code generation comparisons.

The reason I used this solution is because I found that with other approaches VC6 would not actually generate the correct code, or it would change too much.

I removed the old comment for isFiring because it's no longer accurate (neither for GEN or ZH). The compiler currently generates the same code for both versions. It's not clear to me if the comment refers to the exact same problem that this PR aims to address.

Zero Hour already generates the correct code here, so I put this behind the RTS_GENERALS macro.

TODO:

• Add Generals replay to replay repository to catch this if it happens again.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 70cc8da0-652a-4d8e-a98e-c310fde44818

📥 Commits

Reviewing files that changed from the base of the PR and between 33c1104 and f777a0b.

📒 Files selected for processing (2)

• Generals/Code/GameEngine/Source/GameLogic/Object/Update/ParticleUplinkCannonUpdate.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/ParticleUplinkCannonUpdate.cpp

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

Walkthrough

Both game variants now use an explicit floating-point operation order for automated target distance in retail-compatible CRC builds. Other builds retain Coord3D::length(). GeneralsMD also removes a comment above the unchanged firing condition.

Changes

Particle uplink cannon update

Layer / File(s)	Summary
Target distance compatibility adjustment Generals/Code/GameEngine/Source/GameLogic/Object/Update/ParticleUplinkCannonUpdate.cpp, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/ParticleUplinkCannonUpdate.cpp	Both variants calculate distance as sqrt((x*x + y*y) + z*z) when RTS_GENERALS and RETAIL_COMPATIBLE_CRC are enabled. Other builds retain Coord3D::length(). GeneralsMD removes a comment above the unchanged isFiring condition.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: xezon

Merge Risk: ⚪ Minimal · up to f777a

This change aims to restore Generals replay compatibility by changing the distance calculation’s operation order. The available evidence does not confirm whether VC6 emits that order, but it also does not establish a remaining replay mismatch or another actionable merge-blocking issue.

Architecture Summary

Architecture risk: 🔵 Low · up to f777a

The change affects 2 systems.

Changed systems: Generals, GeneralsMD

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — Generals (service) was modified; 1 changed file maps to changed impact.
• observed — GeneralsMD (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in Generals/Code/GameEngine/Source/GameLogic/Object/Update/ParticleUplinkCannonUpdate.cpp: For Generals retail-compatible CRC builds, replaces Coord3D::length() with an explicit square-root calculation ordered as (x*x + y*y) + z*z; other builds retain the original length() call. The comments describing compiler sensitivity and operation order are added.
• observed — Modified behavior in GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/ParticleUplinkCannonUpdate.cpp: Removes the retail-compatibility comment above isFiring; the firing condition is unchanged.
• observed — Modified behavior in GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/ParticleUplinkCannonUpdate.cpp: When both RTS_GENERALS and RETAIL_COMPATIBLE_CRC are enabled, replaces Coord3D::length() with an explicit distance calculation ordered as (x*x + y*y) + z*z; other configurations still call length().

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Issue #3364 requires retail-compatible left-to-right distance calculation in ParticleUplinkCannonUpdate::update. Both affected source variants now use sqrt(sqr(x) + sqr(y) + sqr(z)) under `RTS_GEN…
Out of Scope Changes check	✅ Passed	The changes are limited to the two corresponding ParticleUplinkCannonUpdate.cpp implementations. The removed isFiring comment is an outdated retail-compatibility comment in the same function. The …
Title check	✅ Passed	The title clearly identifies the main change: restoring Generals retail compatibility by correcting floating-point operation order in ParticleUplinkCannonUpdate::update.
Description check	✅ Passed	The description directly explains the compatibility issue, the affected function, the floating-point operation order, the build guard, and the related issue and pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Adjusts floating-point calculation order in weapon targeting logic.

The PR appears safe to merge; no actionable new failure was established.

Summary

The PR replaces the Generals retail-compatible particle-cannon distance calculation with an explicit expression and removes the earlier dummy-function workaround. Zero Hour retains its existing Coord3D::length() path.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Particle cannon swath update] --> B{Generals and retail-compatible CRC?}
  B -- Yes --> C[Explicit x² + y² + z² distance]
  B -- No --> D[Coord3D::length]
  C --> E[Beam target position]
  D --> E

Loading

_{Reviews (2) · Last reviewed commit: "bugfix: Restore Generals retail compatib..."}