Skip to content

unify(commandxlat): Move AcademyStats files to Core and merge CommandXlat and related code from Zero Hour#2765

Open
xezon wants to merge 2 commits into
TheSuperHackers:mainfrom
xezon:xezon/unity-commandxlat
Open

unify(commandxlat): Move AcademyStats files to Core and merge CommandXlat and related code from Zero Hour#2765
xezon wants to merge 2 commits into
TheSuperHackers:mainfrom
xezon:xezon/unity-commandxlat

Conversation

@xezon
Copy link
Copy Markdown

@xezon xezon commented Jun 5, 2026

Merge with Rebase

This change has 2 commits.

  1. Move AcademyStats files to Core and merge related Code from Zero Hour
  2. Merge CommandXlat and related code from Zero Hour

Behavior should be unchanged in both games.

Generals gets

  • AcademyStats class
  • New KindOf flags from Zero Hour (Drone selection and Airfields required a migration strategy)
  • Command behavior related to USA Firebase
  • Move voice logic for GLA worker with shoes upgrade
  • Special Power object auto-selection for Spectre Gunship and Particle Cannon
  • Command behavior related to GLA Sabotage Building
  • Command behavior related to GLA Sneak Attach placement
  • Cursor behavior related to USA Colonel Burton and GLA Bike on cliffs
  • Cheat and Demo commands for Debug
  • Command behavior related to Double Click Attack Move with Alternative Mouse setup

TODO

  • Add commit id to pull titles
  • Test what can be tested in Generals

@xezon xezon added this to the Code foundation build up milestone Jun 5, 2026
@xezon xezon added Gen Relates to Generals ZH Relates to Zero Hour Unify Unifies code between Generals and Zero Hour labels Jun 5, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Jun 5, 2026

Greptile Summary

This PR unifies AcademyStats into Core and ports a large batch of CommandXlat behavior from Zero Hour into Generals, including Firebase garrison-attack routing, GLA Sabotage Building and Sneak Attack placement, cliff-locomotor cursor logic, double-click attack-move feedback, cheat/demo messages, and the full KINDOF_NO_SELECT / new KindOf flags from ZH.

  • AcademyStats moved to Core — both Generals and ZH CMake lists updated; Player.h gains getAcademyStats() accessors and the iterateObjects signature is now correctly const.
  • canObjectForceAttack refactored — fixes a pre-existing bug (testObj = obj instead of testObj = slave) and adds a new branch for the Firebase garrison-container case via the new getClosestRider virtual in ContainModuleInterface.
  • KindOf alignment — ~25 ZH-only KINDOF_ entries added to Generals; KINDOF_AIRFIELD guarded with #if RTS_GENERALS in both headers; AIPathfind.cpp in Core now unconditionally uses KINDOF_FS_AIRFIELD.

Confidence Score: 3/5

The bulk of the changes are correct ports from ZH, but the CONSTRUCT special-power handler in Generals CommandXlat.cpp has switch-case labels that can never match, making that entire DO_COMMAND path dead code.

The switch-case mismatch in the GUI_COMMAND_SPECIAL_POWER_CONSTRUCT handler means special-power-construct commands (e.g. Sneak Attack placement) will silently never issue via that branch in Generals. The rest of the port looks correct and behaviorally equivalent to ZH.

Generals/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp (switch-case mismatch at lines 1797-1809 and missing DESTROY_MESSAGE in MSG_CHEAT_ADD_CASH); the identical switch-case issue is also present in GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp line 1799.

Important Files Changed

Filename Overview
Generals/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp Large port from Zero Hour adding Firebase attack logic, Sneak Attack commands, cliff locomotor cursor, cheat/demo messages, and double-click attack move; contains a switch-case mismatch in the CONSTRUCT special power handler that makes it dead code, plus a missing DESTROY_MESSAGE in MSG_CHEAT_ADD_CASH.
Generals/Code/GameEngine/Include/GameLogic/Module/ContainModule.h Adds pure-virtual getClosestRider to ContainModuleInterface; all concrete subclasses go through OpenContain which provides the implementation, so the hierarchy is complete.
Core/GameEngine/Include/Common/AcademyStats.h Moved from GeneralsMD to Core unchanged; uses #pragma once correctly; copyright header still references Generals Zero Hour(tm) but the Core directory has no specific convention rule.
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp Adds getClosestRider implementation iterating the contain list by squared distance; straightforward logic with safe uninitialized-distance handling via the !closest short-circuit.
Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp Adds double-click attack-move feedback via a frame-countdown timer and a stashed guard-hint position; timer decrements below zero on subsequent frames but the > 0 guard prevents visible effects.
Generals/Code/GameEngine/Include/Common/KindOf.h Adds ~25 KindOf flags from Zero Hour and wraps KINDOF_AIRFIELD in #if RTS_GENERALS to keep enum values aligned between Generals and ZH builds.
Core/GameEngine/Source/GameLogic/AI/AIPathfind.cpp Removes #if RTS_GENERALS guard since both builds now share KINDOF_FS_AIRFIELD; correct since Generals KindOf.h also gains KINDOF_FS_AIRFIELD in this PR.
GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp Minor cleanup: removes duplicate comment banners, adds null-check for workerShoeTemplate, and adds #if guard around INVALID_ANGLE append for retail-compatible networking.

Sequence Diagram

sequenceDiagram
    participant Player
    participant CommandTranslator
    participant InGameUI
    participant ContainModule
    participant MessageStream

    Note over Player,MessageStream: Double-click attack-move (new feature)
    Player->>CommandTranslator: MSG_MOUSE_LEFT/RIGHT_DOUBLE_CLICK
    CommandTranslator->>MessageStream: MSG_DO_GUARD_POSITION
    CommandTranslator->>InGameUI: triggerDoubleClickAttackMoveGuardHint()
    InGameUI->>InGameUI: "m_duringDoubleClickAttackMoveGuardHintTimer = 11"

    Note over Player,MessageStream: Firebase garrison attack (new routing)
    Player->>CommandTranslator: canObjectForceAttack(firebase, victim)
    alt "SPAWNS_ARE_THE_WEAPONS AND result==POSSIBLE"
        CommandTranslator->>ContainModule: "getClosestRider(victim->pos)"
        ContainModule-->>CommandTranslator: rider
        CommandTranslator->>CommandTranslator: rider.getAbleToAttackSpecificObject()
    else "SPAWNS_ARE_THE_WEAPONS AND result!=POSSIBLE"
        CommandTranslator->>CommandTranslator: spawnInterface.getClosestSlave(pos)
    end

    Note over Player,MessageStream: Shortcut special power auto-select (new feature)
    Player->>CommandTranslator: GUI_COMMAND_SPECIAL_POWER_FROM_SHORTCUT
    CommandTranslator->>CommandTranslator: findMostReadyShortcutSpecialPowerOfType()
    CommandTranslator->>InGameUI: deselectAllDrawables()
    CommandTranslator->>MessageStream: MSG_CREATE_SELECTED_GROUP_NO_SOUND(unit)
Loading
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
Generals/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp:1797-1809
**Unreachable switch cases in CONSTRUCT handler**

The outer `else if` only enters this block when `commandType` is `GUI_COMMAND_SPECIAL_POWER_CONSTRUCT` or `GUI_COMMAND_SPECIAL_POWER_CONSTRUCT_FROM_SHORTCUT`, but the inner `switch` tests the same value against `GUI_COMMAND_SPECIAL_POWER_FROM_SHORTCUT` and `GUI_COMMAND_SPECIAL_POWER` — entirely different enum values. No case will ever match, so `issueSpecialPowerCommand` is never called and the command silently does nothing. The case labels should be `GUI_COMMAND_SPECIAL_POWER_CONSTRUCT_FROM_SHORTCUT` and `GUI_COMMAND_SPECIAL_POWER_CONSTRUCT` respectively. Note: the identical pattern exists in `GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp` (line 1799), so the same bug is present in the Zero Hour build as well.

### Issue 2 of 3
Generals/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp:3883-3886
Newly added comment references July 2003, which is prior to the current year (2026). Per project convention, newly added code comments should not reference dates in the past.

```suggestion
			// Added this code to deselect build placement mode when right clicked. This fixes
			// a bug where you couldn't cancel the sneak attack mode via right click. This only happened when you
			// didn't have anything selected which is possible via the shortcut bar. Normally, it would get deselected
			// via the deselect drawable code.
```

### Issue 3 of 3
Generals/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp:3607-3617
`MSG_CHEAT_ADD_CASH` is the only cheat case in this block that does not set `disp = DESTROY_MESSAGE` after acting. Without it the message continues propagating through the pipeline when not in multiplayer; all neighboring cases (`MSG_CHEAT_INSTANT_BUILD`, `MSG_CHEAT_GIVE_ALL_SCIENCES`, etc.) consistently destroy the message.

```suggestion
    case GameMessage::MSG_CHEAT_ADD_CASH:
		{
			if ( !TheGameLogic->isInMultiplayerGame() )
			{
				Player *localPlayer = ThePlayerList->getLocalPlayer();
				Money *money = localPlayer->getMoney();
				money->deposit( 10000 );
				TheInGameUI->messageNoFormat( TheGameText->FETCH_OR_SUBSTITUTE("GUI:DebugAddCash", L"Add Cash") );
				disp = DESTROY_MESSAGE;
			}
			break;
		}
```

Reviews (1): Last reviewed commit: "unify(commandxlat): Merge CommandXlat an..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Gen Relates to Generals Unify Unifies code between Generals and Zero Hour ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant