You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I wonder if this is an indication that we should have a struct or type defined to represent world state, since this sort of empty initialization should be already set up on the default base world state.
It does look like there's some common code that should be extracted to avoid duplication and risks of having replicated state, particularly at the base of the world state initialization. That said, this is still non-blocking.
xsofy.world/make-world is the only real world constructor, and it's fused to generate-floor — full dungeon gen. Anything that wants a world without dungeon gen (a property-test generator, a scenario harness) has no lighter-weight constructor to call, so it hand-rolls the map literal instead. Two examples already exist, and they disagree:
tools/scenariotest/build.lg's base (added in tool: scenariotest: EDN scenario + play-loop harness #170) sets 19 keys, including :fov :memory :lights :stains :gases :fire-ttl :depth :floors :running :seed :seed-input :action-log — none of which minimal-stone-room sets.
Nothing enforces that either literal tracks what update-world and friends actually read. If the world map grows a key that some code path depends on, minimal-stone-room-derived property tests silently keep running against a world missing that key, covering less than they appear to. gen-rich-world already patches :depth, :floors, and :fire-ttl onto gen-minimal-world's shape — the in-code comment says :depth/:floors are "needed by save/restore code paths"; :fire-ttl gets added the same way, unexplained, for the fire properties that need it.
Proposal
Pull a single default/empty world-state constructor out of xsofy.world, independent of generate-floor, that both the property-test generator and the scenario harness build on:
(defnempty-world"Bare world skeleton: no floor, no entities but the player. Not playable until terrain/entities are populated by a caller."
[w h seed]
{:terrainnil:width w :height h
:entities {} :fov #{} :memory {} :lightsnil:stains {} :gases {} :fire-ttl {}
:log [] :turn0:depth1:floors {} :gold0:rune-table {}
:runningtrue:seed seed :seed-input seed :action-log []})
make-world becomes (-> (empty-world w h seed) (assoc :terrain ...) generate-floor ...), minimal-stone-room and scenariotest/build.lg's base both start from empty-world instead of hand-listing keys, and a future field addition is one edit instead of an audit across three files.
Open questions
Should this be a plain map constructor (as sketched) or a defrecord/spec that can validate shape at construction time? A record would catch the "missing key" class of bug harder than a doc comment does, at the cost of deftype/protocol churn across the codebase that treats world as an ordinary map.
Where does :terrain belong in the skeleton — nil (caller must fill it before the world is usable) or a zero-size/degenerate terrain so empty-world alone never crashes?
#170's base map is a reasonable rendering of the current shape — one more copy of it. Happy to take a pass at extracting empty-world if there's appetite.
Prompted by @nnunley's review comment on #170, on the
baseworld map intools/scenariotest/build.lg:And restated in the approval review:
xsofy.world/make-worldis the only real world constructor, and it's fused togenerate-floor— full dungeon gen. Anything that wants a world without dungeon gen (a property-test generator, a scenario harness) has no lighter-weight constructor to call, so it hand-rolls the map literal instead. Two examples already exist, and they disagree:xsofy.check_gen/minimal-stone-room(property-test generator) sets 7 keys::terrain :width :height :entities :turn :gold :rune-table.tools/scenariotest/build.lg'sbase(added in tool: scenariotest: EDN scenario + play-loop harness #170) sets 19 keys, including:fov :memory :lights :stains :gases :fire-ttl :depth :floors :running :seed :seed-input :action-log— none of whichminimal-stone-roomsets.Nothing enforces that either literal tracks what
update-worldand friends actually read. If the world map grows a key that some code path depends on,minimal-stone-room-derived property tests silently keep running against a world missing that key, covering less than they appear to.gen-rich-worldalready patches:depth,:floors, and:fire-ttlontogen-minimal-world's shape — the in-code comment says:depth/:floorsare "needed by save/restore code paths";:fire-ttlgets added the same way, unexplained, for the fire properties that need it.Proposal
Pull a single default/empty world-state constructor out of
xsofy.world, independent ofgenerate-floor, that both the property-test generator and the scenario harness build on:make-worldbecomes(-> (empty-world w h seed) (assoc :terrain ...) generate-floor ...),minimal-stone-roomandscenariotest/build.lg'sbaseboth start fromempty-worldinstead of hand-listing keys, and a future field addition is one edit instead of an audit across three files.Open questions
defrecord/spec that can validate shape at construction time? A record would catch the "missing key" class of bug harder than a doc comment does, at the cost ofdeftype/protocol churn across the codebase that treats world as an ordinary map.:terrainbelong in the skeleton —nil(caller must fill it before the world is usable) or a zero-size/degenerate terrain soempty-worldalone never crashes?#170's
basemap is a reasonable rendering of the current shape — one more copy of it. Happy to take a pass at extractingempty-worldif there's appetite.