refactor(gamelogic): Split 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 ~50 switch cases from GameLogic::logicMessageDispatcher into individual bool on*() member functions for readability and maintainability, with a matching TODO replicated in the Generals target. No logic changes are intended.

The GameLogicDispatch.cpp dispatch body is collapsed to one-liner calls per case; all handler implementations are appended at the end of the file.
Both Generals/ and GeneralsMD/ GameLogic.h headers receive the full set of private method declarations and gain a #include \"GameLogic/AI.h\" (moved from the .cpp) to satisfy the AIGroupPtr reference parameters.

Confidence Score: 5/5

Pure refactor with no logic changes; all extraction is mechanically correct and matches the original switch behavior.

Every issue raised in prior review threads has been addressed: msgPlayer is now declared inside the #if !RETAIL_COMPATIBLE_CRC guards in the special-power handlers; the ALLOW_SURRENDER functions correctly receive AIGroupPtr & currentlySelectedGroup; both header files have the correct ALLOW_SURRENDER signatures; and onLogicCrc preserves the break that exits the slot-finding loop while using return false only in place of the switch-case break, so CRC validation is not silenced. No new functional differences were introduced.

No files require special attention.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp	Large refactor: switch cases replaced with on*() calls; all handler implementations appended at end of file. break→return false conversions and msgPlayer/currentlySelectedGroup parameter threading are all correct in this revision.
Generals/Code/GameEngine/Include/GameLogic/GameLogic.h	Adds private on*() declarations and moves #include "GameLogic/AI.h" here to satisfy AIGroupPtr parameters. ALLOW_SURRENDER signatures correctly carry the AIGroupPtr & parameter.
GeneralsMD/Code/GameEngine/Include/GameLogic/GameLogic.h	Mirror of the Generals header change — identical on*() declarations and AI.h include added.

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 / onDoSpecialPowerAtLocation / onDoSpecialPowerAtObject / onDoSpecialPowerOverrideDestination]
    B -->|Movement msgs| G[onDoMoveto / onDoAttackmoveto / onAddWaypoint / ...]
    B -->|Production msgs| H[onQueueUnitCreate / onCancelUnitCreate / onDozerConstruct / ...]
    B -->|MSG_LOGIC_CRC| I[onLogicCrc]
    B -->|ALLOW_SURRENDER msgs| J[onDoSurrender / onPickUpPrisoner / onReturnToPrison]
    B -->|...more cases| K[30+ more on* functions]
    I -->|slotIndex found and connected| L[m_shouldValidateCRCs = TRUE / m_cachedCRCs update]
    I -->|slot not found or disconnected| M[return false]
    I -->|playback mode| N[TheRecorder handleCRCMessage]

_{Reviews (7): Last reviewed commit: "Add MAYBE_UNUSED to GameMessage args" | 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.

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

Rebased without conflicts.

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.

Caball009 · 2026-05-31T18:27:35Z

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.

They ought to be marked as unused, though, otherwise this PR is going to add a bunch of new warnings for it.

xezon · 2026-06-01T20:44:37Z

They ought to be marked as unused, though, otherwise this PR is going to add a bunch of new warnings for it.

I think unused function arguments trigger no warning.

I did a recompile in VS2022 and there is no warning for this file.

43>GameLogicDispatch.cpp
43>RankInfo.cpp

Caball009 · 2026-06-01T21:02:36Z

Perhaps it only shows up when compiling with clang-cl and / or using clang-tidy.

Edit: -Wunused-variable / -Wunused-but-set-variable

xezon · 2026-06-03T18:57:33Z

Do we care for this? I don't mind unused function arguments. Just disable the warning for that particular aspect.

Caball009 · 2026-06-03T19:03:00Z

Disabling a warning should be the last resort.

xezon · 2026-06-03T19:37:12Z

Yes but what is this warning good for? What problem does it solve?

The problem with removing unused function argument names is that they will need to be added again when they are needed anyway.

void doStuff(float value, const char* name);
void doStuff(int value, const char*); // const char* unused

->
next we use const char*

void doStuff(float value, const char* name);
void doStuff(int value, const char* myNewCustomName); // Now argument name is
                  // added but different from the other because we did not pay attention when we added it

Why is not ok?

void doStuff(float value, const char* name);
void doStuff(int value, const char* name); // const char* unused

Caball009 · 2026-06-03T20:09:41Z

The problem with removing unused function argument names is that they will need to be added again when they are needed anyway.

I'm not arguing in favor of removing them. I can see the merit of having uniform arguments for all those functions. Unused variables are unexpected and could indicate an issue (typo or badly refactored code or whatever), though.

I'm arguing in favor marking them as ununsed with MAYBE_UNUSED. That signals that there's no issue and the compiler doesn't emit a warning for it.

xezon · 2026-06-03T20:20:41Z

Oh ok. I actually was not aware this is possible to set on function arguments. I will try.

…patcher into separate functions

xezon · 2026-06-03T20:26:02Z

MAYBE_UNUSED added to function arguments.

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 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 requested a review from Caball009 May 28, 2026 20:11

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

a84b13d

…patcher into separate functions

xezon added 10 commits June 3, 2026 22:21

Fix RETAIL_COMPATIBLE_CRC

7fd75d4

Pass currentlySelectedGroup to Surrender functions

ae7d03a

Fix 2 wrong returns

741addf

Move createOptimizedTree function decl down

b93c6a4

Replicate in Generals

907c807

Fix compile error

accb84c

Fix indentation

23d12a9

Remove superfluous AIGroupPtr argument from onEnableRetaliationMode

c5bbc39

Fix line spacing

fc6b975

Add MAYBE_UNUSED to GameMessage args

19df18e

xezon force-pushed the xezon/refactor-gamelogicdispatch branch from 3539719 to 19df18e Compare June 3, 2026 20:25

xezon changed the title ~~refactor(gamelogic): Break switch cases of GameLogic::logicMessageDispatcher into separate functions~~ refactor(gamelogic): Split switch cases of GameLogic::logicMessageDispatcher into separate functions Jun 4, 2026

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Caball009 May 24, 2026

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!

Caball009 May 24, 2026

Choose a reason for hiding this comment

Uh oh!

Caball009 May 24, 2026

Choose a reason for hiding this comment

Uh oh!

Caball009 May 24, 2026

Choose a reason for hiding this comment

Uh oh!

Caball009 May 24, 2026

Choose a reason for hiding this comment

Uh oh!

Caball009 May 24, 2026

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!

xezon May 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xezon commented May 25, 2026

Uh oh!

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

Uh oh!

Uh oh!

xezon commented Jun 1, 2026

Uh oh!

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

Uh oh!

Uh oh!

xezon commented Jun 3, 2026

Uh oh!

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

Uh oh!

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

Caball009 commented May 31, 2026 •

edited

Loading

Caball009 commented Jun 1, 2026 •

edited

Loading

Caball009 commented Jun 3, 2026 •

edited

Loading

Caball009 commented Jun 3, 2026 •

edited

Loading