Skip to content

GameEvent refactor#9760

Draft
MostCromulent wants to merge 7 commits intoCard-Forge:masterfrom
MostCromulent:gameeventrefactor
Draft

GameEvent refactor#9760
MostCromulent wants to merge 7 commits intoCard-Forge:masterfrom
MostCromulent:gameeventrefactor

Conversation

@MostCromulent
Copy link
Contributor

@MostCromulent MostCromulent commented Feb 15, 2026

@tool4ever here is the GameEvent refactor.

This has been done based on the refactor plan which has been updated to reflect implementation. See implementation notes section for deviations from original plan.

I have not had a chance to test in depth yet - but unit test is succesful and local network testing indicates client is at least playing sounds now.

In relation to subgame events Claude ran into an issue, summarised below:

Subgame Event Reclassification — Options

The Problem

The event refactor (Step 5) added blanket forwarding of all GameEvents to remote clients via handleGameEvent(). The two subgame events (GameEventSubgameStart, GameEventSubgameEnd) contain Game references — not serializable, and their handlers do host-only GUI wiring (subscribing to EventBus, switching GameView, iterating PlayerControllerHuman). Forwarding these to a remote client would fail at serialization and is conceptually wrong — they're local lifecycle signals.

Per your earlier feedback, these should be reclassified as UiEvent to create a clean dividing line: GameEvent = networked game state, UiEvent = local GUI concerns.

The Constraint

SubgameEffect (in forge-game) creates and fires both events. UiEvent is defined in forge-gui. forge-game cannot depend on forge-gui — the dependency goes the other way. There is no existing precedent for forge-game code firing UiEvents; all 3 current UiEvent types are created exclusively in forge-gui input handlers.

Option A: Filter in Forwarding Code

Skip subgame events in the HostedMatch.receiveGameEvent() forwarding loop with an instanceof check.

  • Pros: 2-line change, zero architectural disruption
  • Cons: No type-system enforcement — the dividing line is a runtime check, not a compile-time guarantee. Future GameEvent types that shouldn't be forwarded would need additional filters.

Option B: LocalGameEvent Marker Interface in forge-game

Create a new interface LocalGameEvent extends GameEvent in forge-game. Subgame events implement LocalGameEvent instead of GameEvent directly.
Forwarding code skips any LocalGameEvent instance.

  • Pros: Type-system enforced dividing line within forge-game. Any future host-only events can implement LocalGameEvent. No module boundary issues.
  • Cons: Requires removing subgame visit methods from IGameEventVisitor and updating all 5 visitor implementations. A new concept (LocalGameEvent) that's effectively a subset of GameEvent — not as clean as GameEvent vs UiEvent.

Option C: Replace Events with Callback Hooks

Remove the subgame events entirely. SubgameEffect calls hooks on Game or Match (like the existing startGameHook/endGameHook pattern in
HostedMatch). HostedMatch registers the hooks during game setup.

  • Pros: Cleanest separation — subgame lifecycle exits the event system entirely. No forwarding concern. Follows existing precedent (startGameHook, endGameHook).
  • Cons: Most invasive. Requires refactoring SubgameEffect to use callbacks, and moving the GUI wiring logic from event visitors into hook implementations. Changes the execution model (hooks are synchronous callbacks vs EventBus dispatch).

MostCromulent and others added 7 commits February 16, 2026 06:04
Add isInstant()/isSorcery() to CardStateView, isSpell()/isTrigger()/
getActivatingPlayer() tracked properties to SpellAbilityView,
toString() override to GameEntityView, and Serializable to GameEvent.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Change canonical fields from Card/Player to CardView/PlayerView with
convenience constructors for backward compatibility. Fix cascading
compilation in GameLogFormatter, FControlGameEventHandler,
FControlGamePlayback, QuestController, CMatchUI, IGuiGame, and
AbstractGuiGame. Change handleLandPlayed to accept CardView.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Convert remaining complex events to use view types (CardView,
PlayerView, SpellAbilityView, StackItemView, GameEntityView) instead
of engine types. Includes convenience constructors for backward
compatibility at call sites.

Complex conversions include:
- Collection/map events (CardStatsChanged, CardRegenerated,
  CombatEnded, CombatUpdate, AttackersDeclared, BlockersDeclared,
  AnteCardsSelected, PlayerStatsChanged)
- SpellAbility events (SpellAbilityCast, SpellResolved,
  SpellRemovedFromStack) with pre-computed fields
- Zone events (CardChangeZone, Zone, ManaPool, CardAttachment)
- Game lifecycle events (GameStarted, GameOutcome, PlayerControl)

Update all visitors (GameLogFormatter, FControlGameEventHandler,
FControlGamePlayback, EventVisualizer, CMatchUI) to work with view
types returned by event accessors.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add handleGameEvent protocol method to forward all GameEvents from
server to client. Events are now serializable (view-based fields)
and can be sent directly through the existing Netty pipeline.

Changes:
- ProtocolMethod: add handleGameEvent(GameEvent) entry
- IGuiGame: add handleGameEvent(GameEvent) interface method
- AbstractGuiGame: default no-op implementation
- NetGuiGame: override to updateGameView() then send event to client
- HostedMatch: forward all events to GUI instances after visiting

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove 5 ProtocolMethod entries now replaced by GameEvent forwarding:
- updatePlayerControl, updateZones, updateCards, updateManaPool, updateLives

Remove corresponding NetGuiGame overrides that sent these as separate
protocol messages. Add no-op defaults in AbstractGuiGame for the
interface methods (still called locally by event visitors).

IGuiGame signatures are kept — only the network-specific forwarding
is removed. Event forwarding via handleGameEvent now carries the
equivalent data to clients.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix SpellAbilityView NPE: defer isSpell/isTrigger/activatingPlayer
  updates from constructor to lazy getView(), preventing NPE when
  WrappedAbility.sa is null during super() construction
- Fix GameEventManaPool NPE: guard null mana in convenience constructor
  (ManaPool.clearPool fires with null mana on Cleared events)
- Add GameEventSerializationTest: 9 tests validating ObjectOutputStream
  round-trips for view-typed event records (simple, collection, null
  safety, pre-computed fields, no-field events)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SoundSystem received GameEvents via direct event bus subscription,
which only exists on the host. Move sound triggering to
AbstractGuiGame.handleGameEvent() so the client plays sounds when
events arrive over the network protocol.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants