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