-
Notifications
You must be signed in to change notification settings - Fork 0
feat(p1): per-tile session status indicators (PRD §GM-8) #28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| """Tests for per-tile session status indicator (PRD §GM-8). | ||
|
|
||
| The bridge emits per_city_status alongside per_city_rewards in the save | ||
| exchange so the Unciv mod can surface a status glyph on each city tile | ||
| without opening the TUI. | ||
| """ | ||
|
|
||
| import json | ||
| from datetime import datetime | ||
|
|
||
| import pytest | ||
|
|
||
| from bridge.save_exchange import SaveExchange | ||
| from bridge.scoring import ScoringEngine, SimpleScoringStrategy | ||
| from bridge.session_state import Session, SessionState, SessionStatus | ||
|
|
||
|
|
||
| def _session(name: str, city_id: str, status: SessionStatus) -> Session: | ||
| s = Session(name, name, status, datetime.now()) | ||
| s.city_id = city_id | ||
| return s | ||
|
|
||
|
|
||
| def test_per_city_status_single_session(): | ||
| sessions = [_session("s1", "camelot", SessionStatus.ACTIVE)] | ||
| out = ScoringEngine(SimpleScoringStrategy()).calculate_per_city_status(sessions) | ||
| assert out == {"camelot": "active"} | ||
|
|
||
|
|
||
| def test_per_city_status_resolves_multi_session_via_priority(): | ||
| """Two sessions on one city: surface the most-engaged status.""" | ||
| sessions = [ | ||
| _session("s1", "camelot", SessionStatus.WAITING), | ||
| _session("s2", "camelot", SessionStatus.ACTIVE), # higher priority | ||
| ] | ||
| out = ScoringEngine(SimpleScoringStrategy()).calculate_per_city_status(sessions) | ||
| assert out["camelot"] == "active" | ||
|
|
||
|
|
||
| def test_per_city_status_redirected_beats_waiting(): | ||
| sessions = [ | ||
| _session("s1", "x", SessionStatus.WAITING), | ||
| _session("s2", "x", SessionStatus.REDIRECTED), | ||
| ] | ||
| out = ScoringEngine(SimpleScoringStrategy()).calculate_per_city_status(sessions) | ||
| assert out["x"] == "redirected" | ||
|
|
||
|
|
||
| def test_per_city_status_suspended_at_lowest_priority(): | ||
| """Even one ACTIVE session in a city with mostly suspended sessions | ||
| surfaces ACTIVE as the indicator.""" | ||
| sessions = [ | ||
| _session("s1", "x", SessionStatus.SUSPENDED), | ||
| _session("s2", "x", SessionStatus.SUSPENDED), | ||
| _session("s3", "x", SessionStatus.ACTIVE), | ||
| ] | ||
| out = ScoringEngine(SimpleScoringStrategy()).calculate_per_city_status(sessions) | ||
| assert out["x"] == "active" | ||
|
|
||
|
|
||
| def test_per_city_status_includes_inactive_and_quiet_cities(): | ||
| """PRD: 'readable without opening TUI' — inactive cities must show too.""" | ||
| sessions = [ | ||
| _session("s1", "active_city", SessionStatus.ACTIVE), | ||
| _session("s2", "quiet_city", SessionStatus.INACTIVE), | ||
| _session("s3", "done_city", SessionStatus.COMPLETED), | ||
| ] | ||
| out = ScoringEngine(SimpleScoringStrategy()).calculate_per_city_status(sessions) | ||
| assert out == { | ||
| "active_city": "active", | ||
| "quiet_city": "inactive", | ||
| "done_city": "completed", | ||
| } | ||
|
|
||
|
|
||
| def test_per_city_status_buckets_unmapped_under_empty_key(): | ||
| s = Session("s1", "x", SessionStatus.ACTIVE, datetime.now()) | ||
| s.city_id = "" | ||
| out = ScoringEngine(SimpleScoringStrategy()).calculate_per_city_status([s]) | ||
| assert "" in out | ||
| assert out[""] == "active" | ||
|
|
||
|
|
||
| def test_save_exchange_emits_per_city_status(tmp_path): | ||
| state = SessionState(kingdom_name="Camelot") | ||
| a = state.add_session("Court") | ||
| a.session_id = "sa" | ||
| a.city_id = "camelot" | ||
| a.status = SessionStatus.ACTIVE | ||
|
|
||
| b = state.add_session("Camp") | ||
| b.session_id = "sb" | ||
| b.city_id = "north_fort" | ||
| b.status = SessionStatus.WAITING | ||
|
|
||
| ex = SaveExchange(tmp_path / "saves") | ||
| ex.write_payload(state) | ||
| data = json.loads(ex.path.read_text(encoding="utf-8")) | ||
| assert "per_city_status" in data | ||
| assert data["per_city_status"] == { | ||
| "camelot": "active", | ||
| "north_fort": "waiting", | ||
| } | ||
|
|
||
|
|
||
| def test_per_city_status_survives_save_round_trip(tmp_path): | ||
| state = SessionState(kingdom_name="X") | ||
| s = state.add_session("S1") | ||
| s.city_id = "z" | ||
| s.status = SessionStatus.REDIRECTED | ||
|
|
||
| ex = SaveExchange(tmp_path / "saves") | ||
| ex.write_payload(state) | ||
| data = json.loads(ex.path.read_text(encoding="utf-8")) | ||
| assert data["per_city_status"]["z"] == "redirected" | ||
|
|
||
|
|
||
| def test_per_city_status_empty_when_no_sessions(tmp_path): | ||
| state = SessionState() | ||
| ex = SaveExchange(tmp_path / "saves") | ||
| ex.write_payload(state) | ||
| data = json.loads(ex.path.read_text(encoding="utf-8")) | ||
| assert data["per_city_status"] == {} | ||
|
|
||
|
|
||
| # ──────────────────────────────────────────────────────────────────────── | ||
| # Mod-side contract sanity (no Lua runtime — just documents the keys) | ||
| # ──────────────────────────────────────────────────────────────────────── | ||
|
|
||
| def test_per_city_status_uses_lowercase_status_values_for_lua_glyph_table(): | ||
| """mod.lua's STATUS_GLYPHS table is keyed by lowercase strings — | ||
| "active", "redirected", "waiting", etc. — so the bridge must emit | ||
| SessionStatus.value (already lowercase) verbatim. | ||
| """ | ||
| sessions = [ | ||
| _session("a", "c", SessionStatus.ACTIVE), | ||
| _session("b", "d", SessionStatus.REDIRECTED), | ||
| ] | ||
| out = ScoringEngine(SimpleScoringStrategy()).calculate_per_city_status(sessions) | ||
| assert all(v.islower() for v in out.values()) | ||
| assert out["c"] == "active" | ||
| assert out["d"] == "redirected" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,6 +78,12 @@ local function readKingdomSave(currentTurn) | |
| rewards = perCityRewards | ||
| end | ||
|
|
||
| -- Per-city status indicator (PRD §GM-8) — kept on the parsed table | ||
| -- so callers can opt in via getKingdomStatus(). | ||
| if parsed.per_city_status and type(parsed.per_city_status) == "table" then | ||
| rewards.__per_city_status__ = parsed.per_city_status | ||
| end | ||
|
|
||
| -- Cache the rewards for this turn | ||
| rewardCache = rewards | ||
| cacheTurn = currentTurn | ||
|
|
@@ -88,6 +94,66 @@ local function readKingdomSave(currentTurn) | |
| return rewards | ||
| end | ||
|
|
||
| --[[ | ||
| Returns the kingdom status table (city_id → status string) as last | ||
| parsed from the save exchange. Used by applyCityStatusIndicator at | ||
| turn boundary to surface the per-tile session indicator (PRD §GM-8). | ||
| ]] | ||
| local function getKingdomStatus() | ||
| if rewardCache and rewardCache.__per_city_status__ then | ||
| return rewardCache.__per_city_status__ | ||
| end | ||
| return {} | ||
| end | ||
|
|
||
| --[[ | ||
| Maps the bridge's session status string to a short tile glyph the | ||
| player can read at a glance without opening the TUI (PRD §GM-8). | ||
| Falls back to the raw status string if no glyph is registered. | ||
| ]] | ||
| local STATUS_GLYPHS = { | ||
| active = "[*]", -- working | ||
| redirected = "[!]", -- redirected (transient) | ||
| waiting = "[~]", -- awaiting human | ||
| inactive = "[ ]", -- registered but quiet | ||
| completed = "[v]", -- session done | ||
| suspended = "[#]", -- game closed elsewhere; preserved | ||
| } | ||
|
|
||
| local function statusGlyph(status) | ||
| if not status then return "" end | ||
| return STATUS_GLYPHS[tostring(status)] or ("[" .. tostring(status) .. "]") | ||
| end | ||
|
|
||
| --[[ | ||
| Applies the per-tile session status indicator to a city tile (PRD §GM-8). | ||
| Tries setOverlay → setTooltip → setName-suffix in priority order; each | ||
| is wrapped in pcall so a missing Unciv API method never crashes the | ||
| mod. Logs the indicator regardless so the data flow is auditable. | ||
| ]] | ||
| local function applyCityStatusIndicator(city, status) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The function |
||
| if not city or not status then return end | ||
| local cityName = "unknown" | ||
| if city.getName then cityName = city.getName(city) end | ||
| local glyph = statusGlyph(status) | ||
| print(string.format("Status %s on %s", glyph, cityName)) | ||
|
|
||
| if city.setOverlay then | ||
| pcall(function() city.setOverlay(city, glyph) end) | ||
| return | ||
| end | ||
| if city.setTooltip then | ||
| pcall(function() city.setTooltip(city, "Session: " .. tostring(status)) end) | ||
| return | ||
| end | ||
| if city.setName and city.getName then | ||
| local base = city.getName(city) | ||
| -- Avoid stacking glyphs across turns: strip any existing trailing glyph. | ||
| local stripped = base:gsub(" %[[^%]]+%]$", "") | ||
| pcall(function() city.setName(city, stripped .. " " .. glyph) end) | ||
| end | ||
| end | ||
|
|
||
| --[[ | ||
| Applies rewards to a specific city. | ||
| @param city: The city object (from Unciv API) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current implementation of
calculate_per_city_statusis inefficient because it performs redundant string-to-enum conversions (SessionStatus(current)) inside the loop. By tracking theSessionStatusobjects directly in theoutdictionary and only converting them to their string values in the return statement, the code becomes cleaner and more performant.