Skip to content

refactor(network): Clean up utility functions for network commands#2725

Open
Caball009 wants to merge 8 commits into
TheSuperHackers:mainfrom
Caball009:refactor_network_if_else_chains
Open

refactor(network): Clean up utility functions for network commands#2725
Caball009 wants to merge 8 commits into
TheSuperHackers:mainfrom
Caball009:refactor_network_if_else_chains

Conversation

@Caball009
Copy link
Copy Markdown

@Caball009 Caball009 commented May 17, 2026

This PR cleans up some utility functions in the network code, mainly by refactoring them from if/else chains to switch statements. No change should have a functional difference.

See commits for cleaner (per function) diffs.

@Caball009 Caball009 added Minor Severity: Minor < Major < Critical < Blocker Network Anything related to network, servers Refactor Edits the code with insignificant behavior changes, is never user facing labels May 17, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 17, 2026

Greptile Summary

This PR refactors four network utility functions in NetworkUtil.cpp from verbose if/else chains to cleaner switch statements, and adds const-correctness to two function signatures. No functional changes were intended, and verification confirms the enum value sets are identical before and after for each function.

  • CommandRequiresAck is now a one-liner that delegates to DoesCommandRequireACommandID, eliminating a near-duplicate list that had a copy-paste duplicate (NETCOMMANDTYPE_DISCONNECTPLAYER) in the original.
  • GetNetCommandTypeAsString gains a CASE_LABEL macro for DRY string expansion, adds three previously missing enum values (NETCOMMANDTYPE_UNKNOWN, NETCOMMANDTYPE_DISCONNECTSTART, NETCOMMANDTYPE_DISCONNECTEND), and restores the DEBUG_CRASH guard on the default branch per the discussion on the previous thread.

Confidence Score: 5/5

Safe to merge — all refactored switch statements cover exactly the same enum values as the original if/else chains, and the previous thread concerns have been resolved.

The diff is a pure structural refactor: every function's return-true set was manually cross-checked and is identical to the original. CommandRequiresAck now correctly delegates rather than maintaining a separate (previously duplicate-containing) list. The DEBUG_CRASH on the unhandled default in GetNetCommandTypeAsString has been restored. The three newly added enum literals in the string function are additions, not removals. No logic paths changed.

No files require special attention.

Important Files Changed

Filename Overview
Core/GameEngine/Source/GameNetwork/NetworkUtil.cpp Refactored four if/else chain functions to switch statements; CommandRequiresAck simplified to delegate to DoesCommandRequireACommandID; GetNetCommandTypeAsString updated with CASE_LABEL macro, three new enum values, and DEBUG_CRASH restored on default. All functional sets verified equivalent.
Core/GameEngine/Include/GameNetwork/networkutil.h CommandRequiresAck and CommandRequiresDirectSend signatures updated to take const NetCommandMsg* — a straightforward const-correctness improvement matching the implementation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CommandRequiresAck\nconst NetCommandMsg*] -->|delegates to| B[DoesCommandRequireACommandID\nNetCommandType]
    C[CommandRequiresDirectSend\nconst NetCommandMsg*] -->|switch on getNetCommandType| D{DirectSend\nenum set}
    B -->|switch on type| E{CommandID\nenum set}
    F[IsCommandSynchronized\nNetCommandType] -->|switch on type| G{Sync\nenum set}
    H[GetNetCommandTypeAsString\nNetCommandType] -->|CASE_LABEL macro| I{All enum values\n+ UNKNOWN/START/END}
    E -->|match| J[TRUE]
    E -->|default| K[FALSE]
    D -->|match| J
    D -->|default| K
    G -->|match| J
    G -->|default| K
    I -->|match| L[return name string]
    I -->|default| M[DEBUG_CRASH +\nreturn NETCOMMANDTYPE_INVALID]
Loading

Reviews (7): Last reviewed commit: "Moved default case for 'GetNetCommandTyp..." | Re-trigger Greptile

Comment thread Core/GameEngine/Source/GameNetwork/NetworkUtil.cpp Outdated
Comment thread Core/GameEngine/Source/GameNetwork/NetworkUtil.cpp
Comment thread Core/GameEngine/Source/GameNetwork/NetworkUtil.cpp
Comment thread Core/GameEngine/Source/GameNetwork/NetworkUtil.cpp
Comment thread Core/GameEngine/Source/GameNetwork/NetworkUtil.cpp
@Caball009 Caball009 force-pushed the refactor_network_if_else_chains branch from 4d28261 to 4d43c80 Compare May 18, 2026 17:05
Comment thread Core/GameEngine/Source/GameNetwork/NetworkUtil.cpp Outdated
CASE_LABEL(NETCOMMANDTYPE_DISCONNECTSCREENOFF)
CASE_LABEL(NETCOMMANDTYPE_DISCONNECTEND)

CASE_LABEL(NETCOMMANDTYPE_UNKNOWN)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one goes to the top, in order.

@Caball009 Caball009 force-pushed the refactor_network_if_else_chains branch from daef210 to 9746679 Compare May 18, 2026 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Minor Severity: Minor < Major < Critical < Blocker Network Anything related to network, servers Refactor Edits the code with insignificant behavior changes, is never user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants