Skip to content

refactor(metaevent): Split MetaEventTranslator::translateGameMessage() into smaller functions#2758

Merged
xezon merged 4 commits into
TheSuperHackers:mainfrom
xezon:xezon/refactor-metaevent
Jun 4, 2026
Merged

refactor(metaevent): Split MetaEventTranslator::translateGameMessage() into smaller functions#2758
xezon merged 4 commits into
TheSuperHackers:mainfrom
xezon:xezon/refactor-metaevent

Conversation

@xezon
Copy link
Copy Markdown

@xezon xezon commented May 30, 2026

This change splits MetaEventTranslator::translateGameMessage() into smaller functions for better maintainability.

Logically it should function the same.

TODO

  • Replicate in Generals

@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Refactor Edits the code with insignificant behavior changes, is never user facing labels May 30, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 30, 2026

Greptile Summary

This PR refactors MetaEventTranslator::translateGameMessage() in both the Generals and GeneralsMD codebases by extracting its body into six private helper methods (onMouseEvent, onKeyEvent, onKeyModStateRemoved, onKeyPressed, getActionKeyType, getKeyModState), addressing a previous review that flagged the methods as incorrectly public.

  • Key event path is split into onKeyEvent → either onKeyModStateRemoved (modifier-key release) or onKeyPressed (regular key action), with getActionKeyType and getKeyModState as pure extractors.
  • Mouse event path is moved verbatim into onMouseEvent; no logic changes.
  • Both Generals and GeneralsMD receive identical refactors.

Confidence Score: 5/5

Pure refactor — no observable behavior changes; safe to merge.

Every branch of the original translateGameMessage is preserved verbatim in the extracted helpers. The modifier-key state tracking, meta-map matching, mouse click synthesis, and the fast-forward-replay special case all produce the same side effects and disp value as before. The previously reported public visibility issue is fixed, and the KEY_STATE_DOWN flag vs. message-type question was explained and is sound.

No files require special attention.

Important Files Changed

Filename Overview
Generals/Code/GameEngine/Include/GameClient/MetaEvent.h Adds six new private helper method declarations to MetaEventTranslator; correctly placed in private: section.
Generals/Code/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp Extracts translateGameMessage body into five helper methods; logic appears equivalent to the original, with improved readability and consistent indentation.
GeneralsMD/Code/GameEngine/Include/GameClient/MetaEvent.h Mirror of the Generals header change; six new private helper declarations added correctly.
GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp Identical refactor to the Generals counterpart; same helper extraction with equivalent logic.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[translateGameMessage] --> B{message type?}
    B -- MSG_RAW_KEY_DOWN or MSG_RAW_KEY_UP --> C[onKeyEvent]
    B -- MSG_RAW_MOUSE_BEGIN..MSG_RAW_MOUSE_END --> D[onMouseEvent]
    B -- other --> E[return KEEP_MESSAGE]
    C --> F[getActionKeyType / getKeyModState]
    F --> G{modStateRemoved? keyType==MK_NONE && KEY_UP}
    G -- yes --> H[onKeyModStateRemoved: clear stale key-mod states, fire UP transitions]
    G -- no --> I[onKeyPressed: match meta-map, set/clear key-mod state]
    D --> J{mouse message type}
    J -- BUTTON_DOWN --> K[record down position, clear double-click flag]
    J -- DOUBLE_CLICK --> L[set double-click flag]
    J -- BUTTON_UP --> M[insert click or double-click message]
Loading

Reviews (4): Last reviewed commit: "Replicate in Generals" | Re-trigger Greptile

Comment thread GeneralsMD/Code/GameEngine/Include/GameClient/MetaEvent.h
Copy link
Copy Markdown

@RikuAnt RikuAnt left a comment

Choose a reason for hiding this comment

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

Logically sound change. The focus is clearly to just relocate the code into new helper methods, but I would recommend some minor refactors as well.

Comment thread GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp Outdated
@RikuAnt
Copy link
Copy Markdown

RikuAnt commented Jun 1, 2026

I don't have permission to resolve my own comments, so feel free to resolve them. Applies also for approval, so someone elses blessing is needed here. FYI @xezon

@xezon xezon force-pushed the xezon/refactor-metaevent branch from 95375c8 to 0575c40 Compare June 4, 2026 14:18
@xezon
Copy link
Copy Markdown
Author

xezon commented Jun 4, 2026

Replicated in Generals without conflicts

@xezon xezon merged commit 7c78a5e into TheSuperHackers:main Jun 4, 2026
32 of 33 checks passed
@xezon xezon deleted the xezon/refactor-metaevent branch June 4, 2026 14:45
fbraz3 pushed a commit to fbraz3/GeneralsX that referenced this pull request Jun 5, 2026
Upstream: TheSuperHackers/main @ 7c78a5e
Ancestor: 20f4254 (previous sync)
Divergence: 5 commits upstream / 1813 ours (small sync)

PRs brought in:
  - TheSuperHackers#2710 fix(memory): Fix various memory leaks (2)
  - TheSuperHackers#2747 bugfix(object): Avoid crash with dangling contain module (Object::onDestroy)
  - TheSuperHackers#2718 fix/refactor(milesaudiomanager): Prevent multithread crashing + simplify
  - TheSuperHackers#2577 fix(metaevent): Ignore order in which modifier keys are released
  - TheSuperHackers#2758 refactor(metaevent): Split MetaEventTranslator::translateGameMessage()

Conflict resolution (this commit):
  - Generals/GeneralsMD/.../MetaEvent.h: replaced m_lastKeyDown/m_lastModState
    members with upstream's KeyDownInfo m_keyDownInfos[KEY_COUNT] struct to
    match the refactored translator (auto-merge had unioned both).
  - Generals/GeneralsMD/.../MetaEvent.cpp: resolved UU by taking upstream's
    onKeyEvent() dispatch + dropped now-dead m_lastKeyDown/m_lastModState
    member initializers in the constructor.
  - Core/GameEngine/Include/Common/GameAudio.h: kept upstream's signature
    change (nextMusicTrack/prevMusicTrack now return AsciiString;
    getMusicTrackName removed from base). Updated our OpenAL backend
    (Core/GameEngineDevice/{Include,Source}/OpenALAudioDevice/) to match.
  - Core/Libraries/Include/Lib/BaseType.h: added missing BitsAreSet macro
    that upstream's MetaEvent code depends on.
  - AGENTS.md: cleared pre-existing merge markers (not from this sync;
    left from commit 996eedd).
  - GeneralsMD/.../Source/OpenALAudioManager.cpp stub: signature parity.

Preserved (no merge change):
  - GeneralsX cross-platform stack: SDL3, DXVK, OpenAL, FFmpeg
  - All .github/ CI/CD infrastructure intact
  - GeneralsX-specific patches and GeneralsX @BugFix annotations

Verified:
  - macos-vulkan configure + build (GeneralsX + GeneralsXZH)
  - Runtime smoke for both products (clean init, no crash)
  - No merge markers in tree (excluding build/ artifacts)
  - Cross-platform audit: no platform-specific code leaked into shared
    headers; SDL3/DXVK/OpenAL/FFmpeg layers untouched by upstream

Deferred to CI:
  - linux64-deploy configure + build (validated locally but heavy
    docker image build; CI covers)

See docs/WORKDIR/planning/PLAN-2026-06-04_THESUPERHACKERS_SYNC.md for
the full conflict resolution report and risk analysis.

Refs: TheSuperHackers#2710, TheSuperHackers#2747, TheSuperHackers#2718, TheSuperHackers#2577, TheSuperHackers#2758
fbraz3 pushed a commit to fbraz3/GeneralsX that referenced this pull request Jun 5, 2026
Upstream: TheSuperHackers/main @ 7c78a5e
Ancestor: 20f4254 (previous sync)
Divergence: 5 commits upstream / 1813 ours (small sync)

PRs brought in:
  - TheSuperHackers#2710 fix(memory): Fix various memory leaks (2)
  - TheSuperHackers#2747 bugfix(object): Avoid crash with dangling contain module (Object::onDestroy)
  - TheSuperHackers#2718 fix/refactor(milesaudiomanager): Prevent multithread crashing + simplify
  - TheSuperHackers#2577 fix(metaevent): Ignore order in which modifier keys are released
  - TheSuperHackers#2758 refactor(metaevent): Split MetaEventTranslator::translateGameMessage()

Conflict resolution (this commit):
  - Generals/GeneralsMD/.../MetaEvent.h: replaced m_lastKeyDown/m_lastModState
    members with upstream's KeyDownInfo m_keyDownInfos[KEY_COUNT] struct to
    match the refactored translator (auto-merge had unioned both).
  - Generals/GeneralsMD/.../MetaEvent.cpp: resolved UU by taking upstream's
    onKeyEvent() dispatch + dropped now-dead m_lastKeyDown/m_lastModState
    member initializers in the constructor.
  - Core/GameEngine/Include/Common/GameAudio.h: kept upstream's signature
    change (nextMusicTrack/prevMusicTrack now return AsciiString;
    getMusicTrackName removed from base). Updated our OpenAL backend
    (Core/GameEngineDevice/{Include,Source}/OpenALAudioDevice/) to match.
  - Core/Libraries/Include/Lib/BaseType.h: added missing BitsAreSet macro
    that upstream's MetaEvent code depends on.
  - AGENTS.md: cleared pre-existing merge markers (not from this sync;
    left from commit 996eedd).
  - GeneralsMD/.../Source/OpenALAudioManager.cpp stub: signature parity.

Preserved (no merge change):
  - GeneralsX cross-platform stack: SDL3, DXVK, OpenAL, FFmpeg
  - All .github/ CI/CD infrastructure intact
  - GeneralsX-specific patches and GeneralsX @BugFix annotations

Verified:
  - macos-vulkan configure + build (GeneralsX + GeneralsXZH)
  - Runtime smoke for both products (clean init, no crash)
  - No merge markers in tree (excluding build/ artifacts)
  - Cross-platform audit: no platform-specific code leaked into shared
    headers; SDL3/DXVK/OpenAL/FFmpeg layers untouched by upstream

Deferred to CI:
  - linux64-deploy configure + build (validated locally but heavy
    docker image build; CI covers)

See docs/WORKDIR/planning/PLAN-2026-06-04_THESUPERHACKERS_SYNC.md for
the full conflict resolution report and risk analysis.

Refs: TheSuperHackers#2710, TheSuperHackers#2747, TheSuperHackers#2718, TheSuperHackers#2577, TheSuperHackers#2758
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 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