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 refactors GameLogic::logicMessageDispatcher by extracting each switch case into a dedicated bool-returning member function (e.g., onNewGame, onSetRallyPoint, onDoSpecialPower, …). A shared getMessagePlayer static helper is also extracted. Both Generals/ and GeneralsMD/ headers receive the new method declarations and gain an #include "GameLogic/AI.h" (moved from the .cpp) to satisfy the AIGroupPtr & parameter type.

All 50+ cases are delegated to dedicated on* methods; post-switch cleanup in the dispatcher runs correctly in all cases because each call site still executes a break after the function returns.
onClearGameData correctly receives and nulls currentlySelectedGroup through a reference, preserving the original semantics for the post-switch group-cleanup code.
Functions that need msgPlayer only inside a preprocessor-guarded block (e.g., onSetRallyPoint, onDoSpecialPower*) declare it locally within that block, avoiding an unnecessary call to getMessagePlayer in other build configurations.

Confidence Score: 5/5

Safe to merge — the refactoring is a pure mechanical extraction with no behavioral changes identified.

Every extracted handler is a faithful translation of its original switch case. Post-switch cleanup in the dispatcher runs correctly for all handlers because each call site still reaches its break. The onClearGameData reference-parameter pattern correctly propagates the nullptr assignment back to the dispatcher. All preprocessor-guarded msgPlayer uses are declared within their respective #if blocks. Both header files have matching, correct signatures.

No files require special attention.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp	All switch cases extracted into dedicated on* member functions; logic preserved; getMessagePlayer helper introduced; include AI.h correctly removed (moved to headers). No behavioral differences found.
Generals/Code/GameEngine/Include/GameLogic/GameLogic.h	New on* method declarations added with correct signatures; include AI.h added for AIGroupPtr; ALLOW_SURRENDER overloads include the AIGroupPtr currentlySelectedGroup parameter, matching the implementation.
GeneralsMD/Code/GameEngine/Include/GameLogic/GameLogic.h	Identical set of on* declarations as the Generals header; signatures match the implementation including ALLOW_SURRENDER overloads.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[logicMessageDispatcher] --> B{switch msgType}
    B -->|MSG_NEW_GAME| C[onNewGame]
    B -->|MSG_CLEAR_GAME_DATA| D[onClearGameData]
    B -->|MSG_SET_RALLY_POINT| E[onSetRallyPoint]
    B -->|MSG_DO_SPECIAL_POWER| F[onDoSpecialPower variants]
    B -->|MSG_LOGIC_CRC| H[onLogicCrc]
    B -->|MSG_EXIT| I[onExit]
    B -->|other cases| J[on* handlers]
    C --> K[break to post-switch cleanup]
    D --> K
    E --> K
    F --> K
    H --> K
    I --> K
    J --> K
    K --> L[AIGroup cleanup and Replay camera sync]

_{Reviews (5): Last reviewed commit: "Fix compile error" | 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?

Caball009 · 2026-05-24T20:56:59Z

+	Player *msgPlayer = getMessagePlayer(msg);
+
+#if RETAIL_COMPATIBLE_CRC
+	(void)msgPlayer;


FWIW we do have MAYBE_UNUSED now.

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

+{
+	Object* obj = findObjectByID( msg->getArgument( 0 )->objectID );
+	if (!obj)
+			return false;


Indentation; 2 times in this function.

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

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


currentlySelectedGroup is unused.

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

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


GameMessage *msg is unused.

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.

Caball009 · 2026-05-24T22:07:30Z

+
+		TheRecorder->handleCRCMessage(newCRC, msgPlayer->getPlayerIndex(), (msg->getArgument(1)->boolean));
+	}
+	return true;


Empty line before return like all other functions.

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

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!

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!

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!

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

Caball009 left a comment •

edited

Loading

Caball009 May 24, 2026 •

edited

Loading