From eadae4dc4fbac385378fb131201e32574bb5c20b Mon Sep 17 00:00:00 2001 From: Kamil Krzywda Date: Sun, 11 Oct 2026 19:22:33 +0000 Subject: [PATCH] map: click a region to switch the dossier to its owner; fix "no owner" (v0.7.0) --- AGENTS.md | 13 ++++- CHANGELOG.md | 8 +++ package.json | 2 +- server/map.test.ts | 66 +++++++++++++++++++++++- shared/protocol.ts | 17 ++++++ src/components/WorldMap.tsx | 100 +++++++++++++++++++++--------------- 6 files changed, 162 insertions(+), 44 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index dd86e87..1796abb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -72,7 +72,7 @@ history/ │ └── utils.ts # cn() └── components/ ├── PresetPicker.tsx / NationSelect.tsx # home + nation pick screens - ├── WorldMap.tsx # MapLibre basemap + choropleth + click popup + player highlight + ├── WorldMap.tsx # MapLibre basemap + choropleth + click→dossier + hover tooltip + player highlight ├── NationPanel.tsx # a nation's stats/territory/diplomacy dossier ├── ActionPanel.tsx # tabs: Action / Diplomacy / Advisor + pending-queue list (per-item trash button + confirm dialog) + "Jump forward" ├── EventFeed.tsx # scrolling event log (newest first) @@ -140,6 +140,17 @@ history/ (model down) surfaces as an HTTP 502 to the client (shown as a toast). - **Stable per-nation colors.** `shared.nationColor(id)` hashes a nation id → hue, shared by the server's `RegionMap` and any client coloring so the choropleth always agrees. +- **Map click → dossier target (owner resolved from the RegionMap).** Clicking a map region does + NOT open a popup — it switches the dossier to that region's owning nation + (`setViewingId(ownerId)` in `GameScreen`); the manual "Inspect" selector and a map click both set + the same target and stay in sync. The owning nation is resolved from the authoritative `RegionMap` + by the feature's stable ISO_A3 code, which `applyRegions` stamps into `properties.regionId` + (properties survive into the rendered features) and the shared `regionOwner()` helper looks up. + It must **never** be read from the feature's top-level `id`: MapLibre repacks a GeoJSON source's + `id` into a numeric index when it stores the source, so at click/hover time `feature.id` is a + number, never the ISO_A3 (that repacking was the "no owner" bug, #6). Unowned regions (no + `RegionMap` entry) leave the current dossier target unchanged — never a broken dossier. The + hover path (a non-closeable tooltip with owner + key stats) uses the same resolution. - **Deterministic stub.** `server/stub.ts` answers the model contract deterministically per queued round item: each `action` item → one military/success event, each `diplomacy` item → one diplomatic/partial event, an empty round → one political "The world moves on" event. The player diff --git a/CHANGELOG.md b/CHANGELOG.md index d9400bb..1692c8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ All notable changes to **History** are listed here, newest first. +## v0.7.0 — 2026-10-11 19:17 + +- Clicking a map region now switches the selected dossier to that region's owning nation instead of opening a popup (#7). The closeable click popup is gone; a hover tooltip (owner + key stats) is kept. The manual "Inspect" selector still works and stays in sync — both it and a map click drive the same dossier target. +- Fixed: a clicked owned region no longer reports "no owner" (#6). Root cause: MapLibre repacks a GeoJSON source's top-level `id` (the ISO_A3 string) into a numeric index when it stores the source, so the feature a click/hover handler receives has a *number* for `id` — the old `regions[f.id]` lookup resolved to nothing, and the popup read "No owner" even for owned regions (the choropleth fill still worked because it is stamped from the original `id` before `setData`). Fix: `applyRegions` now stamps the stable ISO_A3 code into `properties.regionId` (and `ownerId`), which survives into the rendered features, and both the click and hover handlers resolve the owning nation from that code via the authoritative RegionMap — never from the repacked `feature.id`. +- Unowned regions (no entry in the RegionMap) do not change the dossier when clicked — the current target is left in place (a deliberate, non-crashing choice rather than a special "unowned" dossier state). +- New shared helper `regionOwner(regions, isoA3)` in `@shared/protocol` (the pure, single source of truth for "ISO_A3 → owner record"). +- **Unit tests** — new `regionOwner` tests (owned → owner record, unowned → undefined, empty/missing → undefined) plus a #6 regression guard that pins the exact MapLibre-repacked feature shape (numeric `id`, ISO_A3 in `properties.regionId`) and asserts resolution via `properties.regionId` works while the old `feature.id` path fails (44 tests, up from 39). + ## v0.6.0 — 2026-10-11 18:27 - Advisor fetches now show a "thinking" indicator (mirroring the turn's "Simulating…" hint) while the model responds, and the Ask-advisor button disables during the fetch; the top-bar status shows "Advisor thinking…" in the meantime. Fixes #15. diff --git a/package.json b/package.json index cc9d003..be7bbb0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "history", - "version": "0.6.0", + "version": "0.7.0", "private": true, "type": "module", "description": "AI-driven alternate-history simulator — the LLM is the world engine (Pax Historia style).", diff --git a/server/map.test.ts b/server/map.test.ts index 724001d..72c713c 100644 --- a/server/map.test.ts +++ b/server/map.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; import { buildRegionMap } from './map'; -import { nationColor } from '../shared/protocol'; +import { nationColor, regionOwner, type RegionMap } from '../shared/protocol'; import type { GameState } from '../shared/types'; const state: GameState = { @@ -33,6 +33,9 @@ const state: GameState = { version: 1, }; +// The RegionMap built from the fixture: the authoritative ISO_A3 → owner source. +const regions: RegionMap = buildRegionMap(state); + describe('buildRegionMap', () => { it('maps each ISO_A3 region to its owner + that nation\'s stable color', () => { const m = buildRegionMap(state); @@ -44,6 +47,67 @@ describe('buildRegionMap', () => { }); }); +describe('regionOwner', () => { + it('resolves an owned region\'s ISO_A3 to its owner record', () => { + expect(regionOwner(regions, 'GBR')).toEqual({ + color: nationColor('uk'), + owner: 'UK', + ownerId: 'uk', + }); + expect(regionOwner(regions, 'DEU')).toEqual({ + color: nationColor('de'), + owner: 'DE', + ownerId: 'de', + }); + }); + + it('returns undefined for an unowned region (no RegionMap entry)', () => { + expect(regionOwner(regions, 'FRA')).toBeUndefined(); + expect(regionOwner(regions, 'USA')).toBeUndefined(); + }); + + it('returns undefined for an empty or missing code', () => { + expect(regionOwner(regions, '')).toBeUndefined(); + expect(regionOwner(regions, null)).toBeUndefined(); + expect(regionOwner(regions, undefined)).toBeUndefined(); + }); +}); + +/** + * #6 regression guard. The click/hover handlers resolve the owning nation from a + * map feature. MapLibre repacks a GeoJSON source's top-level `id` (the ISO_A3 + * string) into a *numeric index* when it stores the source, so a feature the map + * hands back at interaction time has a number for `id` and the ISO_A3 only lives + * in `properties.regionId`. This test pins the exact data shape and proves the + * fix: resolution via `properties.regionId` works, while the pre-fix path that + * read `feature.id` does not. + */ +describe('#6 — owner must be resolved from properties.regionId, not the repacked feature.id', () => { + // The shape MapLibre returns for a source feature after setData(): `id` is a + // number (here 0, as observed in the live map), and the ISO_A3 is preserved + // only in properties (the client stamps it as `regionId`). + const repackedFeature = { + id: 0, + properties: { name: 'DE', regionId: 'DEU' }, + }; + + it('resolves the owner from properties.regionId (the fix)', () => { + const code = typeof repackedFeature.properties.regionId === 'string' ? repackedFeature.properties.regionId : ''; + expect(regionOwner(regions, code)).toEqual({ + color: nationColor('de'), + owner: 'DE', + ownerId: 'de', + }); + }); + + it('the old path (feature.id as a string key) does NOT resolve — the bug', () => { + // String(0) === '0' is not a valid ISO_A3 key, so the pre-fix lookup failed + // and the click popup reported "No owner" even for owned regions. + expect(regions[String(repackedFeature.id)]).toBeUndefined(); + expect(regionOwner(regions, String(repackedFeature.id))).toBeUndefined(); + }); +}); + describe('nationColor', () => { it('is deterministic per id and distinct across different ids', () => { expect(nationColor('uk')).toBe(nationColor('uk')); diff --git a/shared/protocol.ts b/shared/protocol.ts index 52e8fb1..5bbe3e9 100644 --- a/shared/protocol.ts +++ b/shared/protocol.ts @@ -56,6 +56,23 @@ export interface RegionOwner { } export type RegionMap = Record; +/** + * Look up the owner record for a map region from the authoritative RegionMap by + * its ISO_A3 code. Returns `undefined` for an empty/missing code or an unowned + * region. Pure + shared: the client resolves a clicked or hovered map feature by + * first reading the feature's stable ISO_A3 code (stored in `properties.regionId` + * by the client when it repacks the source — see WorldMap) and then calling this. + * + * The code must NOT be taken from the feature's top-level `id`: MapLibre repacks + * a GeoJSON source's `id` (an ISO_A3 string) into a numeric index when it stores + * the source, so at click/hover time `feature.id` is a number, never the ISO_A3 + * — resolving from it is exactly the "no owner" bug (#6). + */ +export function regionOwner(regions: RegionMap, isoA3: string | null | undefined): RegionOwner | undefined { + if (!isoA3) return undefined; + return regions[isoA3]; +} + /** Server -> client Server-Sent Events payload (see server/index.ts /sse). */ export type SseMessage = | { type: 'state'; state: GameState } diff --git a/src/components/WorldMap.tsx b/src/components/WorldMap.tsx index 2de8fab..d79db52 100644 --- a/src/components/WorldMap.tsx +++ b/src/components/WorldMap.tsx @@ -8,7 +8,7 @@ import 'maplibre-gl/dist/maplibre-gl.css'; // Point it at a Vite-emitted copy of the worker module so it loads in prod. import workerUrl from 'maplibre-gl/dist/maplibre-gl-worker.mjs?url'; maplibregl.setWorkerUrl(workerUrl); -import type { RegionMap, RegionOwner } from '@shared/protocol'; +import { regionOwner, type RegionMap, type RegionOwner } from '@shared/protocol'; import type { Nation } from '@shared/types'; import { cn } from '@/lib/utils'; @@ -101,21 +101,36 @@ function cameraForPreset(presetId: string): { center: [number, number]; zoom: nu * carries its owner's stable fill color, a subtle stroke, and a flag for the * player's own regions (rendered as a brighter, thicker border by a dedicated * layer). Unknown owners fall back to a faint neutral fill — never an error. + * + * Crucially, the base GeoJSON feature's top-level `id` (the ISO_A3 code) is + * stamped into `properties.regionId`, and the owner's id into `properties.ownerId`. + * MapLibre repacks a GeoJSON source's top-level `id` into a numeric index when it + * stores it, so a later click/hover event exposes that number on + * `e.features[i].id` — the top-level `id` is *not* a reliable lookup key at + * interaction time (the root cause of #6). `properties`, however, survive into the + * rendered features, so the interaction handlers resolve the owning nation from + * `properties.regionId` via the RegionMap (see `regionOwner`). */ function applyRegions(geo: FeatureCollection, regions: RegionMap, playerNationId: string): FeatureCollection { const out: FeatureCollection = JSON.parse(JSON.stringify(geo)); for (const f of out.features) { f.properties = f.properties ?? {}; - const r = f.id ? regions[f.id as string] : undefined; + // The base GeoJSON `id` is the ISO_A3 code; keep it in a property that + // survives into the features MapLibre hands back on click/hover. + const code = f.id == null ? '' : String(f.id); + f.properties.regionId = code; + const r = code ? regions[code] : undefined; if (r) { f.properties.fill = r.color; f.properties.owner = r.owner; + f.properties.ownerId = r.ownerId; f.properties.stroke = 'rgba(255,255,255,0.10)'; f.properties.isPlayer = r.ownerId === playerNationId ? 1 : 0; } else { f.properties.fill = 'rgba(148,163,184,0.05)'; f.properties.stroke = 'rgba(255,255,255,0.06)'; f.properties.owner = ''; + f.properties.ownerId = ''; f.properties.isPlayer = 0; } } @@ -150,35 +165,19 @@ function hoverHtml(countryName: string, region?: RegionOwner, nation?: Nation): `; } -/** Click popup: the full owning-nation profile with all four stats. */ -function clickHtml(countryName: string, region?: RegionOwner, nation?: Nation): string { - if (!region || !nation) { - return `
${esc(countryName || 'Unclaimed')}
No owner
`; - } - const stats = [ - statRow('Mil', nation.stats.military), - statRow('Eco', nation.stats.economy), - statRow('Stab', nation.stats.stability), - statRow('Pop', nation.stats.population), - ].join(''); - const regions = nation.territory.length; - return `
-
${esc(countryName || region.owner)}
-
${esc(region.owner)}
-
${regions} region${regions === 1 ? '' : 's'} · click focuses the dossier
-
${stats}
-
`; -} - interface Props { regions: RegionMap; playerNationId: string; - /** All nations, for hover/click stats and color chips. */ + /** All nations, for the hover tooltip's owner stats and color chips. */ nations: Record; /** The active scenario id, for a sensible initial camera. */ presetId: string; className?: string; - /** Called when a region is clicked, so the dossier can focus its owner. */ + /** + * Called with a region's owning-nation id when that region is clicked, so the + * dossier switches to that nation. Not called for unowned regions (the dossier + * keeps its current target). See the component doc for the full behavior. + */ onFocusNation?: (id: string) => void; } @@ -187,10 +186,18 @@ interface Props { * nation. Rendered as a fixed, full-viewport background layer (z-0) behind the * floating HUD windows: a dark-navy basemap with subtle terrain, per-nation * stable fills (the choropleth is the star), thin borders, a brighter/thicker - * border for the player, hover + click popups with - * owner + stats, a transient flash on territory changes, and a - * per-scenario initial camera. Zoom and pan are driven directly by the user - * (MapLibre wheel/trackpad + drag) — no floating control and no external API. + * border for the player, a hover tooltip (owner + key stats), a **click that + * switches the dossier to the region's owning nation** (no popup — see #7), a + * transient flash on territory changes, and a per-scenario initial camera. + * + * Ownership for a hovered/clicked region is resolved from the authoritative + * RegionMap by the feature's stable ISO_A3 code in `properties.regionId` + * (see `regionOwner` in @shared/protocol) — never from the feature's top-level + * `id`, which MapLibre repacks into a numeric index at interaction time (#6). + * Unowned regions (no RegionMap entry) simply do not change the dossier. + * + * Zoom and pan are driven directly by the user (MapLibre wheel/trackpad + drag) + * — no floating control and no external API. */ export default function WorldMap({ regions, @@ -243,7 +250,6 @@ export default function WorldMap({ (containerRef.current as unknown as { __map?: maplibregl.Map }).__map = map; const hoverPopup = new maplibregl.Popup({ closeButton: false, closeOnClick: false, offset: 10 }); - const clickPopup = new maplibregl.Popup({ closeButton: true, closeOnClick: true, offset: 12 }); let lastHoverId: string | null = null; const loadGeo = (): Promise => @@ -300,24 +306,33 @@ export default function WorldMap({ } flashApiRef.current = animateFlash; + // The region a hovered/clicked feature belongs to is identified by its stable + // ISO_A3 code, read from `properties.regionId` (stamped by `applyRegions`). + // We must NOT use the feature's top-level `id`: MapLibre repacks a GeoJSON + // source's `id` into a numeric index when it stores the source, so the value + // exposed on `e.features[i].id` at interaction time is a number, never the + // ISO_A3 — resolving the owner from it is the "no owner" bug (#6). const onHover = (e: maplibregl.MapLayerMouseEvent) => { const f = e.features?.[0]; if (!f) return; - const id = f.id == null ? null : String(f.id); - if (id && id !== lastHoverId) { - lastHoverId = id; + const p = f.properties as { name?: string; regionId?: string }; + const code = typeof p?.regionId === 'string' ? p.regionId : ''; + if (code && code !== lastHoverId) { + lastHoverId = code; const geo = baseGeoRef.current; - const feat = geo?.features.find((x) => x.id != null && String(x.id) === id); + // The base GeoJSON feature's top-level `id` IS the ISO_A3, so match on + // that to recover the feature for the hover-highlight outline (the + // feature MapLibre hands us has a repacked numeric `id` we can't use). + const feat = geo?.features.find((x) => x.id != null && String(x.id) === code); (map.getSource(HOVER_SOURCE) as GeoSource | undefined)?.setData( feat ? { type: 'FeatureCollection', features: [JSON.parse(JSON.stringify(feat))] } : emptyFC(), ); } - const p = f.properties as { name?: string }; - const region = id ? regionsRef.current[id] : undefined; + const region = regionOwner(regionsRef.current, code); const nation = region ? nationsRef.current[region.ownerId] : undefined; hoverPopup .setLngLat(e.lngLat) - .setHTML(hoverHtml(p?.name ?? id ?? '', region, nation)) + .setHTML(hoverHtml(p?.name ?? code, region, nation)) .addTo(map); map.getCanvas().style.cursor = 'pointer'; }; @@ -329,14 +344,17 @@ export default function WorldMap({ map.getCanvas().style.cursor = ''; }; + // #7: a click does NOT open a popup — it switches the dossier to the region's + // owning nation. The owner is resolved from the stable ISO_A3 in + // `properties.regionId` via the RegionMap (never the repacked top-level + // `f.id`, #6). A region with no owner in the RegionMap (unowned territory) + // leaves the current dossier target unchanged — never a broken dossier. const onClick = (e: maplibregl.MapLayerMouseEvent) => { const f = e.features?.[0]; if (!f) return; - const id = f.id == null ? null : String(f.id); - const p = f.properties as { name?: string }; - const region = id ? regionsRef.current[id] : undefined; - const nation = region ? nationsRef.current[region.ownerId] : undefined; - clickPopup.setLngLat(e.lngLat).setHTML(clickHtml(p?.name ?? id ?? '', region, nation)).addTo(map); + const p = f.properties as { regionId?: string }; + const code = typeof p?.regionId === 'string' ? p.regionId : ''; + const region = regionOwner(regionsRef.current, code); if (region?.ownerId) onFocusNationRef.current?.(region.ownerId); }; -- 2.54.0