Skip to content

refactor(world): empty-world — one canonical world-state skeleton - #176

Merged
mparrett merged 1 commit into
mainfrom
refactor/empty-world
Jul 24, 2026
Merged

refactor(world): empty-world — one canonical world-state skeleton#176
mparrett merged 1 commit into
mainfrom
refactor/empty-world

Conversation

@mparrett

@mparrett mparrett commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

refactor(world): empty-world — one canonical world-state skeleton

Closes #172. Follows up @nnunley's review comments on #170.

Why

xsofy.world/make-world is the only world constructor and it's fused to generate-floor, so it always runs full dungeon gen. Anything that wants a world without dungeon gen has no lighter constructor to call and hand-rolls the map literal instead. Four copies existed, in two shapes that disagree:

Site Keys set
check-gen/minimal-stone-room 7
check-gen/build-feature-room 7
combat_prop's fresh fixture 7
tools/scenariotest/build.lg's base (added in #170) 20

The three 7-key literals have identical key sets; base sets 20. So the split is the one #172 named — a test-side shape and a scenario-side shape, 13 keys apart.

Nothing tied any of them to what update-world and friends read. Add a field to the world map and the generated worlds silently fall behind: a property test keeps running green against a world missing the very key the code under test reads, covering less than it looks like it covers. gen-rich-world was already patching :depth, :floors, and :fire-ttl back on after the fact to close part of that gap.

What

world/empty-world is now the one place the construction-time world shape is written down. generate-floor builds on it, and each hand-rolled literal starts from it and assocs only what's specific to that caller:

(assoc (world/empty-world w h seed)
       :terrain terrain
       :entities {:player player}
       :log boot-log
       :depth depth)

Adding a construction-time field is one edit here instead of an audit across four files. A field that also has to survive a floor transition still needs restore-floor and change-floor updated by hand — see the follow-up note below.

Decisions

Plain map, not a defrecord. #172 raised the option. A record would catch the missing-key class harder than a docstring does, but the codebase treats world as an ordinary map throughout: assoc, get-in, :keys destructuring, serialize/canonical-bytes walking it generically. The churn isn't worth it for a shape one function now owns. Happy to run with this or switch to a record if you'd rather have construction-time validation.

:terrain is nil, not a degenerate grid. The other open question in #172. A zero-size grid would let empty-world alone avoid crashing, at the price of a world whose :width/:height disagree with its terrain. Nil means a caller that forgets to fill it fails at the first tget instead of reading plausible garbage.

The skeleton covers construction-time shape only. :rooms-cache, :next-id, and :scheme belong to the systems that add them later, so they stay out.

invariant-test's sparse room is unchanged. The tests for world-invariants? shouldn't be built from the production constructor, or a broken empty-world goes invisible to exactly the predicate meant to catch it.

generate-floor merge order. The skeleton goes down first, then world-after-terrain merges over it so terrain gen's advanced :seed lineage still wins. The skeleton contributes :seed-input as the origin seed, which is what that key means on both call paths. Both callers reset :seed afterward, same as before.

Verification

World content is unchanged except for one added empty key. make-world at seeds 1, 42, and 1000, plus a d1→d2→d3→d2 floor-transition chain, produce the same canonical-bytes hash on this branch as on main once the newly-present :fire-ttl {} is removed. That key is new on every freshly generated world, so a whole-world checkpoint-hash does change; nothing persists or asserts against one (replay codes are (seed, action-log), dag/commit-id is :seed alone).

  • make test — 307 tests, 2803 assertions, 0 fail
  • make smoke-lg — OK
  • lg tools/scenariotest.lg --check on both scenarios — OK

`make-world` was the only world constructor and it is fused to
`generate-floor`, so anything wanting a world without dungeon gen
hand-rolled the map literal instead. Four copies existed and they
disagreed: the property-test room set 7 keys, the scenario builder 19,
the feature-room generator 6, and a combat-prop fixture 6. Nothing tied
any of them to what `update-world` and friends actually read, so a new
world field would silently leave the generated worlds behind — property
tests would keep passing against a world missing the key under test.

`world/empty-world` is now the single place the construction-time shape
is written down. `generate-floor` builds on it and every hand-rolled
literal starts from it.

Notes:

- `generate-floor` merges `world-after-terrain` over the skeleton so the
  terrain-advanced :seed lineage still wins; the skeleton contributes
  :seed-input as the origin seed, which is what that key means. Both
  callers reset :seed afterward as before.
- `:terrain` is nil, not a zero-size grid — a caller that forgets to
  fill it should fail at the first tget rather than read garbage out of
  a grid disagreeing with :width/:height.
- Plain map, not a defrecord. A record would catch missing keys harder,
  but the whole codebase treats world as an ordinary map (assoc,
  get-in, destructuring, canonical-bytes) and the churn is not worth it
  for a shape one function now owns.
- `xsofy.test.invariant-test`'s sparse room is left alone on purpose: it
  tests that world-invariants? tolerates a world missing optional keys,
  so building it from the skeleton would remove the point.

Verified byte-identical world content: `make-world` for seeds 1/42/1000
and a d1→d2→d3→d2 floor-transition chain produce the same canonical-bytes
hash before and after, modulo the newly-present `:fire-ttl {}`. Test
suite 307/2803 green, smoke-lg OK, both scenariotest scenarios --check OK.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mparrett
mparrett requested a review from nnunley July 24, 2026 19:14
@mparrett mparrett added the deploy-preview Publish a gh-pages WASM preview for this PR label Jul 24, 2026
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-24 19:34 UTC

@mparrett

mparrett commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

A second review pass over this turned up things worth recording before merge. None of them change the diff as approved; they are all follow-up work.

percept.lg:63 becomes a latent trap. entity-fov uses key presence as its "player FOV already computed" sentinel:

(and (= entity-id :player) (some? (:fov world))) (:fov world)

The test-side worlds previously had no :fov key, so they fell through and computed fresh FOV. They now get :fov #{} from the skeleton, which some? accepts, so the empty set is returned as though it were a result. Nothing calls it today — percept is reached only from balance.lg, which runs on real make-world output — so this is not a live regression, but it will surprise the first percept-based property test. fov/compute-fov seeds its result with #{[ox oy]} (fov.lg:80), so a computed FOV is never empty and (seq (:fov world)) is a sound replacement.

The docstring overclaims. world.lg:195 says "this is the one place that shape is written down." restore-floor (world.lg:400-421) is a 21-key world literal 180 lines below it, and change-floor (world.lg:694-702) carries eight more keys by hand. For any field that has to survive a floor transition, the PR body's "one edit instead of an audit" is wrong — it is three. Correcting the docstring and folding both into the skeleton is the follow-up.

:next-id is excluded for the wrong stated reason. The docstring attributes it to entity spawning, but entities/next-id is seed-derived and never writes the key; the only writer is det/next-id, called only from det_test.lg. Right to leave it out, wrong justification.

Corrected the key counts in the body. The three test-side literals set 7 keys each with identical key sets, and scenariotest's base set 20 — the same 20 the skeleton has. So there were two distinct shapes, not four disagreeing ones, which is what #172 said in the first place. The original table overstated it.

Filed the :title/:quest-item transition drift as #177 — it is pre-existing and independent of this change.

@mparrett
mparrett merged commit 890fa88 into main Jul 24, 2026
6 checks passed
@mparrett
mparrett deleted the refactor/empty-world branch July 24, 2026 19:34
mparrett added a commit that referenced this pull request Jul 28, 2026
`main.lg` assocs `:title`, `:scheme`, and `:quest-item` onto the world
right after `make-world`. Descending dropped two of the three: both
`change-floor`'s carry-over assoc and `restore-floor`'s 21-key literal
hand-listed what survives, and only `:scheme` was on both lists. The
visible symptom is a nameless tombstone — `render.lg` reads `:title` for
the death screen, so anyone dying below depth 1 lost it. `:quest-item`
has no reader yet, so its loss was silent and waiting.

#176 unified world *construction* onto `empty-world` while leaving the
transition path spelling the shape out twice by hand, which is how a
field could be construction-correct and transition-lossy at once. This
finishes that job:

- `run-scoped-keys` names what belongs to the run rather than the floor,
  and `carry-run-state` is the one thing both transition paths call.
- `restore-floor` builds on `empty-world` like every other constructor,
  assoc'ing the floor-scoped fields it reads back out of `:floors`.
- `empty-world`'s docstring no longer claims to be the only place the
  shape is written down — it now names `run-scoped-keys` as the
  companion answer to "does this field survive a descent?". It also
  dropped a false claim that `:next-id` comes from entity spawning;
  `entities/next-id` is seed-derived and never writes that key.

`:seed` is deliberately excluded from `run-scoped-keys`. It heads the
RNG chain and both callers restore it as their final step, so that
`spawn-runestones` in between consumes the floor lineage rather than the
gameplay one. Carrying it with the rest would move that restore earlier
and shift every subsequent roll.

`generate-floor` now takes `:seed` across from the terrain-advanced world
explicitly instead of merging it wholesale. Behavior is identical today —
terrain gen threads only `:seed` — but the blanket merge would have
silently handed it every other key the day it threads more.

New `world_shape_test.lg` pins both halves: every skeleton key present
after construction, after a descent, and after a revisit; and the
run-scoped fields surviving both. Its four `:title`/`:quest-item`
assertions fail on `main` and pass here.

Determinism verified unchanged: a d1→d2→d3→d2→d1 chain produces
identical `canonical-bytes` hashes and identical `:seed` before and
after, once the newly-surviving `:title`/`:quest-item` are excluded.
Suite 314/2818 green, smoke-lg OK, both scenariotest scenarios OK.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mparrett added a commit that referenced this pull request Jul 28, 2026
`entity-fov` decides whether the player's FOV is already cached by
asking whether `:fov` is present:

    (and (= entity-id :player) (some? (:fov world))) (:fov world)

That read the world's shape as a signal. It worked only while worlds
that hadn't been through `update-fov` had no `:fov` key at all — which
stopped being true in #176, when `empty-world` began seeding it to
`#{}`. Any caller perceiving a skeleton-built world now gets the empty
set handed back as though it were a computed result, so "can the player
see anything" answers no.

Nothing hits this today: `percept` is reached only from `balance.lg`,
which runs on `make-world` output that `generate-floor` has already
FOV'd. It's the first percept-based property test that would have found
it, by watching every visibility assertion quietly answer false.

`(seq (:fov world))` is the honest test. `fov/compute-fov` seeds its
result with the origin cell, so a computed FOV always contains at least
the entity's own position and an empty one can only mean uncomputed.

New `percept_test.lg` covers both sides of the branch plus the NPC path;
its two player-side assertions fail against the current `percept.lg` and
pass with this change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mparrett added a commit that referenced this pull request Aug 7, 2026
`entity-fov` decides whether the player's FOV is already cached by
asking whether `:fov` is present:

    (and (= entity-id :player) (some? (:fov world))) (:fov world)

That read the world's shape as a signal. It worked only while worlds
that hadn't been through `update-fov` had no `:fov` key at all — which
stopped being true in #176, when `empty-world` began seeding it to
`#{}`. Any caller perceiving a skeleton-built world now gets the empty
set handed back as though it were a computed result, so "can the player
see anything" answers no.

Nothing hits this today: `percept` is reached only from `balance.lg`,
which runs on `make-world` output that `generate-floor` has already
FOV'd. It's the first percept-based property test that would have found
it, by watching every visibility assertion quietly answer false.

`(seq (:fov world))` is the honest test. `fov/compute-fov` seeds its
result with the origin cell, so a computed FOV always contains at least
the entity's own position and an empty one can only mean uncomputed.

New `percept_test.lg` covers both sides of the branch plus the NPC path;
its two player-side assertions fail against the current `percept.lg` and
pass with this change.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deploy-preview Publish a gh-pages WASM preview for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No canonical empty/skeleton world-state constructor — two hand-rolled copies already disagree

2 participants