fix(engine): add_location defaults discovered=True so runtime-named places stay on the Atlas (#261/#371 follow-up)

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

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