refactor(gamelogic): Break switch cases of GameLogic::logicMessageDispatcher into separate functions by xezon · Pull Request #2702 · TheSuperHackers/GeneralsGameCode

xezon · 2026-05-12T20:10:24Z

This change breaks all switch cases of GameLogic::logicMessageDispatcher into separate functions for ease of readability and maintainability.

The logic is unchanged.

TODO

Replicate in Generals

greptile-apps · 2026-05-12T20:13:43Z

Greptile Summary

This PR extracts all switch cases of GameLogic::logicMessageDispatcher into individual on* member functions for readability and maintainability. The function implementations are in GameLogicDispatch.cpp and the declarations are added to both Generals/GameLogic.h and GeneralsMD/GameLogic.h. The PR also moves #include "GameLogic/AI.h" from the .cpp into both headers since AIGroupPtr is now used in function signatures, and introduces a getMessagePlayer static helper to reduce duplication.

~50 extracted functions each correspond to one message type handled by the dispatcher; logic is preserved verbatim with break replaced by return false/return true as appropriate.
Issues flagged in a previous review round — missing msgPlayer declarations inside #if !RETAIL_COMPATIBLE_CRC blocks, missing AIGroupPtr& parameter in ALLOW_SURRENDER functions, and the Generals/GameLogic.h signature mismatch — are all addressed in this revision.

Confidence Score: 5/5

Safe to merge — this is a pure mechanical extraction refactor with no changes to game logic.

Every extracted function is a direct transcription of its corresponding switch-case body. The previously flagged compile errors (missing msgPlayer declarations in non-retail CRC builds, missing AIGroupPtr parameter in ALLOW_SURRENDER functions, Generals header signature mismatch) are all resolved in this revision. The onLogicCrc early-exit path uses return false instead of break, which is semantically equivalent since the switch dispatcher always executes its own break after the call. Include dependencies are correctly updated in both header files.

No files require special attention.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp	~1600 lines of switch-case body moved into ~50 extracted member functions; logic is faithful to the original. Previously flagged issues (missing msgPlayer, missing currentlySelectedGroup param in ALLOW_SURRENDER functions, onLogicCrc early-exit) are all resolved. The static getMessagePlayer helper is correctly used.
Generals/Code/GameEngine/Include/GameLogic/GameLogic.h	Adds declarations for all extracted on* methods including the three ALLOW_SURRENDER functions with the correct AIGroupPtr& parameter; adds #include "GameLogic/AI.h" required for AIGroupPtr. Matches the GeneralsMD header.
GeneralsMD/Code/GameEngine/Include/GameLogic/GameLogic.h	Mirrors the Generals header: adds declarations for all extracted on* methods with correct signatures and adds #include "GameLogic/AI.h". Unchanged from the correct version used as the reference in the prior review.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["logicMessageDispatcher(msg)"] --> B["getMessagePlayer(msg)\nvalidate msgPlayer != null"]
    B --> C["AIGroup selection setup"]
    C --> D["switch(msgType)"]
    D --> E1["MSG_NEW_GAME → onNewGame(msg)"]
    D --> E2["MSG_CLEAR_GAME_DATA → onClearGameData(msg, grp)"]
    D --> E3["MSG_SET_RALLY_POINT → onSetRallyPoint(msg)"]
    D --> E4["MSG_DO_SPECIAL_POWER → onDoSpecialPower(msg, grp)"]
    D --> E5["MSG_DO_SPECIAL_POWER_AT_LOCATION → onDoSpecialPowerAtLocation(msg, grp)"]
    D --> E6["MSG_DO_SPECIAL_POWER_AT_OBJECT → onDoSpecialPowerAtObject(msg, grp)"]
    D --> E7["MSG_DO_SPECIAL_POWER_OVERRIDE_DESTINATION → onDoSpecialPowerOverrideDestination(msg, grp)"]
    D --> E8["MSG_ENTER / MSG_EXIT → onEnter / onExit(msg, grp)"]
    D --> E9["MSG_LOGIC_CRC → onLogicCrc(msg)"]
    D --> E10["MSG_DOZER_CONSTRUCT → onDozerConstruct(msg, grp)"]
    D --> E11["~40 more cases..."]
    E1 --> F["return bool\n(ignored at call site)"]
    E9 --> F
    F --> G["Post-switch:\nRETAIL_COMPATIBLE_AIGROUP\ngroup cleanup"]

_{Reviews (6): Last reviewed commit: "Fix line spacing" | Re-trigger Greptile}

xezon · 2026-05-12T20:13:49Z

Generals fails to compile until replicated.

Caball009 · 2026-05-13T18:54:55Z

Do we need the braces per case? It makes the function a lot longer.

xezon · 2026-05-13T19:41:43Z

Do we need the braces per case? It makes the function a lot longer.

Is not strictly necessary. Braces are only needed if a variable needs declaration in the top switch case scope. Do you want it removed?

Caball009 · 2026-05-13T19:43:13Z

I don't have a strong opinion on it either way, but I lean toward removing them to make the function shorter.

xezon · 2026-05-22T07:24:05Z

Needs rebase.

…patcher into separate functions

xezon · 2026-05-22T19:20:03Z

Rebased without conflicts.

Caball009 · 2026-05-24T20:49:22Z

+	return true;
+}
+
+bool GameLogic::onDoWeapon(GameMessage *msg, AIGroupPtr &currentlySelectedGroup)


Why is currentlySelectedGroup passed as AIGroup*& everywhere?

Because some of them do actually release the AI group and set the pointer to null. To keep the code simple, I passed it as reference to all of them. It reduces the likelyhood of mistakes this way, because we then do not need to verify that all the correct functions get it as reference.

Caball009 · 2026-05-24T21:13:55Z

+	return true;
+}
+
+bool GameLogic::onInternetHack(GameMessage *msg, AIGroupPtr &currentlySelectedGroup)


GameMessage *msg is unused.

Ok but that is fine. We can keep msg anyway to keep it consistent with all the others. Plus there is a commented code line that uses msg.

Many times in this class.

Caball009 · 2026-05-24T21:37:31Z

+	return true;
+}
+
+bool GameLogic::onDoCheer(GameMessage *msg, AIGroupPtr &currentlySelectedGroup)


GameMessage *msg is unused.

Caball009 · 2026-05-24T21:38:02Z

+	return true;
+}
+
+bool GameLogic::onEvacuate(GameMessage *msg, AIGroupPtr &currentlySelectedGroup)


GameMessage *msg is unused.

Caball009 · 2026-05-24T21:38:15Z

+	return true;
+}
+
+bool GameLogic::onExecuteRailedTransport(GameMessage *msg, AIGroupPtr &currentlySelectedGroup)


GameMessage *msg is unused.

Caball009 · 2026-05-24T21:38:47Z

+	return true;
+}
+
+bool GameLogic::onToggleOvercharge(GameMessage *msg, AIGroupPtr &currentlySelectedGroup)


GameMessage *msg is unused.

Caball009 · 2026-05-24T21:47:02Z

+	return true;
+}
+
+bool GameLogic::onReturnToPrison(GameMessage *msg, AIGroupPtr &currentlySelectedGroup)


GameMessage *msg is unused.

Caball009

Incomplete review.

There's a bunch of unused function parameters. Could remove or mark with MAYBE_UNUSED.
Player *msgPlayer = getMessagePlayer(msg) can be made const in some functions.
currentlySelectedGroup can be passed by pointer instead of pointer ref, I think.

Caball009 · 2026-05-24T21:55:18Z

 		case GameMessage::MSG_SELECTED_GROUP_COMMAND:
 		{
-
 			break;
-
 		}


This case could be removed technically.

I will keep it because the original had it too.

xezon · 2026-05-25T10:46:04Z

There's a bunch of unused function parameters. Could remove or mark with MAYBE_UNUSED.

I would like to keep passing all the message pointers because it is the common expectation that the message data is needed to properly act on a message - when it has message arguments.

Player *msgPlayer = getMessagePlayer(msg) can be made const in some functions.

I checked and would like to keep it as is, because some functions do use it in const context but the constness is not consistently applied to all getter methods across the code base. I think that would need a polishing pass first.

currentlySelectedGroup can be passed by pointer instead of pointer ref, I think.

I looked over it one more time and it strictly needs passing as reference only to onClearGameData. We could remove it from the others but that then would mean for the non retail compatible code it would Add_Ref and Release_Ref on function calls, because of RefCountPtr<AIGroup>. I think we can keep passing by reference here. It is ok.

xezon added Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Refactor Edits the code with insignificant behavior changes, is never user facing labels May 12, 2026

xezon mentioned this pull request May 12, 2026

tweak(gamemessage): Reduce number of MSG_DESTROY_SELECTED_GROUP messages #2668

Open

1 task

greptile-apps Bot reviewed May 12, 2026

View reviewed changes

Comment thread Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp

greptile-apps Bot reviewed May 12, 2026

View reviewed changes

Comment thread Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp

Comment thread Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp

greptile-apps Bot reviewed May 13, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Include/GameLogic/GameLogic.h

xezon force-pushed the xezon/refactor-gamelogicdispatch branch from 91c7c5d to 5d14b30 Compare May 13, 2026 19:41

xezon added 7 commits May 22, 2026 21:19

refactor(gamelogic): Break switch cases of GameLogic::logicMessageDis…

37525db

…patcher into separate functions

Fix RETAIL_COMPATIBLE_CRC

6515f0e

Pass currentlySelectedGroup to Surrender functions

dc69b77

Fix 2 wrong returns

bdf1e88

Move createOptimizedTree function decl down

6d0075c

Replicate in Generals

ad43a66

Fix compile error

883093b

xezon force-pushed the xezon/refactor-gamelogicdispatch branch from 5d14b30 to 883093b Compare May 22, 2026 19:19

Caball009 reviewed May 24, 2026

View reviewed changes

xezon added 3 commits May 25, 2026 12:21

Fix indentation

5fe94f5

Remove superfluous AIGroupPtr argument from onEnableRetaliationMode

1eef718

Fix line spacing

3539719

Conversation

xezon commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Uh oh!

greptile-apps Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

xezon commented May 12, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Caball009 commented May 13, 2026

Uh oh!

xezon commented May 13, 2026

Uh oh!

Caball009 commented May 13, 2026

Uh oh!

xezon commented May 22, 2026

Uh oh!

xezon commented May 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xezon May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Caball009 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Caball009 May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xezon commented May 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xezon commented May 12, 2026 •

edited

Loading

greptile-apps Bot commented May 12, 2026 •

edited

Loading

xezon May 25, 2026 •

edited

Loading

Caball009 left a comment •

edited

Loading

Caball009 May 24, 2026 •

edited

Loading