Skip to content

feat(p1): visible plan/goal/redirect event announcements per turn (PRD §GM-5)#29

Merged
SolshineCode merged 1 commit into
mainfrom
feat/p1-gm5-visible-event-bonuses
May 8, 2026
Merged

feat(p1): visible plan/goal/redirect event announcements per turn (PRD §GM-5)#29
SolshineCode merged 1 commit into
mainfrom
feat/p1-gm5-visible-event-bonuses

Conversation

@SolshineCode
Copy link
Copy Markdown
Owner

Closes audit P1: GM-5. Bridge buffers plan/goal/redirect events per city per turn → SaveExchange embeds per_city_events → mod.lua announces them with medieval phrasing per tile. 9 new tests; 210 total. Refs PR #22, kanban t_26404be3.

…M-5)

Closes audit P1: GM-5. The bridge now buffers plan/goal/redirect
events as they fire in the listener, flushes them on turn boundary,
and embeds them in the save exchange so the Unciv mod can announce
the discrete bonus per city tile (e.g. '+15 from plan'). Without this
the plan/goal bonuses are still applied as resources but the player
can't see WHY the totals jumped — making the M3 emotional contract
ring hollow.

bridge/bridge_loop.py:
- New _pending_events_by_city buffer (city_id → list of event records).
- HookListener.on_change is now wrapped to record events into the
  buffer, then chain through the audit log, then any pre-existing
  callback. Records only the kinds that produce a visible bonus:
  plan_added (1 record), goal_completed (1 record — dedupes the
  paired momentum bump), redirect (1 record).
- _flush_per_city_events called inside process_turn snapshots the
  buffer, clears it, stashes on state._pending_per_city_events for
  SaveExchange to pick up, and fires a per_city_events callback.

bridge/save_exchange.py — write_payload reads
state._pending_per_city_events and emits per_city_events alongside
per_city_rewards / per_city_status.

mod/ClaudeKingdoms/mod.lua:
- readKingdomSave now keeps per_city_events on the cached rewards
  table via __per_city_events__.
- New getKingdomEvents() helper returns the city_id → events map.
- New announcementFor(event) → medieval-flavor announcement string
  ('A new charter is drafted for X (+15 production)' etc.).
- New applyCityEventAnnouncements(city, eventList) tries
  city.announce → city.addNotification → falls back to print so the
  data flow is visible without an Unciv UI surface. Each pcall-wrapped.

bridge/tests/test_per_city_events.py — 9 tests:
- Plan event buffered then flushed on turn (buffer empty after turn)
- Goal event records once (dedup the momentum-paired StateChange)
- Redirect event recorded
- per_city_events lands in save exchange JSON
- Empty buffer → empty dict
- Buffer resets between turns
- per_city_events callback fires on turn complete
- Event records carry session_id + session_name
- Multi-city: events segregate by city_id

Total: 210 passing (was 201; adds 9).

Closes audit P1: GM-5. Refs: PR #22, kanban t_26404be3.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@SolshineCode SolshineCode merged commit 18b87ba into main May 8, 2026
1 check passed
@SolshineCode SolshineCode deleted the feat/p1-gm5-visible-event-bonuses branch May 8, 2026 20:26
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements per-turn per-city event buffering and surfacing to allow the Unciv mod to display transient announcements for plan, goal, and redirect events. Key changes include the addition of an event buffer in BridgeLoop, logic to flush these events into the SaveExchange payload, and new Lua functions in the mod for formatting and applying announcements. Review feedback points out that the Lua functions for displaying announcements are defined but never actually called in the turn-processing logic. Additionally, the implementation uses monkey-patching on the SessionState object to pass event data, which is noted as a brittle approach that should be replaced by updating the class definition or function signatures.

addNotification → falls back to print so the data flow is visible
even without an Unciv UI surface. pcall-wrapped.
]]
local function applyCityEventAnnouncements(city, eventList)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The function applyCityEventAnnouncements (and its helper announcementFor) is defined but never called anywhere in the provided changes. To fulfill the PR's objective of surfacing announcements in the mod, this function should be invoked within the turn-processing logic (e.g., inside mod.turnEnd), likely after rewards are applied to each city.

Comment thread bridge/bridge_loop.py
per_city_events = self._flush_per_city_events()
# Stash on the SessionState so SaveExchange can read it without
# changing its signature.
self.state._pending_per_city_events = per_city_events # type: ignore[attr-defined]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Monkey-patching SessionState with a private attribute (_pending_per_city_events) is brittle and bypasses type checking. While the comment explains this avoids changing the SaveExchange.write_payload signature, it would be cleaner to either update the signature of write_payload to accept the events explicitly or add the field to the SessionState dataclass definition in bridge/session_state.py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant