Skip to content

fix(actionmanager): Improve validation of Sneak Attack placement legality#2533

Merged
xezon merged 6 commits intoTheSuperHackers:mainfrom
stephanmeesters:fix/sneak-attack-validate-placement
Apr 6, 2026
Merged

fix(actionmanager): Improve validation of Sneak Attack placement legality#2533
xezon merged 6 commits intoTheSuperHackers:mainfrom
stephanmeesters:fix/sneak-attack-validate-placement

Conversation

@stephanmeesters
Copy link
Copy Markdown

@stephanmeesters stephanmeesters commented Apr 4, 2026

This PR fixes potential cheating in the placement of a Sneak Attack, where the validity of the building placement was only checked locally. This means a Sneak Attack could be placed in the fog, on top of any other building, steep mountain slope, water etc. A second check is placed that runs on all clients and mirrors the first check at InGameUI:1688.

Todo

  • Replicate in Generals (first commit only)

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 4, 2026

Greptile Summary

Fixes a multiplayer cheat vector where Sneak Attack building placement was validated only client-side. The new server-side check runs on all clients and mirrors the existing InGameUI:1688 placement validation using isLocationLegalToBuild with identical flags, gated behind #if RETAIL_COMPATIBLE_CRC for CRC compatibility. The builderObject parameter across BuildAssistant is also correctly tightened to const Object*.

Confidence Score: 5/5

Safe to merge; the anti-cheat logic correctly mirrors the existing InGameUI placement check with no functional regressions.

All findings are P2 style/cleanup. The core logic is correct and directly mirrors the reference check at InGameUI:1688 using identical flags and parameters. The const-correctness changes are appropriate. No P0 or P1 issues found.

ActionManager.cpp — minor bib cleanup omission after isLocationLegalToBuild call

Important Files Changed

Filename Overview
GeneralsMD/Code/GameEngine/Source/Common/RTS/ActionManager.cpp Core anti-cheat fix: SPECIAL_SNEAK_ATTACK moved from shroud-only check to full isLocationLegalToBuild validation mirroring InGameUI:1688; bib cleanup side effect not addressed
GeneralsMD/Code/GameEngine/Include/Common/BuildAssistant.h Const-correctness: builderObject parameter tightened from Object* to const Object*
GeneralsMD/Code/GameEngine/Source/Common/System/BuildAssistant.cpp Const propagation: builderObject and AIUpdateInterface* now const-qualified
Generals/Code/GameEngine/Include/Common/BuildAssistant.h Same const-correctness fix applied to vanilla Generals header
Generals/Code/GameEngine/Source/Common/System/BuildAssistant.cpp Same const-correctness propagation applied to vanilla Generals BuildAssistant

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[canDoSpecialPowerAtLocation] --> B{mod != nullptr?}
    B -- No --> Z[return false]
    B -- Yes --> C[Switch 1: Terrain check\ne.g. paradrop on water]
    C --> D[Switch 2: Shroud check\nfor damaging powers]
    D -- other powers --> Y[return shroud/map result]
    D -- SNEAK_ATTACK removed --> E[Switch 3: Building placement NEW]
    E --> F{RETAIL_COMPATIBLE_CRC?}
    F -- Yes --> G[Shroud-only check\nold behavior preserved]
    F -- No --> H[getReferenceThingTemplate]
    H -- null --> I[return FALSE]
    H -- valid --> J[isLocationLegalToBuild\nUSE_QUICK_PATHFIND\nTERRAIN_RESTRICTIONS\nCLEAR_PATH\nNO_OBJECT_OVERLAP\nSHROUD_REVEALED\nIGNORE_STEALTHED\nSame flags as InGameUI:1688]
    J --> K{== LBC_OK?}
    K -- Yes --> L[return true]
    K -- No --> M[return false]
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/Common/RTS/ActionManager.cpp
Line: 1605-1613

Comment:
**Missing `removeAllBibs()` cleanup after `isLocationLegalToBuild`**

`isLocationLegalToBuild` adds bib visual feedback as a documented side effect — `AIPlayer.cpp:694` notes: `// isLocationLegalToBuild adds bib feedback, turn it off. jba.` Every other call site in `AIPlayer` and `AISkirmishPlayer` follows up with `TheTerrainVisual->removeAllBibs()`. Since `canDoSpecialPowerAtLocation` runs on all clients (per the PR description), stray bibs will briefly appear on every client's screen each time a Sneak Attack placement is validated.

```suggestion
				Bool const result = TheBuildAssistant->isLocationLegalToBuild(
					loc, referenceThing, angle,
					BuildAssistant::USE_QUICK_PATHFIND |
					BuildAssistant::TERRAIN_RESTRICTIONS |
					BuildAssistant::CLEAR_PATH |
					BuildAssistant::NO_OBJECT_OVERLAP |
					BuildAssistant::SHROUD_REVEALED |
					BuildAssistant::IGNORE_STEALTHED,
					obj, nullptr) == LBC_OK;
				TheTerrainVisual->removeAllBibs();
				return result;
```

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

Reviews (4): Last reviewed commit: "Replicate to Generals" | Re-trigger Greptile

Copy link
Copy Markdown

@xezon xezon left a comment

Choose a reason for hiding this comment

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

Makes sense.

@stephanmeesters stephanmeesters force-pushed the fix/sneak-attack-validate-placement branch from b6b2dfa to 8e5fd24 Compare April 6, 2026 09:47
@xezon xezon changed the title fix(logic): Improve validation of building placement for Sneak Attack fix(actionmanager): Improve validation of Sneak Attack placement legality Apr 6, 2026
@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Fix Is fixing something, but is not user facing ZH Relates to Zero Hour labels Apr 6, 2026
@xezon xezon merged commit 5e041b4 into TheSuperHackers:main Apr 6, 2026
23 checks passed
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 ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants