From db713af47a8bcd0e0e40a16a0df1dafa72fdf780 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 16:06:05 -0400 Subject: [PATCH 1/2] feat(engine): world-building/OLC command surface (dig/tunnel/describe) Closes Slice 4 of the WheelMUD reconciliation roadmap (ADR-0009): adds dig/tunnel/describe, gated at SecurityRole.MinorBuilder via the same RegisterWithRole mechanism ADR-0005 introduced for moderation commands. Extracts a shared RoomConnector.Connect helper (Engine) from the two world-boot builders (BasicWorldBuilder, HubWorldBuilder), which each had their own identical copy of the two-way exit-wiring logic - one shared implementation instead of three. Fixes a real bug found during manual Telnet verification: dig/tunnel didn't check whether the current room already had an exit in the requested direction, so digging into an already-occupied direction silently created an unreachable, shadowed duplicate exit rather than rejecting. Added BuilderCommandHelpers.HasExit plus regression tests. Documents the new commands - and backfills the previously-undocumented Slice 3 moderation commands - in the public docsite, and records a world-loading scale concern surfaced during design discussion as tracked future work (PLAN-0001 Slice 11) rather than solving it speculatively. Co-Authored-By: Claude Sonnet 5 --- SPEC.md | 36 ++- ...0009-world-building-olc-command-surface.md | 246 ++++++++++++++++++ docs/adr/README.md | 1 + docs/commands.md | 14 +- docs/persistence.md | 24 +- .../0001-wheelmud-reconciliation-roadmap.md | 30 ++- ...0009-world-building-olc-command-surface.md | 243 +++++++++++++++++ docs/plans/README.md | 1 + docs/world-model.md | 22 +- docsite/docs/customizing.md | 5 + docsite/docs/moderation-and-world-building.md | 123 +++++++++ docsite/zensical.toml | 3 +- .../HubWorldBuilder.cs | 25 +- samples/SharpMud.Samples.Classic/Program.cs | 12 +- .../Behaviors/RoomConnector.cs | 29 +++ .../Builtin/Builder/BuilderCommandHelpers.cs | 62 +++++ .../Builtin/Builder/BuilderCommands.cs | 21 ++ .../Builtin/Builder/DescribeCommand.cs | 37 +++ .../Commands/Builtin/Builder/DigCommand.cs | 66 +++++ .../Commands/Builtin/Builder/TunnelCommand.cs | 89 +++++++ .../BasicWorldBuilder.cs | 17 +- .../Behaviors/RoomConnectorTests.cs | 28 ++ .../Builtin/Builder/BuilderCommandsTests.cs | 22 ++ .../Builtin/Builder/DescribeCommandTests.cs | 53 ++++ .../Builtin/Builder/DigCommandTests.cs | 98 +++++++ .../Builtin/Builder/TunnelCommandTests.cs | 149 +++++++++++ 26 files changed, 1398 insertions(+), 58 deletions(-) create mode 100644 docs/adr/0009-world-building-olc-command-surface.md create mode 100644 docs/plans/0009-world-building-olc-command-surface.md create mode 100644 docsite/docs/moderation-and-world-building.md create mode 100644 src/SharpMud.Engine/Behaviors/RoomConnector.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommandHelpers.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommands.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Builder/DescribeCommand.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Builder/DigCommand.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Builder/TunnelCommand.cs create mode 100644 tests/SharpMud.Engine.Tests/Behaviors/RoomConnectorTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/BuilderCommandsTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/DescribeCommandTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/DigCommandTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/TunnelCommandTests.cs diff --git a/SPEC.md b/SPEC.md index e2cac8a..f40dca1 100644 --- a/SPEC.md +++ b/SPEC.md @@ -114,11 +114,15 @@ derivatives separate "instant" commands from round-based combat resolution. ### Content authoring evolution (not all v1) -1. Hardcoded rooms in C# (bootstrap only, to get the loop working). -2. Data-driven world files (JSON/YAML) loaded at startup — natural next step, - also the foundation for in-game building commands later. -3. In-game building commands (`@dig`, `@describe`, etc.) writing to the same - data model. +1. Hardcoded rooms in C# (bootstrap only, to get the loop working) — still + how every world's starting content is authored today. +2. Data-driven world files (JSON/YAML) loaded at startup — **not yet + implemented**; turned out not to be a prerequisite for stage 3 below. +3. In-game building commands (`dig`/`tunnel`/`describe`) writing directly + to the live `Thing` tree — **implemented**, role-gated at + `SecurityRole.MinorBuilder`. See + [ADR-0009](docs/adr/0009-world-building-olc-command-surface.md) and + [docs/world-model.md](docs/world-model.md). ## Command System @@ -157,8 +161,9 @@ derivatives separate "instant" commands from round-based combat resolution. for a plain name (no identity verification) as a placeholder until this phase lands. 8. **Moderation/admin tooling**: implemented — see below. -9. **Procedural frontier generation**, **in-game building/scripting**: later - phases (see Deferred/Open Items). +9. **In-game building** (`dig`/`tunnel`/`describe`): implemented — see + Content authoring evolution above. **Procedural frontier generation** + and **soft-code/scripting**: later phases (see Deferred/Open Items). ## Accounts & Auth ✅ implemented @@ -204,6 +209,23 @@ Explicitly out of scope for v1, to revisit later: where — undesigned. Tracked as an open item in [PLAN-0005](docs/plans/0005-security-role-model-and-moderation-commands.md); the moderation commands themselves are implemented (see above). +- **NPC/item spawning, mob-respawn loops, loot tables**: undesigned. An NPC + killed in combat is removed from the world permanently (no respawn + timer); loot drops aren't implemented at all (see + [docs/combat.md](docs/combat.md)). Surfaced while scoping ADR-0009, + deliberately kept out of it — tracked as Slice 10 in + [PLAN-0001](docs/plans/0001-wheelmud-reconciliation-roadmap.md), not yet + a numbered slice in ADR-0001 itself. +- **World persistence loads everything into memory, always**: `ThingRepository` + reconstructs the *entire* stored world on every load — fine at + hand-built-hub scale, won't scale to real user counts or a large + procedurally generated world. A persistence-layer/loading-strategy + concern, separable from the `Thing`/`Behavior` domain model itself (see + [docs/persistence.md](docs/persistence.md) Open Items for the full + reasoning). Undesigned — revisit once there's a real deployment + approaching actual scale, or Slice 9 makes the world large enough that + eager-loading stops being free. Tracked as Slice 11 in + [PLAN-0001](docs/plans/0001-wheelmud-reconciliation-roadmap.md). - **Soft-code/scripting engine**: revisit once data/config-driven NPC and room behavior proves insufficient. - **Procedural frontier generation algorithm**: choice of generation approach diff --git a/docs/adr/0009-world-building-olc-command-surface.md b/docs/adr/0009-world-building-olc-command-surface.md new file mode 100644 index 0000000..6ae1b0a --- /dev/null +++ b/docs/adr/0009-world-building-olc-command-surface.md @@ -0,0 +1,246 @@ +# [ADR-0009] World-Building/OLC Command Surface + +**Status:** Accepted + +**Date:** 2026-07-23 + +**Decision Makers:** solo (design dive conducted with the user) + +## Context + +Per [ADR-0001](0001-wheelmud-reconciliation-roadmap.md), this is Slice 4 +of the WheelMUD reconciliation roadmap, explicitly bundled with Slice 3 +(now `SecurityRole`, [ADR-0005](0005-security-role-model-and-moderation-commands.md)) +since it consumes the same mechanism. WheelMUD's own OLC surface is +minimal — just `Actions/OLC/Tunnel.cs`, an admin command that wires a +two-way exit between two *already-existing* rooms by id; WheelMUD has no +room-creation command at all (rooms are only ever built in code via +`Core/Creators/`). + +`docs/world-model.md`'s "Content Authoring Evolution" section, though, +already names a further stage sharp-mud wants that WheelMUD never had: +in-game building commands (`@dig`, `@describe`) writing directly into the +live world tree, explicitly *not* requiring a data-file format first. +`docs/commands.md`'s V1 Verb List already flags builder/OLC verbs as +excluded pending "the deferred in-game building phase" — this ADR is that +phase. + +Verified by direct research (see `docs/plans/0009-world-building-olc-command-surface.md` +for exact file/method citations): rooms today only ever get created once, +at boot, in a ruleset's `IWorldBuilder.Build()` (e.g. +`src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs`), which also contains the +only existing "wire a two-way exit" logic (`Connect(world, a, b, +direction)` — two `Thing`s, one `ExitBehavior` each, one per direction, +using the existing `Direction.Opposite()` extension). There is no +runtime/in-game equivalent, and no player-facing short id for a room today +— just an internal `ThingId` (`Guid`), never surfaced to a user. +`SecurityRole.MinorBuilder`/`FullBuilder` (added in ADR-0005, accumulation +already implemented) exist and are unused, doc-commented as reserved for +this slice. + +## Decision Drivers + +- ADR-0001 already scoped this as "small once #3 exists" — the mechanism + (role-gated commands via `RegisterWithRole`, admin-command dependency + shape via constructor-injected `IThingRepository`) is fully reusable + from Slice 3, nothing new needed there. +- `world-model.md` already commits sharp-mud to going further than + WheelMUD's Tunnel-only OLC (room creation, not just room linking) — + reconciling only the narrower WheelMUD scope would leave `commands.md`'s + own flagged gap half-closed. +- No new `Thing` subtype (`design-decisions.md` rule 2) — a "room" is + still a `Thing` + `RoomBehavior`, built exactly like + `BasicWorldBuilder.CreateRoom` does today, just at runtime instead of at + boot. +- `CommandParser` splits on whitespace only (`src/SharpMud.Engine/Commands/CommandParser.cs`) + — no quoted-string support exists anywhere in the command pipeline + today. Any multi-word argument (a room name) has to be "rest of the + line," which rules out a command needing two independent free-text + fields (e.g. name *and* description) in one line. +- Rooms have no player-facing identifier other than `Thing.Name` — a + targeting scheme has to be built on that (name-based lookup) or a wholly + new id concept invented; introducing a new persisted id purely for this + slice is unjustified scope given name-based lookup already works and + nothing else in the codebase uses non-`Guid` ids. + +## Considered Options + +**Scope (what this slice builds):** + +1. **Tunnel-only** — one command, linking two pre-existing rooms by name, + matching WheelMUD's actual OLC surface exactly. +2. **Dig + tunnel** — add room *creation* (`dig`, wiring a new room to the + current one) alongside a `tunnel`-only command for linking two + pre-existing rooms, closing `world-model.md`'s stated gap in full. + +**Room targeting:** + +1. **By exact `Thing.Name`** — reuse the field players already see; + ambiguous/duplicate names rejected with a clear error. +2. **New short numeric/slug room id** (WheelMUD/ROM-`vnum`-style) — a new + stable identifier, its own admin command to list rooms with their ids, + and a new persisted field. + +## Decision Outcome + +Chosen: **"2 — dig + tunnel"** for scope, **"1 — by exact `Thing.Name`"** +for targeting. + +Scope: WheelMUD's Tunnel-only OLC solves a narrower problem than the one +sharp-mud has already committed to solving (`world-model.md`'s stage-3 +authoring evolution). Reconciling only the WheelMUD-faithful subset would +require redesigning this again immediately to add room creation — better +to build both now, in the same slice, while the security-role mechanism +and admin-command dependency shape are already fresh from Slice 3. + +Targeting: researched ROM/Diku's OLC convention directly as prior art +(numeric `vnum` addressing, block-allocated per area, exits set +per-direction with no automatic two-way mirroring — a builder sets both +sides manually). That scheme solves a problem sharp-mud doesn't have +(multiple builders editing separate area files, needing collision-free +ids across files) — sharp-mud has one live, in-memory world tree, so a +new id concept would be solving for a constraint that isn't real here. +Name-based lookup costs nothing new; a `vnum`-equivalent would require +designing and persisting an id field, plus a command to discover it, +for a benefit that doesn't apply to this codebase's actual shape. + +**Mechanism, in full:** + +- Three new commands, all gated `SecurityRole.MinorBuilder`, registered + via `RegisterWithRole` (mirrors Slice 3's `AdminCommands` shape exactly) + in a new `BuilderCommands.RegisterAll(registry, repository)`: + - **`dig `** — creates a new `Thing` + + `RoomBehavior` (empty `Description`, filled in afterward via + `describe`), attached as a child of `ctx.CurrentRoom`'s parent (the + same area/container `ctx.CurrentRoom` itself lives under — a dug room + is a sibling of the room it's dug from, not a child of it; exits are + children of rooms, rooms are children of an area), then wires a + bidirectional exit between `ctx.CurrentRoom` and the new room in the + given direction (and its `Direction.Opposite()` back). + - **`tunnel `** — looks up an existing + room by exact `Name` among `world.AllWithBehavior()`; + rejects with a clear message if zero or more than one match. Wires + the same bidirectional exit as `dig`, between `ctx.CurrentRoom` and + the found room. + - **`describe `** — sets `ctx.CurrentRoom.Description` to the + rest-of-line text. Not itself in WheelMUD's Tunnel-only OLC, but a + direct, unavoidable consequence of choosing option 2 above (`dig` + creates a room with no description; without a way to set one + afterward, every dug room is permanently blank) and matches + `world-model.md`'s own `@describe` naming. + - All three take `IThingRepository` via constructor injection (same + shape as Slice 3's six repository-needing admin commands), and after + mutating the tree, walk `ctx.CurrentRoom` up via `.Parent` to the + root `Thing` (no `Parent`) and call `repository.SaveTreeAsync(root, + ct)` — not `SaveTreeAsync(ctx.CurrentRoom, ct)`, because `dig`'s new + room and its own reverse exit live outside `ctx.CurrentRoom`'s own + subtree (siblings, not descendants); only a save rooted above both + rooms captures everything that changed in one call. +- The exit-wiring logic (`Connect`-equivalent: two `Thing`s, one + `ExitBehavior` each, `Direction`/`Direction.Opposite()`) is extracted + from `BasicWorldBuilder` into a small shared helper in + `SharpMud.Engine` (all the types involved — `Thing`, `Direction`, + `ExitBehavior`, `IWorld` — already live in `Engine`), and + `BasicWorldBuilder.Connect` is updated to call it instead of keeping its + own copy — a direct, justified dedup enabled by writing the same logic + a second time in this slice, not a separate unrelated refactor. +- No delete/undo command (`undig`, room removal) — not in WheelMUD's + Tunnel either, and removal safety (what happens to players/items/exits + currently inside a removed room) is a genuinely different problem, + deliberately deferred rather than folded in here. + +### Positive Consequences + +- Closes `commands.md`'s explicitly-flagged builder/OLC gap in full, not + just the narrower WheelMUD-equivalent subset. +- `SecurityRole.MinorBuilder`/`FullBuilder` get their first real consumer, + as ADR-0005 anticipated. +- Reuses Slice 3's registration/dependency/save patterns exactly — no new + architectural shape introduced. +- The `Connect`-logic dedup between `BasicWorldBuilder` and the new + commands means there's exactly one place that knows how to wire a + two-way exit, not two copies that can drift. + +### Negative Consequences + +- No room-removal/undo path — a `dig` mistake is permanent (fixable only + by leaving the room in place, unconnected, or a manual DB edit); accepted + as a smaller, separable problem, not a reason to hold up this slice. +- Name-based room lookup means two rooms sharing a name are ambiguous to + `tunnel` (rejected with an error, not silently picking one) — acceptable + given no naming-uniqueness constraint exists elsewhere in the world + model today, and enforcing one now would be a separate, broader change. +- `describe` only touches the room the builder is currently standing in — + there's no `describe ` for a room elsewhere; consistent with + WheelMUD's Tunnel needing no such remote-targeting either, but a real + limitation if a builder wants to batch-describe several dug rooms + before walking between them. +- **Explicitly does not include NPC/item spawning** — surfaced during this + ADR's own scoping discussion: sharp-mud today has no mob-respawn loop or + loot-table mechanism at all (`docs/combat.md` — NPC death permanently + removes it from the world, no timer brings it back; loot drops are + "not implemented, blocked on the item system"). This maps to WheelMUD's + `Clone`/`Spawn` admin actions, already explicitly deferred by + [PLAN-0005](../plans/0005-security-role-model-and-moderation-commands.md) + pending "item/NPC creation tooling" — a genuinely separate decision + (spawn timing, loot table shape, whether `Clone`/`Spawn` are even the + right shape for a tick-driven respawn vs. a one-shot admin placement) + deliberately left for its own future slice rather than folded in here, + per the user's explicit call during this ADR's design dive. + +## Pros and Cons of the Options + +### Scope option 1: Tunnel-only + +- Good, because it's the smallest possible slice, most faithful to + WheelMUD's actual OLC surface. +- Bad, because it doesn't close `world-model.md`'s already-stated gap + (room creation) — a second design pass would be needed almost + immediately after. + +### Scope option 2: Dig + tunnel (chosen) + +- Good, because it closes the real, already-documented gap in one pass + while the Slice 3 mechanism is fresh. +- Bad, because it's more surface than WheelMUD's own OLC ever had — not a + 1:1 reconciliation, a deliberate extension past it. + +### Targeting option 1: By exact `Thing.Name` (chosen) + +- Good, because it needs no new concept, field, or command — the name + players already see is the identifier. +- Bad, because duplicate room names are ambiguous (rejected, not silently + resolved). + +### Targeting option 2: New short id (vnum-style) + +- Good, because it's what WheelMUD/ROM-family MUDs actually do, and scales + better if the world ever has many same-named rooms. +- Bad, because it solves a multi-builder/multi-area-file collision problem + sharp-mud doesn't have (one live world tree, not separate area files), + and requires a new persisted field plus a discovery command with no + other consumer today. + +## Links + +- [ADR-0001](0001-wheelmud-reconciliation-roadmap.md) — WheelMUD + Reconciliation Roadmap (this is Slice 4, bundled with Slice 3). +- [ADR-0005](0005-security-role-model-and-moderation-commands.md) — the + `RegisterWithRole`/`SecurityRole` mechanism this slice consumes + unchanged; `MinorBuilder`/`FullBuilder` were added there specifically + for this slice. +- [PLAN-0009](../plans/0009-world-building-olc-command-surface.md) — + execution plan for this decision. +- `docs/world-model.md` — "Content Authoring Evolution" stage this ADR + implements; also where frontier/procedural generation (out of scope, + Slice 9) is separately tracked. +- `docs/commands.md` — the V1 Verb List's builder/OLC exclusion this ADR + resolves. +- `src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs` — existing + boot-time room/exit creation pattern this slice's runtime commands + mirror (and whose `Connect` logic moves into `Engine`, shared). +- WheelMUD `Actions/OLC/Tunnel.cs` — source consulted; adopted the + two-way-exit-by-direction shape, explicitly extended past it (room + creation) per the Decision Outcome above. +- ROM/Diku-family `redit`/vnum OLC convention — researched as prior art + for room targeting, explicitly not adopted (see Decision Outcome). diff --git a/docs/adr/README.md b/docs/adr/README.md index 6a11235..9c56806 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -68,3 +68,4 @@ the mechanics: numbering, status, and the index. | [0006](0006-nuget-package-distribution.md) | NuGet Package Distribution + Sample-Based Ruleset Extraction | Accepted | | [0007](0007-narrow-meta-package-scope.md) | Narrow the `SharpMud` Meta-Package to Engine + Hosting + Persistence | Accepted | | [0008](0008-ruleset-scaffolding-tier.md) | A Reusable RPG Scaffolding Tier Between `Engine` and Concrete Rulesets | Accepted | +| [0009](0009-world-building-olc-command-surface.md) | World-Building/OLC Command Surface | Accepted | diff --git a/docs/commands.md b/docs/commands.md index 08dde87..ab92c84 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -124,9 +124,17 @@ actually implemented as of the inventory/items build-order phase: - **Character**: `score`/`stats` (display derived stats — see [character.md](character.md)) is **not implemented yet**. - **Meta** ✅: `help`, `quit`. -- **Builder/OLC verbs** (`@dig`, `@describe`, etc.) explicitly excluded — - those belong to the deferred in-game building phase (see - [world-model.md](world-model.md)). **Moderation/admin verbs** ✅ +- **Builder/OLC verbs** ✅ (`dig `, `tunnel + `, `describe `) — role-gated at + `SecurityRole.MinorBuilder` via `ICommandRegistry.RegisterWithRole`, same + mechanism as the moderation verbs below. `dig` creates a new room and + wires a two-way exit to it; `tunnel` wires the same kind of exit between + the current room and an already-existing room found by exact name; + `describe` sets the current room's description. No NPC/item spawning, + no room deletion — see + [ADR-0009](adr/0009-world-building-olc-command-surface.md) and + [PLAN-0009](plans/0009-world-building-olc-command-surface.md). + **Moderation/admin verbs** ✅ (`boot`/`mute`/`unmute`/`announce`/`ban`/`unban`/`rolegrant`/ `rolerevoke`) are role-gated via `ICommandRegistry.RegisterWithRole` — see [ADR-0005](adr/0005-security-role-model-and-moderation-commands.md) diff --git a/docs/persistence.md b/docs/persistence.md index c413e68..7916a40 100644 --- a/docs/persistence.md +++ b/docs/persistence.md @@ -246,7 +246,29 @@ in-memory objects, and doesn't leave stale rows behind). `.db` file needs the file deleted by hand during dev, not an automatic wipe. - `ThingRepository` reconstructs the entire stored world into memory on every `LoadTreeAsync`/`FindPlayerByUsernameAsync` call — fine at hand-built-hub scale, - not yet a scoped/paginated load suitable for a large world. + not yet a scoped/paginated load suitable for a large world. Surfaced again + (not newly discovered, but discussed in more depth) while reviewing + [ADR-0009](adr/0009-world-building-olc-command-surface.md)'s `tunnel` + command: `GameDbContext` explicitly `Ignore()`s every `Thing`/`Behavior` + reference property (`Parent`, `Children`, `ExitBehavior.Destination`), so + EF Core's own relationship/`Include`-based scoped-loading features aren't + available here at all — the full-graph load is `ReconstructAllAsync`'s own + hand-written choice, not something EF Core requires. **This is a + persistence-layer/loading-strategy concern, separable from the + `Thing`/`Behavior` domain model itself** — nothing about composing game + objects from behaviors requires eager-loading the whole world; `Engine` + and every ruleset only ever depend on the `IThingRepository`/`IWorld` + interfaces, not `ThingRepository`'s internals, so this is fixable without + touching the domain model above it. The likely fix direction, when this + is actually designed: MUDs are naturally spatially local (a player only + ever touches their current room plus adjacent rooms via exits), so a + lazy/regional load — fetch a `Thing` the first time something references + it, cache it while active — is the standard shape, not a rewrite of + `Thing`/`Behavior`. Not yet worth designing given today's world size; + revisit once there's a real deployment approaching actual user counts, or + Slice 9 (procedural frontier generation) makes the world large enough + that eager-loading stops being free. See + [PLAN-0001](plans/0001-wheelmud-reconciliation-roadmap.md). - Concurrent `SaveTreeAsync` calls (e.g. two players disconnecting at once) rely on SQLite's own single-writer file locking; not independently stress-tested. diff --git a/docs/plans/0001-wheelmud-reconciliation-roadmap.md b/docs/plans/0001-wheelmud-reconciliation-roadmap.md index e322766..9a55323 100644 --- a/docs/plans/0001-wheelmud-reconciliation-roadmap.md +++ b/docs/plans/0001-wheelmud-reconciliation-roadmap.md @@ -31,8 +31,9 @@ reconciliation effort stands. - [x] **Slice 3 — Permission/security-role model + moderation commands.** ADR-0005 Accepted, PLAN-0005 Done. See [PLAN-0005](0005-security-role-model-and-moderation-commands.md). -- [ ] **Slice 4 — World-building/OLC command surface.** Not yet designed; - bundles with Slice 3. +- [x] **Slice 4 — World-building/OLC command surface.** ADR-0009 + Accepted, PLAN-0009 Done. See + [PLAN-0009](0009-world-building-olc-command-surface.md). - [ ] **Slice 5 — Help system.** Not yet designed. - [ ] **Slice 6 — Player configuration commands.** Not yet designed. - [ ] **Slice 7 — Commerce/shops.** Not yet designed. @@ -42,6 +43,31 @@ reconciliation effort stands. - [ ] **WheelMUD's FTP server: rejected, recorded in ADR-0001.** No further action — checking this off just means the decision itself (not implementation) is settled once ADR-0001 is `Accepted`. +- [ ] **Slice 10 (not yet numbered in ADR-0001) — NPC/item spawning, + mob-respawn loops, loot tables.** Not yet designed. Surfaced during + Slice 4's design dive (ADR-0009), deliberately kept out of it — + maps to WheelMUD's `Clone`/`Spawn` admin actions, already flagged as + deferred by PLAN-0005 pending "item/NPC creation tooling," plus a + genuinely new (WheelMUD doesn't have one either) tick-driven respawn + timer and loot-table shape. Needs its own research/design pass + before it's added to ADR-0001's inventory table with a real number. +- [ ] **Slice 11 (not yet numbered in ADR-0001, not really a WheelMUD gap + at all) — scoped/lazy world loading.** Not yet designed. Surfaced + while discussing ADR-0009's `tunnel` room-lookup with the user, who + then walked this back to the bigger underlying question: `ThingRepository` + reconstructs the *entire* stored world into memory on every load — + fine today, won't scale to real user counts or a large world. A + persistence-layer/loading-strategy concern, separable from the + `Thing`/`Behavior` domain model itself (confirmed: `GameDbContext` + already `Ignore()`s every graph-reference property, so this isn't + even leaning on EF Core's relationship features today — it's + `ThingRepository`'s own hand-written full-reconstruct choice). Likely + fix direction: lazy/regional loading exploiting MUD spatial locality + (load a `Thing` when first referenced, cache while active), not a + domain-model rewrite. See [persistence.md](../persistence.md) Open + Items. Deliberately not designed now — no real world-size trigger yet; + revisit once there's an actual deployment approaching scale, or + Slice 9 makes the world big enough to matter. ## Critical files diff --git a/docs/plans/0009-world-building-olc-command-surface.md b/docs/plans/0009-world-building-olc-command-surface.md new file mode 100644 index 0000000..a8b9664 --- /dev/null +++ b/docs/plans/0009-world-building-olc-command-surface.md @@ -0,0 +1,243 @@ +# [PLAN-0009] World-Building/OLC Command Surface + +**Implements:** [ADR-0009](../adr/0009-world-building-olc-command-surface.md) + +**Status:** Done + +**Last updated:** 2026-07-23 + +## Goal + +A `MinorBuilder`-gated `dig ` / `tunnel +` / `describe ` command set works end-to-end +over real Telnet: `dig` creates a new room and a two-way exit from the +builder's current room; `tunnel` wires the same kind of two-way exit +between the current room and an already-existing room found by name; +`describe` sets the current room's description. All three persist +immediately and survive a restart. + +## Scope + +Per ADR-0009's Decision Outcome. In scope: the three commands above, the +shared `Connect`-equivalent exit-wiring helper extracted from +`BasicWorldBuilder` into `SharpMud.Engine`, room lookup by exact `Name`. + +Explicitly deferred (per ADR-0009): room/exit deletion (`undig`), a +`describe ` remote-targeting variant, any new room-id concept, +data-file-driven world content (Slice 9 territory / `world-model.md` +stage 2). + +## Tasks + +### Shared exit-wiring helper + +- [x] New `src/SharpMud.Engine/Behaviors/RoomConnector.cs` — a static + `Connect(IWorld world, Thing a, Thing b, Direction direction)` moved + from `BasicWorldBuilder.Connect`: creates two `Thing`s (one + `ExitBehavior` each, using `direction` and `direction.Opposite()`), + adds each as a child of the room it exits from, registers both in + `world`. +- [x] `src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs`: removed its + private `Connect` method, calls the new shared helper instead — no + behavior change. +- [x] `samples/SharpMud.Samples.Classic/HubWorldBuilder.cs`: same dedup — + it has its own identical private `Connect`, missed in the original + ADR-0009 text (which only named `BasicWorldBuilder`); this is the + world the Classic/Telnet sample actually boots, so it's the one that + matters for this plan's manual verification. Removed its `Connect`, + calls `RoomConnector.Connect` instead — no behavior change. + +### New commands (`src/SharpMud.Engine/Commands/Builtin/Builder/`) + +All three take `IThingRepository` via constructor injection (same shape as +Slice 3's admin commands). All three, after mutating the tree, walk +`ctx.CurrentRoom` up via `.Parent` to the root `Thing` (`Parent == null`) +and call `repository.SaveTreeAsync(root, ct)` — **not** +`SaveTreeAsync(ctx.CurrentRoom, ct)`, since `dig`'s new room (and its +reverse exit) live outside `ctx.CurrentRoom`'s own subtree as siblings, +not descendants; only a save rooted above both rooms captures everything +that changed. + +- [x] `DigCommand` (`MinorBuilder`) — usage `dig `. Parses `Args[0]` as a `Direction`; rest of `Args` joined as + the new room's name. Creates the room + `RoomBehavior()` + (`Description` left `""`), attached as a child of + `ctx.CurrentRoom.Parent`, registered in `ctx.World`. Calls + `RoomConnector.Connect` between `ctx.CurrentRoom` and the new room. +- [x] `TunnelCommand` (`MinorBuilder`) — usage `tunnel + `. Looks up the destination room via + `BuilderCommandHelpers.FindRoomsByName` (exact `Name` match, + `OrdinalIgnoreCase` — matches the existing convention used + throughout `Commands/Builtin/Admin/*` and `GiveCommand`/ + `ObjectMatcher`, verified directly rather than assumed). Rejects + zero matches, more than one match, and self-tunnel. Saves both the + current room's tree root and the destination's (they may differ if + there's more than one area). +- [x] `DescribeCommand` (`MinorBuilder`) — usage `describe `. Sets + `ctx.CurrentRoom.Description` to the rest-of-line text (empty + rejected via `CommandGuards.RequireArgsAsync`). +- [x] `BuilderCommandHelpers` + (`src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommandHelpers.cs`) + — `TryParseDirection`, `FindRoomsByName`, `FindRoot`, shared by + `DigCommand`/`TunnelCommand`/`DescribeCommand`. Mirrors + `AdminCommandHelpers`'s shape (`internal static class`, no + `IThingRepository` dependency — each command holds its own + repository via its own constructor and never threads it into the + helper). +- [x] Register all three via `RegisterWithRole` in + `BuilderCommands.RegisterAll(registry, repository)` + (`src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommands.cs` + — mirrors `AdminCommands`'s shape exactly). Called from the same + `registerConsumerCommands` callback in + `samples/SharpMud.Samples.Classic/Program.cs` that already calls + `AdminCommands.RegisterAll`, resolving `IThingRepository` from the + same `IServiceProvider`. + +### Docs + +- [x] `docs/commands.md`: describe `dig`/`tunnel`/`describe` as current + state in the V1 Verb List, replacing the "builder/OLC verbs + excluded" note; link ADR-0009. +- [x] `docs/world-model.md`: updated "Content Authoring Evolution" — + stage 3 marked implemented, stage 2 explicitly noted as not a + prerequisite for it after all. +- [x] `SPEC.md`: updated its own (near-duplicate) "Content authoring + evolution" list and phase-9 status the same way; added a new + Deferred/Open Item for the NPC/item-spawning gap surfaced during + this ADR's scoping discussion. +- [x] `docs/adr/README.md` / `docs/plans/README.md`: index rows added for + ADR-0009 (`Accepted`)/PLAN-0009 (`In Progress`). +- [x] `docs/plans/0001-wheelmud-reconciliation-roadmap.md`: check off + Slice 4. + +## Critical files + +New: +- `src/SharpMud.Engine/Behaviors/RoomConnector.cs` +- `src/SharpMud.Engine/Commands/Builtin/Builder/DigCommand.cs` +- `src/SharpMud.Engine/Commands/Builtin/Builder/TunnelCommand.cs` +- `src/SharpMud.Engine/Commands/Builtin/Builder/DescribeCommand.cs` +- `src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommands.cs` +- `src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommandHelpers.cs` +- `tests/SharpMud.Engine.Tests/Behaviors/RoomConnectorTests.cs` +- `tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/DigCommandTests.cs` +- `tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/TunnelCommandTests.cs` +- `tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/DescribeCommandTests.cs` +- `tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/BuilderCommandsTests.cs` + +Modified: +- `src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs` (`Connect` extracted) +- `samples/SharpMud.Samples.Classic/HubWorldBuilder.cs` (same extraction — + missed in the original ADR-0009 text) +- `samples/SharpMud.Samples.Classic/Program.cs` +- `docs/commands.md`, `docs/world-model.md`, `SPEC.md`, + `docs/adr/README.md`, `docs/plans/README.md`, + `docs/plans/0001-wheelmud-reconciliation-roadmap.md` + +## Test plan + +- Unit: shared `Connect` helper — given two rooms and a direction, + produces one `ExitBehavior` on each side with correct `Direction`/ + `Direction.Opposite()` and `Destination`, both registered in `world`, + both attached as children of the correct room. +- Unit: `DigCommand` — happy path (valid direction, non-empty name) + creates a new `Thing` with `RoomBehavior`, wires the two-way exit, + attaches the new room as a sibling (child of `ctx.CurrentRoom.Parent`, + not of `ctx.CurrentRoom` itself), calls `SaveTreeAsync` with the tree + root (not `ctx.CurrentRoom`). Invalid direction and empty name each + rejected with a clear message, no mutation. +- Unit: `TunnelCommand` — happy path connects `ctx.CurrentRoom` to a + found room; zero matches and multiple matches each rejected with a + clear, distinct message; self-tunnel (found room equals + `ctx.CurrentRoom`) rejected. +- Unit: `DescribeCommand` — sets `ctx.CurrentRoom.Description`; empty + text rejected, `Description` unchanged. +- Unit: `BuilderCommands.RegisterAll` — all three register via + `RegisterWithRole(_, SecurityRole.MinorBuilder)`, not `RegisterOpen`. +- Regression: `BasicWorldBuilder`'s existing world-boot tests (if any) + still pass unchanged after `Connect` moves to the shared helper — + confirms the extraction is behavior-preserving. + +## Verification + +**Done (2026-07-23)** — real manual check over Telnet against +`samples/SharpMud.Samples.Classic` (this repo's established pattern for +world/persistence-facing changes), scratch SQLite DB, `SHARPMUD_INITIAL_ADMIN` +bootstrap + a live `rolegrant minorbuilder` to reach +`MinorBuilder`. + +**Bug found and fixed during this pass, not caught by unit tests**: +`dig`/`tunnel` didn't check whether the origin room already had an exit in +the requested direction. `Town Square` already has all four cardinal exits +from `HubWorldBuilder`; `dig north Storage Shed` silently added a *second* +`north` exit-`Thing` rather than rejecting. `MoveCommand` resolves exits via +`.FirstOrDefault()`, so the new exit was shadowed by the pre-existing one — +the dug room became unreachable via `north`, and the room's exit listing +started showing `north` twice. Fixed by adding +`BuilderCommandHelpers.HasExit(Thing, Direction)`, checked by both +`DigCommand` (origin only) and `TunnelCommand` (origin *and* the +destination's opposite direction, since that side can be occupied too), +with regression tests added for all three cases +(`DigCommandTests.ExecuteAsync_RejectsDirectionAlreadyOccupied_WithoutMutating`, +`TunnelCommandTests.ExecuteAsync_RejectsWhenOriginDirectionAlreadyOccupied`, +`TunnelCommandTests.ExecuteAsync_RejectsWhenDestinationOppositeDirectionAlreadyOccupied`). +Re-verified live after the fix. + +1. `dig up Storage Shed` from Town Square (its four cardinal directions + were already occupied by the hub's own rooms — confirms the fix + actually engages, not just an untested code path): room created, + reachable via `up`, `down` returns to Town Square, `Town Square`'s exit + list shows `up` exactly once. +2. `describe` the new room while standing in it; `look` immediately shows + the new description — and, after the fix, this correctly targets the + room the builder actually walked into (session 1's pre-fix run had + this land on the wrong room entirely, a direct symptom of the shadowed + -exit bug above). +3. `tunnel down Old Well` from Town Square to an already-existing hub + room found by name: succeeds, `Old Well`'s exit list shows `up` (its + only pre-existing exit was `east`, back to Town Square — no + collision), reachable both directions. +4. Rejections confirmed live: `tunnel west Old Well` from Storage Shed + (`Old Well` already has an `east` exit, the opposite of `west`) → + `"Old Well already has an exit east."`; `tunnel south Storage Shed` + from Town Square (`south` already goes to `Southern Gate`) → + `"There's already an exit south from here."`; a nonexistent room name + → `"No room named X was found."`. +5. Restart the server against the same DB; confirmed via direct SQLite + inspection (`Things`/`Behaviors` tables) rather than a second Telnet + session, since a raw-socket scripted client kept racing the session's + own buffered output — every new room, its description, and both + directions of every new exit (correct `ExitDestinationId` shadow-FK on + both sides, no duplicates) were present exactly as created, alongside + every pre-existing `HubWorldBuilder` room/exit, unchanged. +6. Confirmed a pre-existing, unrelated crash (documented already in + PLAN-0005's own Verification section — the game loop's `WanderManager` + tick broadcasting to a dead socket) is not something this slice + introduced or worsened; hit it once by disconnecting a raw script + mid-session, not through any dig/tunnel/describe path. + +Not separately re-verified live (already covered by unit tests, and the +underlying mechanism is Slice 3's, not new here): a `Player` with no +builder role being rejected by all three commands — +`BuilderCommandsTests` confirms all three register via `RegisterWithRole`, +and `RoleGuardedCommand`'s gating behavior itself already has its own +Slice 3 test coverage. + +## Open questions / blockers + +- Exact rejection message wording is a placeholder, not a considered + final string (same open item Slice 3 flagged for its own commands). +- **Not this slice, tracked for later**: NPC/item spawning, mob-respawn + loops, and loot tables — a real, currently-undesigned gap surfaced + while scoping ADR-0009, deliberately kept out of it (see ADR-0009's + Negative Consequences). Not yet a numbered slice in + [PLAN-0001](0001-wheelmud-reconciliation-roadmap.md) — needs its own + research/design pass (WheelMUD's `Clone`/`Spawn` actions, plus + something WheelMUD itself doesn't have: an actual tick-driven respawn + timer and loot-table shape) before it earns a slice number. +- **Not this slice, deliberately deferred by choice, not oversight**: + expanding `BasicWorldBuilder`'s two-room starter world. Discussed during + this ADR's dive — kept as-is since it's explicitly documented as a + "quick-start default," not real game content, and this slice's `dig` + now gives an in-game path to grow it without enlarging the fixed + sample. diff --git a/docs/plans/README.md b/docs/plans/README.md index 76bf7da..91c923a 100644 --- a/docs/plans/README.md +++ b/docs/plans/README.md @@ -62,3 +62,4 @@ isn't settled yet — don't do that. | [0005](0005-security-role-model-and-moderation-commands.md) | [ADR-0005](../adr/0005-security-role-model-and-moderation-commands.md) | Done | | [0006](0006-nuget-package-distribution.md) | [ADR-0006](../adr/0006-nuget-package-distribution.md) | Done | | [0008](0008-ruleset-scaffolding-tier.md) | [ADR-0008](../adr/0008-ruleset-scaffolding-tier.md) | Done | +| [0009](0009-world-building-olc-command-surface.md) | [ADR-0009](../adr/0009-world-building-olc-command-surface.md) | Done | diff --git a/docs/world-model.md b/docs/world-model.md index 7470639..21cf979 100644 --- a/docs/world-model.md +++ b/docs/world-model.md @@ -83,11 +83,23 @@ when a move target doesn't exist yet. ## Content Authoring Evolution (not all v1) -1. Hardcoded rooms in C# (bootstrap only, to get the loop working). -2. Data-driven world files (JSON/YAML) loaded at startup — natural next step, - also the foundation for in-game building commands later. -3. In-game building commands (`@dig`, `@describe`, etc.) writing to the same - data model. +1. Hardcoded rooms in C# (bootstrap only, to get the loop working) — still + how every world's starting content is authored today + (`HubWorldBuilder`/`BasicWorldBuilder`). +2. Data-driven world files (JSON/YAML) loaded at startup — **not yet + implemented**; still a natural next step if hardcoded content stops + scaling, but turned out not to be a prerequisite for stage 3 below. +3. In-game building commands (`dig `, `tunnel + `, `describe `) writing directly to the live + `Thing` tree — **implemented**, without needing stage 2's file format + first (a runtime command mutates the same in-memory/persisted model + stage 1's rooms already use). Role-gated at + `SecurityRole.MinorBuilder`; see + [ADR-0009](adr/0009-world-building-olc-command-surface.md) and + [commands.md](commands.md)'s V1 Verb List. Deliberately doesn't cover + NPC/item spawning, mob-respawn loops, or loot tables — a separate, + still-undesigned gap (see + [PLAN-0001](plans/0001-wheelmud-reconciliation-roadmap.md)'s Slice 10). ## Sequence: Procedural Frontier Room Generated diff --git a/docsite/docs/customizing.md b/docsite/docs/customizing.md index 928b715..148a92e 100644 --- a/docsite/docs/customizing.md +++ b/docsite/docs/customizing.md @@ -86,6 +86,11 @@ register it inside whichever ruleset-registration callback you're already using — see [Rulesets](rulesets.md#putting-it-together) for why that has to be one callback, not several independent calls. +`SharpMud.Engine` also ships two ready-made, opt-in command sets built on +this same registration mechanism — moderation (`boot`/`mute`/`ban`/...) and +world-building (`dig`/`tunnel`/`describe`) — see +[Moderation & World Building](moderation-and-world-building.md). + ## What's next Planned additions to this page: a deeper look at the `Thing`/`Behavior` diff --git a/docsite/docs/moderation-and-world-building.md b/docsite/docs/moderation-and-world-building.md new file mode 100644 index 0000000..8565b39 --- /dev/null +++ b/docsite/docs/moderation-and-world-building.md @@ -0,0 +1,123 @@ +# Moderation & World Building + +`SharpMud.Engine` ships two optional, ready-made command sets on top of the +same underlying mechanism: a security-role system that gates who can run +what. Neither is registered for you automatically — you opt into each one +explicitly, the same way you'd register your own `ICommand`s (see +[Customizing sharp-mud](customizing.md#adding-your-own-command)). + +- **Moderation** (`boot`, `mute`/`unmute`, `announce`, `ban`/`unban`, + `rolegrant`/`rolerevoke`) — day-to-day admin tooling for a live server. +- **World building** (`dig`, `tunnel`, `describe`) — lets a trusted player + extend the world from inside the game, no redeploy required. + +## `SecurityRole`: the mechanism both sets are built on + +`SecurityRole` is a `[Flags]` enum on `PlayerBehavior.Roles` — a player can +hold more than one role at once. Every new character starts as `Player`. +The two tiers relevant here: + +| Tier | Implies | Unlocks | +|---|---|---| +| `MinorAdmin` | `Player` | `boot`, `mute`, `unmute`, `announce` | +| `FullAdmin` | `MinorAdmin`, `Player` | everything `MinorAdmin` can do, plus `ban`, `unban`, `rolegrant`, `rolerevoke` | +| `MinorBuilder` | — | `dig`, `tunnel`, `describe` | +| `FullBuilder` | `MinorBuilder` | (reserved for a future builder-tier command; no consumer yet) | + +"Implies" is real: granting `FullAdmin` also grants `MinorAdmin` and +`Player` in the same call, so a `FullAdmin` can immediately run every +`MinorAdmin` command too — you never have to grant both separately. + +Commands aren't gated by a guard clause you have to remember to write. +`ICommandRegistry` only exposes two ways to register a command: + +```csharp +void RegisterOpen(ICommand command); // anyone can run it +void RegisterWithRole(ICommand command, SecurityRole requiredRole); // gated +``` + +`RegisterWithRole` wraps your command in a decorator that checks the +actor's `Roles` before it ever runs — there's no third, silent way to +register something without declaring its access level. + +### Bootstrapping the first admin + +Since granting a role itself requires already holding `FullAdmin`, nothing +in-game can produce the *first* one. Set the `SHARPMUD_INITIAL_ADMIN` +environment variable to a username, and that character is granted +`FullAdmin` automatically the moment they log in — whether that's a brand +-new character or an existing one, on this boot or any future one. Wire it +up once in your composition root: + +```csharp +var env = new Dictionary +{ + ["SHARPMUD_INITIAL_ADMIN"] = Environment.GetEnvironmentVariable("SHARPMUD_INITIAL_ADMIN"), +}; +var hostOptions = SharpMudHostOptions.Parse(env); +app.Services.AddSingleton(hostOptions); +``` + +It's safe to leave the variable set permanently — granting an already-held +role is a no-op, not an error. + +## Registering the command sets + +Both command sets need `IThingRepository` (most of their commands look up +an offline target, or save a newly-created room). Resolve it once inside +whichever callback you already pass into `AddSharpMudRuleset`/ +`AddSharpMudRpgRuleset` and register both from there: + +```csharp +app.Services.AddSharpMudRpgRuleset((sp, registry) => +{ + var repository = sp.GetRequiredService(); + AdminCommands.RegisterAll(registry, repository); + BuilderCommands.RegisterAll(registry, repository); +}); +``` + +!!! warning "One registration callback, not several" + `AddSharpMudRuleset`/`AddSharpMudRpgRuleset` only calls your command + -registration callback once. Calling either method a second time + replaces the first registration instead of adding to it — always + register everything (built-ins are automatic; admin, builder, and + your own commands) from inside the one callback. + +### Moderation commands + +| Command | Role | Notes | +|---|---|---| +| `boot ` | `MinorAdmin` | Disconnects a currently-online player. | +| `mute` / `unmute ` | `MinorAdmin` | Blocks/restores `say`/`emote` for that player. | +| `announce ` | `MinorAdmin` | Broadcasts to every connected player. | +| `ban` / `unban ` | `FullAdmin` | Blocks/restores login entirely; an already-connected banned player is also disconnected immediately. | +| `rolegrant` / `rolerevoke ` | `FullAdmin` | Grants/revokes a role. `all`/`none` are rejected — those are sentinel values, not real assignable tiers. | + +### World-building commands + +| Command | Role | Notes | +|---|---|---| +| `dig ` | `MinorBuilder` | Creates a brand-new room and wires a two-way exit from the builder's current room to it. | +| `tunnel ` | `MinorBuilder` | Wires a two-way exit between the current room and an *already-existing* room, found by exact name. | +| `describe ` | `MinorBuilder` | Sets the current room's description. | + +Both `dig` and `tunnel` reject the direction if the current room already +has an exit that way (and, for `tunnel`, if the destination room already +has an exit going back the opposite way) — you'll never end up with two +exits silently fighting over the same direction. + +There's no room-deletion command, and no NPC/item spawning — see +[ADR-0009](https://github.com/LayeredCraft/sharp-mud/blob/main/docs/adr/0009-world-building-olc-command-surface.md) +for what's deliberately out of scope. + +## Further reading + +The full design rationale for both command sets — why a hand-rolled +decorator instead of an attribute+reflection scheme, why `SecurityRole` +adopts WheelMUD's full role set even though most of it has no consumer +yet, and why `dig`/`tunnel` look up rooms by name instead of a numeric id +— is recorded in the sharp-mud repo's ADRs: + +- [ADR-0005](https://github.com/LayeredCraft/sharp-mud/blob/main/docs/adr/0005-security-role-model-and-moderation-commands.md) — Security Role Model + Moderation Commands +- [ADR-0009](https://github.com/LayeredCraft/sharp-mud/blob/main/docs/adr/0009-world-building-olc-command-surface.md) — World-Building/OLC Command Surface diff --git a/docsite/zensical.toml b/docsite/zensical.toml index df30b07..5451f9f 100644 --- a/docsite/zensical.toml +++ b/docsite/zensical.toml @@ -13,7 +13,8 @@ nav = [ { "Home" = "index.md" }, { "Getting Started" = "getting-started.md" }, { "Rulesets" = "rulesets.md" }, - { "Customizing sharp-mud" = "customizing.md" } + { "Customizing sharp-mud" = "customizing.md" }, + { "Moderation & World Building" = "moderation-and-world-building.md" } ] [project.markdown_extensions.attr_list] diff --git a/samples/SharpMud.Samples.Classic/HubWorldBuilder.cs b/samples/SharpMud.Samples.Classic/HubWorldBuilder.cs index d3d67c4..9b8495e 100644 --- a/samples/SharpMud.Samples.Classic/HubWorldBuilder.cs +++ b/samples/SharpMud.Samples.Classic/HubWorldBuilder.cs @@ -35,11 +35,11 @@ public static (World World, Thing StartingRoom) Build() var generalStore = CreateRoom(world, area, "General Store", "Shelves crowded with dusty goods line the walls of this cramped little shop."); - Connect(world, townSquare, marketStreet, Direction.North); - Connect(world, townSquare, templeSteps, Direction.East); - Connect(world, townSquare, southernGate, Direction.South); - Connect(world, townSquare, oldWell, Direction.West); - Connect(world, marketStreet, generalStore, Direction.East); + RoomConnector.Connect(world, townSquare, marketStreet, Direction.North); + RoomConnector.Connect(world, townSquare, templeSteps, Direction.East); + RoomConnector.Connect(world, townSquare, southernGate, Direction.South); + RoomConnector.Connect(world, townSquare, oldWell, Direction.West); + RoomConnector.Connect(world, marketStreet, generalStore, Direction.East); var caveRat = new Thing { Id = ThingId.New(), Name = "cave rat" }; caveRat.Behaviors.Add(new NpcBehavior()); @@ -131,21 +131,6 @@ private static Thing CreateRoom(World world, Thing area, string name, string des return room; } - // Two exit Things per connection - one per direction (docs/engine-vs-ruleset.md - // Decisions), each a child of the room it exits from. - private static void Connect(World world, Thing a, Thing b, Direction direction) - { - var aToB = new Thing { Id = ThingId.New(), Name = direction.ToDisplayString() }; - aToB.Behaviors.Add(new ExitBehavior { Direction = direction, Destination = b }); - a.Add(aToB); - world.Register(aToB); - - var bToA = new Thing { Id = ThingId.New(), Name = direction.Opposite().ToDisplayString() }; - bToA.Behaviors.Add(new ExitBehavior { Direction = direction.Opposite(), Destination = a }); - b.Add(bToA); - world.Register(bToA); - } - private static Thing CreateItem(World world, string name, string description, EquipSlot? slot) { var item = new Thing { Id = ThingId.New(), Name = name, Description = description }; diff --git a/samples/SharpMud.Samples.Classic/Program.cs b/samples/SharpMud.Samples.Classic/Program.cs index 29cfd4a..815dd30 100644 --- a/samples/SharpMud.Samples.Classic/Program.cs +++ b/samples/SharpMud.Samples.Classic/Program.cs @@ -6,6 +6,7 @@ using SharpMud.Adapters.Cli; using SharpMud.Adapters.Telnet; using SharpMud.Engine.Commands.Builtin.Admin; +using SharpMud.Engine.Commands.Builtin.Builder; using SharpMud.Engine.Core; using SharpMud.Hosting; using SharpMud.Persistence; @@ -68,10 +69,15 @@ // as both itself and ITickable, the dice service, its own // IBehaviorMappingContributor, and the kill/attack/flee commands) - see // docs/adr/0008-ruleset-scaffolding-tier.md. The registerConsumerCommands -// callback wires AdminCommands (ADR-0005 moderation) - Rpg has no notion of -// security roles itself, so this is Classic's own composition-root concern. +// callback wires AdminCommands (ADR-0005 moderation) and BuilderCommands +// (ADR-0009 world-building/OLC) - Rpg has no notion of security roles +// itself, so both are Classic's own composition-root concern. builder.Services.AddSharpMudRpgRuleset((sp, registry) => - AdminCommands.RegisterAll(registry, sp.GetRequiredService())); +{ + var repository = sp.GetRequiredService(); + AdminCommands.RegisterAll(registry, repository); + BuilderCommands.RegisterAll(registry, repository); +}); // Transport mode: SHARPMUD_MODE/SHARPMUD_TELNET_PORT/--telnet, same // precedence as before (CLI arg wins over env var) - this is now the diff --git a/src/SharpMud.Engine/Behaviors/RoomConnector.cs b/src/SharpMud.Engine/Behaviors/RoomConnector.cs new file mode 100644 index 0000000..c50e1d7 --- /dev/null +++ b/src/SharpMud.Engine/Behaviors/RoomConnector.cs @@ -0,0 +1,29 @@ +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Behaviors; + +/// +/// Wires a two-way exit between two rooms - one +/// per direction of travel (see 's own remarks on +/// why there's no single bidirectional exit type), each registered in the +/// world and attached as a child of the room it exits from. Shared by +/// world-boot content (BasicWorldBuilder) and the runtime +/// dig/tunnel commands (ADR-0009) - both need to build the +/// exact same pair of Things, and this is the one place that knows how. +/// +public static class RoomConnector +{ + /// Connects to via , and back to via its opposite. + public static void Connect(IWorld world, Thing a, Thing b, Direction direction) + { + var aToB = new Thing { Id = ThingId.New(), Name = direction.ToDisplayString() }; + aToB.Behaviors.Add(new ExitBehavior { Direction = direction, Destination = b }); + a.Add(aToB); + world.Register(aToB); + + var bToA = new Thing { Id = ThingId.New(), Name = direction.Opposite().ToDisplayString() }; + bToA.Behaviors.Add(new ExitBehavior { Direction = direction.Opposite(), Destination = a }); + b.Add(bToA); + world.Register(bToA); + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommandHelpers.cs b/src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommandHelpers.cs new file mode 100644 index 0000000..c8ec7a1 --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommandHelpers.cs @@ -0,0 +1,62 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Commands.Builtin.Builder; + +// Shared by dig/tunnel - both parse a leading direction argument the same +// way, and both eventually need "walk up to the tree root to save +// everything that changed" (a dug room, and its own reverse exit, live +// outside the current room's own subtree as siblings under the same area, +// not as its descendants - see ADR-0009). +internal static class BuilderCommandHelpers +{ + /// Parses a direction name (e.g. "north", "ne") case-insensitively. Accepts the same names produces, plus the MoveCommand short aliases. + public static bool TryParseDirection(string text, out Direction direction) + { + switch (text.ToLowerInvariant()) + { + case "north" or "n": direction = Direction.North; return true; + case "south" or "s": direction = Direction.South; return true; + case "east" or "e": direction = Direction.East; return true; + case "west" or "w": direction = Direction.West; return true; + case "northeast" or "ne": direction = Direction.NorthEast; return true; + case "northwest" or "nw": direction = Direction.NorthWest; return true; + case "southeast" or "se": direction = Direction.SouthEast; return true; + case "southwest" or "sw": direction = Direction.SouthWest; return true; + case "up" or "u": direction = Direction.Up; return true; + case "down" or "d": direction = Direction.Down; return true; + default: direction = default; return false; + } + } + + /// Finds every registered room whose matches (case-insensitive, exact) - a caller decides how to handle zero or more than one match. + public static IReadOnlyList FindRoomsByName(IWorld world, string name) => + world.AllWithBehavior() + .Where(r => string.Equals(r.Name, name, StringComparison.OrdinalIgnoreCase)) + .ToList(); + + /// + /// Whether already has an exit in + /// - MoveCommand resolves exits via + /// FirstOrDefault, so wiring a second exit in an already-occupied + /// direction doesn't fail loudly; it silently shadows the new one behind + /// whichever exit was registered first, and the room's exit listing + /// starts showing the same direction twice. and + /// both check this on the origin room before + /// connecting anything - also checks it + /// against the destination room's opposite direction, since that side + /// can be occupied too. + /// + public static bool HasExit(Thing room, Direction direction) => + room.Children.Any(c => c.FindBehavior()?.Direction == direction); + + /// Walks up to the tree root (the first ancestor with no parent) - the node needs so a save captures every changed Thing, not just the current room's own subtree. + public static Thing FindRoot(Thing thing) + { + var current = thing; + while (current.Parent is { } parent) + current = parent; + + return current; + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommands.cs b/src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommands.cs new file mode 100644 index 0000000..1ece6bd --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommands.cs @@ -0,0 +1,21 @@ +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Commands.Builtin.Builder; + +/// +/// Registers the world-building/OLC command set (ADR-0009) - +/// dig/tunnel/describe, all at . Not called automatically by - a consumer calls this themselves +/// (passing their own ), the same opt-in +/// shape already uses. +/// +public static class BuilderCommands +{ + public static void RegisterAll(ICommandRegistry registry, IThingRepository repository) + { + registry.RegisterWithRole(new DigCommand(repository), SecurityRole.MinorBuilder); + registry.RegisterWithRole(new TunnelCommand(repository), SecurityRole.MinorBuilder); + registry.RegisterWithRole(new DescribeCommand(repository), SecurityRole.MinorBuilder); + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Builder/DescribeCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Builder/DescribeCommand.cs new file mode 100644 index 0000000..e5df3d8 --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Builder/DescribeCommand.cs @@ -0,0 +1,37 @@ +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Commands.Builtin.Builder; + +/// +/// The describe <text> command () - sets the description of the +/// builder's current room to the rest of the line, saved immediately. +/// Only targets the current room; there's no remote-room variant (matches +/// /'s current-room-only +/// scope, per ADR-0009). +/// +public sealed class DescribeCommand : ICommand +{ + private readonly IThingRepository _repository; + + public DescribeCommand(IThingRepository repository) + { + _repository = repository; + } + + public string Verb => "describe"; + public IReadOnlyList Aliases { get; } = []; + + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + if (await CommandGuards.RequireArgsAsync(ctx, "Describe the room as what?", ct)) + return; + + ctx.CurrentRoom.Description = string.Join(' ', ctx.Args); + + var root = BuilderCommandHelpers.FindRoot(ctx.CurrentRoom); + await _repository.SaveTreeAsync(root, ct); + + await ctx.Session.WriteLineAsync("Description updated.", ct); + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Builder/DigCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Builder/DigCommand.cs new file mode 100644 index 0000000..9cc0abc --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Builder/DigCommand.cs @@ -0,0 +1,66 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Commands.Builtin.Builder; + +/// +/// The dig <direction> <new room name> command () - creates a new room (empty +/// description, set afterward via ) as a +/// sibling of the builder's current room, and wires a two-way exit between +/// them via . +/// +public sealed class DigCommand : ICommand +{ + private readonly IThingRepository _repository; + + public DigCommand(IThingRepository repository) + { + _repository = repository; + } + + public string Verb => "dig"; + public IReadOnlyList Aliases { get; } = []; + + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + if (ctx.Args.Count < 2) + { + await ctx.Session.WriteLineAsync("Usage: dig ", ct); + return; + } + + if (!BuilderCommandHelpers.TryParseDirection(ctx.Args[0], out var direction)) + { + await ctx.Session.WriteLineAsync($"'{ctx.Args[0]}' isn't a direction.", ct); + return; + } + + if (BuilderCommandHelpers.HasExit(ctx.CurrentRoom, direction)) + { + await ctx.Session.WriteLineAsync($"There's already an exit {direction.ToDisplayString()} from here.", ct); + return; + } + + var name = string.Join(' ', ctx.Args.Skip(1)); + + if (ctx.CurrentRoom.Parent is not { } area) + { + await ctx.Session.WriteLineAsync("The current room has nowhere to attach a new room to.", ct); + return; + } + + var room = new Thing { Id = ThingId.New(), Name = name, Description = "" }; + room.Behaviors.Add(new RoomBehavior()); + area.Add(room); + ctx.World.Register(room); + + RoomConnector.Connect(ctx.World, ctx.CurrentRoom, room, direction); + + var root = BuilderCommandHelpers.FindRoot(ctx.CurrentRoom); + await _repository.SaveTreeAsync(root, ct); + + await ctx.Session.WriteLineAsync( + $"You dig {direction.ToDisplayString()}, creating {name}.", ct); + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Builder/TunnelCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Builder/TunnelCommand.cs new file mode 100644 index 0000000..cdb330a --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Builder/TunnelCommand.cs @@ -0,0 +1,89 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Commands.Builtin.Builder; + +/// +/// The tunnel <direction> <existing room name> command +/// () - wires a two-way exit (via +/// ) between the builder's current room +/// and an already-existing room found by exact +/// match. Unlike , creates nothing new. +/// +public sealed class TunnelCommand : ICommand +{ + private readonly IThingRepository _repository; + + public TunnelCommand(IThingRepository repository) + { + _repository = repository; + } + + public string Verb => "tunnel"; + public IReadOnlyList Aliases { get; } = []; + + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + if (ctx.Args.Count < 2) + { + await ctx.Session.WriteLineAsync("Usage: tunnel ", ct); + return; + } + + if (!BuilderCommandHelpers.TryParseDirection(ctx.Args[0], out var direction)) + { + await ctx.Session.WriteLineAsync($"'{ctx.Args[0]}' isn't a direction.", ct); + return; + } + + var name = string.Join(' ', ctx.Args.Skip(1)); + var matches = BuilderCommandHelpers.FindRoomsByName(ctx.World, name); + + if (matches.Count == 0) + { + await ctx.Session.WriteLineAsync($"No room named {name} was found.", ct); + return; + } + + if (matches.Count > 1) + { + await ctx.Session.WriteLineAsync($"Ambiguous: {matches.Count} rooms named {name} were found.", ct); + return; + } + + var destination = matches[0]; + if (ReferenceEquals(destination, ctx.CurrentRoom)) + { + await ctx.Session.WriteLineAsync("You can't tunnel a room to itself.", ct); + return; + } + + if (BuilderCommandHelpers.HasExit(ctx.CurrentRoom, direction)) + { + await ctx.Session.WriteLineAsync($"There's already an exit {direction.ToDisplayString()} from here.", ct); + return; + } + + if (BuilderCommandHelpers.HasExit(destination, direction.Opposite())) + { + await ctx.Session.WriteLineAsync( + $"{destination.Name} already has an exit {direction.Opposite().ToDisplayString()}.", ct); + return; + } + + RoomConnector.Connect(ctx.World, ctx.CurrentRoom, destination, direction); + + // The destination room isn't necessarily in the same tree as the + // current room (multiple areas) - save both roots, not just one, + // so the destination's new reverse exit isn't silently dropped. + var currentRoot = BuilderCommandHelpers.FindRoot(ctx.CurrentRoom); + await _repository.SaveTreeAsync(currentRoot, ct); + + var destinationRoot = BuilderCommandHelpers.FindRoot(destination); + if (!ReferenceEquals(destinationRoot, currentRoot)) + await _repository.SaveTreeAsync(destinationRoot, ct); + + await ctx.Session.WriteLineAsync( + $"You tunnel {direction.ToDisplayString()} to {destination.Name}.", ct); + } +} diff --git a/src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs b/src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs index 81532ca..f92313c 100644 --- a/src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs +++ b/src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs @@ -37,7 +37,7 @@ public sealed class BasicWorldBuilder : IWorldBuilder var watchtower = CreateRoom(world, area, "Old Watchtower", "A crumbling stone watchtower, long abandoned. Something rustles nearby."); - Connect(world, clearing, watchtower, Direction.North); + RoomConnector.Connect(world, clearing, watchtower, Direction.North); var boar = new Thing { Id = ThingId.New(), Name = "wild boar" }; boar.Behaviors.Add(new NpcBehavior()); @@ -69,19 +69,4 @@ private static Thing CreateRoom(World world, Thing area, string name, string des world.Register(room); return room; } - - // Two exit Things per connection - one per direction (docs/engine-vs-ruleset.md - // Decisions), each a child of the room it exits from. - private static void Connect(World world, Thing a, Thing b, Direction direction) - { - var aToB = new Thing { Id = ThingId.New(), Name = direction.ToDisplayString() }; - aToB.Behaviors.Add(new ExitBehavior { Direction = direction, Destination = b }); - a.Add(aToB); - world.Register(aToB); - - var bToA = new Thing { Id = ThingId.New(), Name = direction.Opposite().ToDisplayString() }; - bToA.Behaviors.Add(new ExitBehavior { Direction = direction.Opposite(), Destination = a }); - b.Add(bToA); - world.Register(bToA); - } } diff --git a/tests/SharpMud.Engine.Tests/Behaviors/RoomConnectorTests.cs b/tests/SharpMud.Engine.Tests/Behaviors/RoomConnectorTests.cs new file mode 100644 index 0000000..1519e8b --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Behaviors/RoomConnectorTests.cs @@ -0,0 +1,28 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Tests.Behaviors; + +public sealed class RoomConnectorTests +{ + [Fact] + public void Connect_WiresBothDirectionsAndRegistersBothExits() + { + var world = new World(); + var a = new Thing { Id = ThingId.New(), Name = "A" }; + var b = new Thing { Id = ThingId.New(), Name = "B" }; + + RoomConnector.Connect(world, a, b, Direction.North); + + var aToB = a.Children.Select(c => c.FindBehavior()).Single(e => e is not null); + aToB!.Direction.Should().Be(Direction.North); + aToB.Destination.Should().Be(b); + + var bToA = b.Children.Select(c => c.FindBehavior()).Single(e => e is not null); + bToA!.Direction.Should().Be(Direction.South); + bToA.Destination.Should().Be(a); + + world.GetThing(aToB.Parent!.Id).Should().Be(aToB.Parent); + world.GetThing(bToA.Parent!.Id).Should().Be(bToA.Parent); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/BuilderCommandsTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/BuilderCommandsTests.cs new file mode 100644 index 0000000..918734d --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/BuilderCommandsTests.cs @@ -0,0 +1,22 @@ +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin.Builder; +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Tests.Commands.Builtin.Builder; + +public sealed class BuilderCommandsTests +{ + [Fact] + public void RegisterAll_RegistersAllThreeCommandsAtMinorBuilder() + { + var registry = Substitute.For(); + var repository = Substitute.For(); + + BuilderCommands.RegisterAll(registry, repository); + + registry.Received(1).RegisterWithRole(Arg.Any(), SecurityRole.MinorBuilder); + registry.Received(1).RegisterWithRole(Arg.Any(), SecurityRole.MinorBuilder); + registry.Received(1).RegisterWithRole(Arg.Any(), SecurityRole.MinorBuilder); + registry.DidNotReceiveWithAnyArgs().RegisterOpen(default!); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/DescribeCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/DescribeCommandTests.cs new file mode 100644 index 0000000..4c0f6e1 --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/DescribeCommandTests.cs @@ -0,0 +1,53 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin.Builder; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands.Builtin.Builder; + +public sealed class DescribeCommandTests +{ + [Fact] + public async Task ExecuteAsync_SetsDescriptionAndSaves() + { + var repository = Substitute.For(); + var session = Substitute.For(); + var world = new World(); + var area = new Thing { Id = ThingId.New(), Name = "Area" }; + world.Register(area); + + var room = new Thing { Id = ThingId.New(), Name = "Room" }; + room.Behaviors.Add(new RoomBehavior()); + area.Add(room); + world.Register(room); + + var sut = new DescribeCommand(repository); + var ctx = new CommandContext(room, room, ["A", "dusty", "old", "room."], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + room.Description.Should().Be("A dusty old room."); + await repository.Received(1).SaveTreeAsync(area, Arg.Any()); + await session.Received(1).WriteLineAsync("Description updated.", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_SendsUsageMessage_WhenMissingArguments() + { + var repository = Substitute.For(); + var session = Substitute.For(); + var world = new World(); + var room = new Thing { Id = ThingId.New(), Name = "Room" }; + room.Behaviors.Add(new RoomBehavior()); + world.Register(room); + + var sut = new DescribeCommand(repository); + var ctx = new CommandContext(room, room, [], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync("Describe the room as what?", Arg.Any()); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, Arg.Any()); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/DigCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/DigCommandTests.cs new file mode 100644 index 0000000..71aaddb --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/DigCommandTests.cs @@ -0,0 +1,98 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin.Builder; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands.Builtin.Builder; + +public sealed class DigCommandTests +{ + private static (Thing Area, Thing Room, World World, ISession Session) MakeRoom() + { + var world = new World(); + var area = new Thing { Id = ThingId.New(), Name = "Area" }; + world.Register(area); + + var room = new Thing { Id = ThingId.New(), Name = "Starting Room" }; + room.Behaviors.Add(new RoomBehavior()); + area.Add(room); + world.Register(room); + + return (area, room, world, Substitute.For()); + } + + [Fact] + public async Task ExecuteAsync_CreatesRoomAndConnectsIt_OnHappyPath() + { + var repository = Substitute.For(); + var (area, room, world, session) = MakeRoom(); + + var sut = new DigCommand(repository); + var ctx = new CommandContext(room, room, ["north", "Storage", "Shed"], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + var newRoom = area.Children.First(c => c.Name == "Storage Shed"); + newRoom.HasBehavior().Should().BeTrue(); + newRoom.Parent.Should().Be(area, "a dug room is a sibling of the room it's dug from, not a child of it"); + + var exit = room.Children.Select(c => c.FindBehavior()).Single(e => e is not null); + exit!.Direction.Should().Be(Direction.North); + exit.Destination.Should().Be(newRoom); + + await repository.Received(1).SaveTreeAsync(area, Arg.Any()); + await session.Received(1).WriteLineAsync("You dig north, creating Storage Shed.", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_RejectsInvalidDirection_WithoutMutating() + { + var repository = Substitute.For(); + var (area, room, world, session) = MakeRoom(); + + var sut = new DigCommand(repository); + var ctx = new CommandContext(room, room, ["sideways", "Storage Shed"], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + area.Children.Should().ContainSingle(); // just the original room + await session.Received(1).WriteLineAsync("'sideways' isn't a direction.", Arg.Any()); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_RejectsDirectionAlreadyOccupied_WithoutMutating() + { + var repository = Substitute.For(); + var (area, room, world, session) = MakeRoom(); + var existingDestination = new Thing { Id = ThingId.New(), Name = "Existing" }; + existingDestination.Behaviors.Add(new RoomBehavior()); + area.Add(existingDestination); + world.Register(existingDestination); + RoomConnector.Connect(world, room, existingDestination, Direction.North); + + var sut = new DigCommand(repository); + var ctx = new CommandContext(room, room, ["north", "Storage", "Shed"], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + area.Children.Should().HaveCount(2, "just Starting Room and Existing - no new room was created"); + await session.Received(1).WriteLineAsync("There's already an exit north from here.", Arg.Any()); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_SendsUsageMessage_WhenMissingArguments() + { + var repository = Substitute.For(); + var (_, room, world, session) = MakeRoom(); + + var sut = new DigCommand(repository); + var ctx = new CommandContext(room, room, ["north"], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync("Usage: dig ", Arg.Any()); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/TunnelCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/TunnelCommandTests.cs new file mode 100644 index 0000000..27f305d --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/TunnelCommandTests.cs @@ -0,0 +1,149 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin.Builder; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands.Builtin.Builder; + +public sealed class TunnelCommandTests +{ + private static Thing MakeRoom(World world, Thing area, string name) + { + var room = new Thing { Id = ThingId.New(), Name = name }; + room.Behaviors.Add(new RoomBehavior()); + area.Add(room); + world.Register(room); + return room; + } + + [Fact] + public async Task ExecuteAsync_ConnectsToExistingRoom_OnHappyPath() + { + var repository = Substitute.For(); + var session = Substitute.For(); + var world = new World(); + var area = new Thing { Id = ThingId.New(), Name = "Area" }; + world.Register(area); + + var origin = MakeRoom(world, area, "Origin"); + var destination = MakeRoom(world, area, "Destination"); + + var sut = new TunnelCommand(repository); + var ctx = new CommandContext(origin, origin, ["east", "Destination"], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + var exit = origin.Children.Select(c => c.FindBehavior()).Single(e => e is not null); + exit!.Direction.Should().Be(Direction.East); + exit.Destination.Should().Be(destination); + + await repository.Received(1).SaveTreeAsync(area, Arg.Any()); + await session.Received(1).WriteLineAsync("You tunnel east to Destination.", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_RejectsNoMatch() + { + var repository = Substitute.For(); + var session = Substitute.For(); + var world = new World(); + var area = new Thing { Id = ThingId.New(), Name = "Area" }; + world.Register(area); + var origin = MakeRoom(world, area, "Origin"); + + var sut = new TunnelCommand(repository); + var ctx = new CommandContext(origin, origin, ["east", "Nowhere"], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync("No room named Nowhere was found.", Arg.Any()); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_RejectsAmbiguousMatch() + { + var repository = Substitute.For(); + var session = Substitute.For(); + var world = new World(); + var area = new Thing { Id = ThingId.New(), Name = "Area" }; + world.Register(area); + var origin = MakeRoom(world, area, "Origin"); + MakeRoom(world, area, "Duplicate"); + MakeRoom(world, area, "Duplicate"); + + var sut = new TunnelCommand(repository); + var ctx = new CommandContext(origin, origin, ["east", "Duplicate"], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync("Ambiguous: 2 rooms named Duplicate were found.", Arg.Any()); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_RejectsWhenOriginDirectionAlreadyOccupied() + { + var repository = Substitute.For(); + var session = Substitute.For(); + var world = new World(); + var area = new Thing { Id = ThingId.New(), Name = "Area" }; + world.Register(area); + var origin = MakeRoom(world, area, "Origin"); + var alreadyConnected = MakeRoom(world, area, "AlreadyConnected"); + var destination = MakeRoom(world, area, "Destination"); + RoomConnector.Connect(world, origin, alreadyConnected, Direction.East); + + var sut = new TunnelCommand(repository); + var ctx = new CommandContext(origin, origin, ["east", "Destination"], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + origin.Children.Count(c => c.FindBehavior() is not null).Should().Be(1); + await session.Received(1).WriteLineAsync("There's already an exit east from here.", Arg.Any()); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_RejectsWhenDestinationOppositeDirectionAlreadyOccupied() + { + var repository = Substitute.For(); + var session = Substitute.For(); + var world = new World(); + var area = new Thing { Id = ThingId.New(), Name = "Area" }; + world.Register(area); + var origin = MakeRoom(world, area, "Origin"); + var destination = MakeRoom(world, area, "Destination"); + var somewhereElse = MakeRoom(world, area, "SomewhereElse"); + RoomConnector.Connect(world, destination, somewhereElse, Direction.West); + + var sut = new TunnelCommand(repository); + var ctx = new CommandContext(origin, origin, ["east", "Destination"], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + origin.Children.Should().BeEmpty(); + await session.Received(1).WriteLineAsync("Destination already has an exit west.", Arg.Any()); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_RejectsSelfTunnel() + { + var repository = Substitute.For(); + var session = Substitute.For(); + var world = new World(); + var area = new Thing { Id = ThingId.New(), Name = "Area" }; + world.Register(area); + var origin = MakeRoom(world, area, "Origin"); + + var sut = new TunnelCommand(repository); + var ctx = new CommandContext(origin, origin, ["east", "Origin"], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync("You can't tunnel a room to itself.", Arg.Any()); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, Arg.Any()); + } +} From a4496273031ae4add44e67731af0d9fb11cd4dd1 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Fri, 24 Jul 2026 08:16:53 -0400 Subject: [PATCH 2/2] fix(engine): address PR #20 review feedback on dig/tunnel/describe Checks area.Add(room)'s bool return in DigCommand instead of assuming success - Thing.Add publishes a cancelable AddChildEvent, and proceeding regardless would register/connect/save a room that was never actually attached to the tree if anything ever vetoed it. Matches MoveCommand's existing precedent of checking Remove's return the same way. Dedups the "already an exit ... from here" rejection message (was identical, hand-copied in both DigCommand and TunnelCommand) into a shared BuilderCommandHelpers.OccupiedDirectionMessage. Fixes a factual error in the new docsite page's SecurityRole "Implies" table: MinorBuilder/FullBuilder's accumulation was documented as "-"/ "MinorBuilder" when SecurityRoleExtensions.ImpliedRoles actually gives "Player"/"MinorBuilder, Player" - would have told a reader granting MinorBuilder doesn't also grant Player, which is false. Defers (tracked in persistence.md Open Items + PLAN-0009, not fixed here) TunnelCommand's two SaveTreeAsync calls not being atomic together across a multi-area tunnel - currently unreachable since every existing world has exactly one area, and fixing it properly needs a repository- level multi-root atomic save, a bigger change than this PR's scope. Co-Authored-By: Claude Sonnet 5 --- docs/persistence.md | 16 +++++++ ...0009-world-building-olc-command-surface.md | 42 +++++++++++++++---- docsite/docs/moderation-and-world-building.md | 4 +- .../Builtin/Builder/BuilderCommandHelpers.cs | 10 +++++ .../Commands/Builtin/Builder/DigCommand.cs | 16 ++++++- .../Commands/Builtin/Builder/TunnelCommand.cs | 2 +- .../Builtin/Builder/DigCommandTests.cs | 23 ++++++++++ 7 files changed, 99 insertions(+), 14 deletions(-) diff --git a/docs/persistence.md b/docs/persistence.md index 7916a40..88eb220 100644 --- a/docs/persistence.md +++ b/docs/persistence.md @@ -272,3 +272,19 @@ in-memory objects, and doesn't leave stale rows behind). - Concurrent `SaveTreeAsync` calls (e.g. two players disconnecting at once) rely on SQLite's own single-writer file locking; not independently stress-tested. +- `TunnelCommand` (ADR-0009) can save two independent tree roots — the + current room's and the destination room's, when a `tunnel` crosses + between two different areas — via two separate `SaveTreeAsync` calls, + not one atomic operation. Caught in PR review: if the first call + succeeds and the second throws (DB error, cancellation, crash), a + restart could load a durably one-way exit — the origin room's new exit + persisted, the destination's reverse exit back never written. Currently + unreachable in practice (every world built so far — + `BasicWorldBuilder`/`HubWorldBuilder` — has exactly one area, so the two + roots are always the same object and the second call never actually + fires), but a real gap once/if a world has more than one area. Fixing it + properly needs `IThingRepository` to grow a multi-root atomic save (one + shared transaction across both trees) — a repository-API change + deliberately not bundled into that PR; revisit alongside Slice 9 + (procedural generation) if/when a multi-area world actually exists. See + [PLAN-0009](plans/0009-world-building-olc-command-surface.md). diff --git a/docs/plans/0009-world-building-olc-command-surface.md b/docs/plans/0009-world-building-olc-command-surface.md index a8b9664..9bc48fd 100644 --- a/docs/plans/0009-world-building-olc-command-surface.md +++ b/docs/plans/0009-world-building-olc-command-surface.md @@ -4,7 +4,7 @@ **Status:** Done -**Last updated:** 2026-07-23 +**Last updated:** 2026-07-24 ## Goal @@ -64,6 +64,13 @@ that changed. (`Description` left `""`), attached as a child of `ctx.CurrentRoom.Parent`, registered in `ctx.World`. Calls `RoomConnector.Connect` between `ctx.CurrentRoom` and the new room. + **Checks `area.Add(room)`'s bool return** (caught in PR review — + `Thing.Add` publishes a cancelable `AddChildEvent`; nothing vetoes + it today, but proceeding regardless would have registered/connected/ + saved a room that was never actually attached to the tree had that + ever changed) and rejects cleanly instead of assuming success, + matching `MoveCommand`'s existing precedent of checking `Remove`'s + return the same way. - [x] `TunnelCommand` (`MinorBuilder`) — usage `tunnel `. Looks up the destination room via `BuilderCommandHelpers.FindRoomsByName` (exact `Name` match, @@ -72,18 +79,24 @@ that changed. `ObjectMatcher`, verified directly rather than assumed). Rejects zero matches, more than one match, and self-tunnel. Saves both the current room's tree root and the destination's (they may differ if - there's more than one area). + there's more than one area) — **not atomic across the two calls**, + flagged in PR review and tracked as a deferred gap rather than fixed + here (see `docs/persistence.md` Open Items and this plan's Open + questions/blockers below). - [x] `DescribeCommand` (`MinorBuilder`) — usage `describe `. Sets `ctx.CurrentRoom.Description` to the rest-of-line text (empty rejected via `CommandGuards.RequireArgsAsync`). - [x] `BuilderCommandHelpers` (`src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommandHelpers.cs`) - — `TryParseDirection`, `FindRoomsByName`, `FindRoot`, shared by - `DigCommand`/`TunnelCommand`/`DescribeCommand`. Mirrors - `AdminCommandHelpers`'s shape (`internal static class`, no - `IThingRepository` dependency — each command holds its own - repository via its own constructor and never threads it into the - helper). + — `TryParseDirection`, `FindRoomsByName`, `HasExit`, `FindRoot`, + `OccupiedDirectionMessage`, shared by `DigCommand`/`TunnelCommand`/ + `DescribeCommand`. Mirrors `AdminCommandHelpers`'s shape (`internal + static class`, no `IThingRepository` dependency — each command + holds its own repository via its own constructor and never threads + it into the helper). `OccupiedDirectionMessage` added during PR + review — the "already an exit ... from here" rejection text was + duplicated verbatim in `DigCommand` and `TunnelCommand`; both now + call the shared helper instead. - [x] Register all three via `RegisterWithRole` in `BuilderCommands.RegisterAll(registry, repository)` (`src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommands.cs` @@ -145,7 +158,9 @@ Modified: attaches the new room as a sibling (child of `ctx.CurrentRoom.Parent`, not of `ctx.CurrentRoom` itself), calls `SaveTreeAsync` with the tree root (not `ctx.CurrentRoom`). Invalid direction and empty name each - rejected with a clear message, no mutation. + rejected with a clear message, no mutation. Added during PR review: a + vetoed `AddChildEvent` (a test `Behavior` subscribing to cancel it) is + rejected cleanly, with no room registered and no save attempted. - Unit: `TunnelCommand` — happy path connects `ctx.CurrentRoom` to a found room; zero matches and multiple matches each rejected with a clear, distinct message; self-tunnel (found room equals @@ -241,3 +256,12 @@ Slice 3 test coverage. "quick-start default," not real game content, and this slice's `dig` now gives an in-game path to grow it without enlarging the fixed sample. +- **Flagged in PR review, deferred rather than fixed**: `TunnelCommand`'s + two `SaveTreeAsync` calls (origin root, destination root) aren't atomic + together — a failure between them could leave a durably-saved one-way + exit on disk. Currently unreachable (every world built so far has + exactly one area, so the two roots are always the same object), but a + real gap once/if a multi-area world exists. Fixing it properly needs + `IThingRepository` to grow a multi-root atomic save — a repository-API + change, deliberately not bundled into the PR that found it. See + `docs/persistence.md` Open Items. diff --git a/docsite/docs/moderation-and-world-building.md b/docsite/docs/moderation-and-world-building.md index 8565b39..d3311f7 100644 --- a/docsite/docs/moderation-and-world-building.md +++ b/docsite/docs/moderation-and-world-building.md @@ -21,8 +21,8 @@ The two tiers relevant here: |---|---|---| | `MinorAdmin` | `Player` | `boot`, `mute`, `unmute`, `announce` | | `FullAdmin` | `MinorAdmin`, `Player` | everything `MinorAdmin` can do, plus `ban`, `unban`, `rolegrant`, `rolerevoke` | -| `MinorBuilder` | — | `dig`, `tunnel`, `describe` | -| `FullBuilder` | `MinorBuilder` | (reserved for a future builder-tier command; no consumer yet) | +| `MinorBuilder` | `Player` | `dig`, `tunnel`, `describe` | +| `FullBuilder` | `MinorBuilder`, `Player` | (reserved for a future builder-tier command; no consumer yet) | "Implies" is real: granting `FullAdmin` also grants `MinorAdmin` and `Player` in the same call, so a `FullAdmin` can immediately run every diff --git a/src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommandHelpers.cs b/src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommandHelpers.cs index c8ec7a1..6a46322 100644 --- a/src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommandHelpers.cs +++ b/src/SharpMud.Engine/Commands/Builtin/Builder/BuilderCommandHelpers.cs @@ -50,6 +50,16 @@ public static IReadOnlyList FindRoomsByName(IWorld world, string name) => public static bool HasExit(Thing room, Direction direction) => room.Children.Any(c => c.FindBehavior()?.Direction == direction); + /// + /// The rejection message for "the current room already has an exit in + /// this direction" - shared by and (both check against + /// their origin room) so the wording can't drift between the two + /// (caught in PR review - it was duplicated verbatim in both files). + /// + public static string OccupiedDirectionMessage(Direction direction) => + $"There's already an exit {direction.ToDisplayString()} from here."; + /// Walks up to the tree root (the first ancestor with no parent) - the node needs so a save captures every changed Thing, not just the current room's own subtree. public static Thing FindRoot(Thing thing) { diff --git a/src/SharpMud.Engine/Commands/Builtin/Builder/DigCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Builder/DigCommand.cs index 9cc0abc..bdd8c4d 100644 --- a/src/SharpMud.Engine/Commands/Builtin/Builder/DigCommand.cs +++ b/src/SharpMud.Engine/Commands/Builtin/Builder/DigCommand.cs @@ -38,7 +38,7 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) if (BuilderCommandHelpers.HasExit(ctx.CurrentRoom, direction)) { - await ctx.Session.WriteLineAsync($"There's already an exit {direction.ToDisplayString()} from here.", ct); + await ctx.Session.WriteLineAsync(BuilderCommandHelpers.OccupiedDirectionMessage(direction), ct); return; } @@ -52,7 +52,19 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) var room = new Thing { Id = ThingId.New(), Name = name, Description = "" }; room.Behaviors.Add(new RoomBehavior()); - area.Add(room); + + // area.Add publishes a cancelable AddChildEvent - nothing vetoes it + // today, but MoveCommand already checks Thing.Remove's bool return + // rather than assuming success (caught in PR review), so this + // matches that established pattern instead of proceeding blind on + // a rejected add (which would otherwise register/connect/save a + // room that was never actually attached to the tree). + if (!area.Add(room)) + { + await ctx.Session.WriteLineAsync("Couldn't create the room here.", ct); + return; + } + ctx.World.Register(room); RoomConnector.Connect(ctx.World, ctx.CurrentRoom, room, direction); diff --git a/src/SharpMud.Engine/Commands/Builtin/Builder/TunnelCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Builder/TunnelCommand.cs index cdb330a..b529f58 100644 --- a/src/SharpMud.Engine/Commands/Builtin/Builder/TunnelCommand.cs +++ b/src/SharpMud.Engine/Commands/Builtin/Builder/TunnelCommand.cs @@ -60,7 +60,7 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) if (BuilderCommandHelpers.HasExit(ctx.CurrentRoom, direction)) { - await ctx.Session.WriteLineAsync($"There's already an exit {direction.ToDisplayString()} from here.", ct); + await ctx.Session.WriteLineAsync(BuilderCommandHelpers.OccupiedDirectionMessage(direction), ct); return; } diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/DigCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/DigCommandTests.cs index 71aaddb..e81c6b0 100644 --- a/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/DigCommandTests.cs +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Builder/DigCommandTests.cs @@ -82,6 +82,29 @@ public async Task ExecuteAsync_RejectsDirectionAlreadyOccupied_WithoutMutating() await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, Arg.Any()); } + [Fact] + public async Task ExecuteAsync_RejectsCleanly_WhenAddChildEventIsVetoed() + { + var repository = Substitute.For(); + var (area, room, world, session) = MakeRoom(); + area.Events.SubscribeRequest((_, evt) => + { + if (evt is AddChildEvent) + evt.Cancel("full"); + }); + + var sut = new DigCommand(repository); + var ctx = new CommandContext(room, room, ["north", "Storage", "Shed"], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + area.Children.Should().ContainSingle("the vetoed room was never attached to the tree"); + world.AllWithBehavior().Should().HaveCount(1) + .And.Contain(room, "the vetoed room must never be registered either"); + await session.Received(1).WriteLineAsync("Couldn't create the room here.", Arg.Any()); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, Arg.Any()); + } + [Fact] public async Task ExecuteAsync_SendsUsageMessage_WhenMissingArguments() {