From eafd9b5cabeaa9461a2ba86728a8ea2aaf7fe6d5 Mon Sep 17 00:00:00 2001 From: Eva Date: Sun, 31 May 2026 00:39:12 +0700 Subject: [PATCH] fix(engine): add_location defaults discovered=True so runtime-named places stay on the Atlas (#261/#371 follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #371 (#261) added `discovered: bool = False` to the Location model and taught seed_world to set discovered=True on the world's day-1 regions + ingested areas. The runtime world-building path `add_location` (servers/engine/server.py) was NOT updated: its `Location(...)` constructor never set `discovered`, so a place the DM named into the world mid-play serialized `discovered=False` and the Atlas predicate (viewer/server.py :: _atlas_visible_location_ids) hid it until the party visited it. That is a silent behavior change vs. pre-#371, where add_location'd places appeared on the Atlas immediately (the field didn't exist, and the predicate treats absent/None as visible). Decision: runtime-added places should be VISIBLE by default. ----------------------------------------------------------- A place the DM names into the world mid-play is, by that act, KNOWN — the player should see it. Evidence this is the right default, not fog-of-war: * seed_world sets day-1 regions discovered=True — "known" == True. * Pre-#371, add_location'd places were visible. * The model already has a SEPARATE `hidden` flag for deliberate suppression (predicate line: `if row.get("hidden"): continue`), so `discovered` was never meant as the runtime "hide it" lever. * The model's False default exists so FOG-OF-WAR SEEDS must opt in; the runtime path was simply missed when the field landed. Fix (additive, engine-only): * Add `discovered: bool = True` param to add_location. * Thread it into the NEW-Location constructor only. * UPDATE path (location_id reuse) is untouched → an existing place's discovered state is PRESERVED, never clobbered by the default. * Pass `discovered=False` to add a rumoured/far-off place that stays fog-of-war until visited (the deliberate path #371 enabled). * make_current=True still arrives the party (visited=True) → visible regardless, unchanged. Tests: servers/engine/tests/test_add_location_discovered.py (4) 1. default add_location → discovered=True AND visited=False (visible purely via discovered — the regression fix) 2. discovered=False → fog-of-war opt-in preserved 3. update path (location_id reuse) preserves discovered=False (no clobber) 4. make_current=True → visited=True + current (visible regardless) Verification: ast.parse clean on both files. Engine pytest left to CI (this checkout is not on the local test-execution allowlist). Scope: engine-only, additive. Does NOT touch PR #371, the viewer predicate, or any seed. Filed from an isolated worktree off origin/main (shared canonical checkout is used by parallel sessions). Refs: follow-up to #371 / #261; sibling to the #380 atlas-tier work. DO NOT MERGE yet (per owner — parallel agent still at work). --- servers/engine/server.py | 18 ++++- .../tests/test_add_location_discovered.py | 74 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 servers/engine/tests/test_add_location_discovered.py diff --git a/servers/engine/server.py b/servers/engine/server.py index ba901a63..da834ac6 100644 --- a/servers/engine/server.py +++ b/servers/engine/server.py @@ -769,6 +769,7 @@ def add_location( travel_times: Optional[dict] = None, make_current: bool = False, advance_time: bool = False, + discovered: bool = True, ) -> dict: """Create — or update — a location in the world DURING live play. The key world-building primitive for generated / sandbox campaigns. @@ -790,6 +791,16 @@ def add_location( while your prose describes the new room. This is the common live-gen pattern: create the Siltwharf Steps and step onto them in a single move. `advance_time=True` also rolls the clock one phase (a journey, not a step next door) and stirs a standing thread. + + `discovered` (default True) controls Atlas visibility for the new place. The Location + model defaults `discovered=False` so a fog-of-war seed must opt IN (#261/#371), but a + place the DM names into the world mid-play is, by that act, KNOWN — so the runtime + default here is True, matching `seed_world`'s day-1 regions and the pre-#371 behavior + where add_location'd places appeared on the Atlas immediately. Pass `discovered=False` + to add a RUMOURED/far-off place that stays fog-of-war until the party visits it. (Use + the separate `hidden` flag to suppress a place from the Atlas entirely.) On the UPDATE + path — when `location_id` matches an existing place — the existing `discovered` state is + PRESERVED, never clobbered by this default; reveal a hidden place through its own path. """ with campaign_lock(campaign_id): c = _require(campaign_id) @@ -818,7 +829,12 @@ def add_location( f"a location named {name!r} already exists ({dup.id!r}) — pass " f"location_id={dup.id!r} to update it instead of creating a duplicate" ) - loc = Location(name=name, description=description, hex=coords, region=region, travel_times=tt) + # discovered: default True so a runtime-named place is visible on the Atlas + # immediately (the model default is False for fog-of-war seeds — see #261/#371; + # add_location'd places were visible pre-#371, and seed_world's day-1 regions + # are discovered=True). Pass discovered=False for a rumoured/far-off place. + loc = Location(name=name, description=description, hex=coords, region=region, + travel_times=tt, discovered=discovered) if location_id: loc.id = location_id # honor a caller-chosen id (e.g. a generated skeleton's) c.locations[loc.id] = loc diff --git a/servers/engine/tests/test_add_location_discovered.py b/servers/engine/tests/test_add_location_discovered.py new file mode 100644 index 00000000..9eb3b5be --- /dev/null +++ b/servers/engine/tests/test_add_location_discovered.py @@ -0,0 +1,74 @@ +"""Regression: runtime add_location must default to discovered=True (#261/#371 follow-up). + +PR #371 added `discovered: bool = False` to the Location model (fog-of-war seeds opt IN) +and updated seed_world to set discovered=True on day-1 regions + ingested areas. But the +runtime world-building path `add_location` was NOT updated — so a location the DM named +into the world mid-play serialized discovered=False and was hidden from the Atlas until +the party visited it (the viewer predicate _atlas_visible_location_ids shows a place only +when visited OR discovered-is-not-False). That is a behavior change vs. pre-#371, where +add_location'd places appeared immediately. + +These tests pin the intended contract: + * default add_location → discovered=True (visible immediately, even before a visit) + * discovered=False → opt-in fog-of-war (hidden until visited) + * the UPDATE path (location_id reuse) PRESERVES an existing place's discovered state + * make_current still arrives the party (visible regardless of discovered) + +Engine-only; additive. Does NOT touch PR #371. +""" + +from __future__ import annotations + +import server + + +def _campaign() -> str: + return server.create_campaign("add_location discovered")["id"] + + +def test_add_location_default_is_discovered_and_visible_without_a_visit(): + """A second (non-current) location added with no kwargs is discovered=True and NOT + visited — so it is Atlas-visible purely via the discovered flag (the regression fix).""" + cid = _campaign() + server.add_location(cid, "Harbor Start") # first place → becomes current + spire = server.add_location(cid, "Rumour Spire")["id"] # second → NOT current + loc = server._require(cid).locations[spire] + assert loc.discovered is True + assert loc.visited is False # proves visibility comes from discovered, not a visit + assert cid and loc.id == spire + + +def test_add_location_fog_of_war_is_opt_in(): + """discovered=False preserves the deliberate fog-of-war path: hidden until visited.""" + cid = _campaign() + server.add_location(cid, "Harbor Start") + rumour = server.add_location(cid, "The Underdark (rumoured)", discovered=False)["id"] + loc = server._require(cid).locations[rumour] + assert loc.discovered is False + assert loc.visited is False + + +def test_update_path_preserves_existing_discovered_state(): + """Re-calling add_location with an existing location_id (the update path) must NOT + clobber that place's discovered with the True default — a fog-of-war place stays fog + when the DM only edits its description/connections.""" + cid = _campaign() + server.add_location(cid, "Harbor Start") + rumour = server.add_location(cid, "Hidden Vault", discovered=False)["id"] + # Update the same place (default discovered=True) — must not reveal it. + server.add_location(cid, "Hidden Vault", location_id=rumour, description="A sealed door.") + loc = server._require(cid).locations[rumour] + assert loc.discovered is False # preserved, not clobbered to True + assert loc.description == "A sealed door." + + +def test_make_current_place_is_visible_regardless_of_discovered(): + """make_current arrives the party (visited=True), so the place is visible via the + current/visited path independent of the discovered flag.""" + cid = _campaign() + server.add_location(cid, "Harbor Start") + res = server.add_location(cid, "Siltwharf Steps", make_current=True) + here = res["id"] + loc = server._require(cid).locations[here] + assert loc.visited is True + assert server._require(cid).current_location_id == here