fix(logic): Improve validation of MSG_DO_SPECIAL_POWER and variants in GameLogicDispatch

[stephanmeesters]

This PR improves the stability of network games by validating the origin of source objects in MSG_DO_SPECIAL_POWER and related MSG variants in GameLogicDispatch. Tested on ~1k replays.

Todo

• Replicate in Generals

[greptile-apps]

Greptile Summary

This PR adds source-object ownership validation to four MSG_DO_SPECIAL_POWER* message handlers in GameLogicDispatch, rejecting any message where the referenced source object is not controlled by the player who sent the message. The change improves network game stability by preventing cheating or desyncs caused by malformed/malicious messages referencing objects a player doesn't own.

• Validation is applied consistently across all four variants: MSG_DO_SPECIAL_POWER, MSG_DO_SPECIAL_POWER_AT_LOCATION, MSG_DO_SPECIAL_POWER_AT_OBJECT, and MSG_DO_SPECIAL_POWER_OVERRIDE_DESTINATION.
• The pattern (DEBUG_CRASH + break) is appropriate: the crash fires in debug builds to surface problems early, while in release builds DEBUG_CRASH is a no-op and the break silently rejects the invalid message.
• The validation only fires when a specific sourceID is present (source != nullptr); the fallback path that uses currentlySelectedGroup is intentionally skipped when an explicit but invalid source was provided.
• Minor: three of the four new blocks leave trailing whitespace on the blank separator line after the closing } (AT_LOCATION, AT_OBJECT, OVERRIDE_DESTINATION).

Confidence Score: 4/5

• Safe to merge — the validation logic is sound, consistent with existing patterns in the file, and the release-build path correctly silences DEBUG_CRASH before breaking out.
• The change is targeted and well-tested (~1k replays), introduces no new state, follows existing ownership-check patterns (lines 1058, 1373, 1440), and only affects the path where a specific source object was explicitly provided. The one-point deduction is for the trailing whitespace nit present in three of the four hunks.
• No files require special attention.

Important Files Changed

Filename	Overview
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp	Adds ownership validation (player vs. source object) for MSG_DO_SPECIAL_POWER and its three variants; change is logically sound and consistent with existing validation patterns elsewhere in the same function, with only minor trailing-whitespace nits in three of the four hunks.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[logicMessageDispatcher receives MSG_DO_SPECIAL_POWER variant] --> B[Extract sourceID from message]
    B --> C{source = findObjectByID sourceID}
    C -- "source == nullptr" --> D[Use currentlySelectedGroup fallback]
    C -- "source != nullptr" --> E{source->getControllingPlayer\n== thisPlayer?}
    E -- "No (invalid owner)" --> F[DEBUG_CRASH in debug builds]
    F --> G[break: silently discard message in release]
    E -- "Yes (valid owner)" --> H[Create AIGroup with source]
    H --> I[Execute groupDoSpecialPower variant]
    I --> J[Cleanup AIGroup]
    D --> K{currentlySelectedGroup != nullptr?}
    K -- "Yes" --> L[Execute groupDoSpecialPower variant on selection]
    K -- "No" --> M[No-op]

Loading

_{Last reviewed commit: 2087cec}

[greptile-apps]

_{1 file reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

[Caball009]

You can use thisPlayer: if ( source->getControllingPlayer() != thisPlayer ), multiple times.

[stephanmeesters]

Good one. There are a lot of instances of ThePlayerList->getNthPlayer(msg->getPlayerIndex()) in this class, some null checked, others not

[Caball009]

Perhaps the other places that do null checks should be replaced with a single check way up in logicMessageDispatcher, but that's for another PR.

[Skyaero42]

Does this resolve an actual bug/crash or is it solving a potential issue in the future?