diff --git a/SPEC.md b/SPEC.md index 68d3720..8fad65b 100644 --- a/SPEC.md +++ b/SPEC.md @@ -204,8 +204,9 @@ Explicitly out of scope for v1, to revisit later: depends on this provider (co-developed with Jonas Ha) reaching a usable state; SQLite remains the dev/default backend until then. - **Hot-swap ruleset/plugin loading**: ruleset assemblies are loaded via a - compile-time project reference plus a startup assembly-scan (see + compile-time project reference plus explicit DI extension methods a + consumer calls at startup (see [docs/engine-vs-ruleset.md](docs/engine-vs-ruleset.md)), not a MEF-style - drop-a-DLL-in-a-folder mechanism. True dynamic loading + drop-a-DLL-in-a-folder mechanism or an attribute-scan. True dynamic loading (`AssemblyLoadContext`-based) is deferred until there's a real third party wanting to redistribute a ruleset without a sharp-mud source checkout. diff --git a/docs/adr/0008-ruleset-scaffolding-tier.md b/docs/adr/0008-ruleset-scaffolding-tier.md new file mode 100644 index 0000000..c62edb5 --- /dev/null +++ b/docs/adr/0008-ruleset-scaffolding-tier.md @@ -0,0 +1,382 @@ +# [ADR-0008] A Reusable RPG Scaffolding Tier Between `Engine` and Concrete Rulesets + +**Status:** Proposed + +**Date:** 2026-07-21 + +**Decision Makers:** Nick Cipollina + +## Context + +[ADR-0006](0006-nuget-package-distribution.md) shipped `SharpMud.Engine`/ +`SharpMud.Hosting`/`SharpMud.Persistence`(`.Sqlite`/`.DynamoDb`)/ +`SharpMud.Adapters.*` as real NuGet packages, and deliberately kept ruleset +logic (stats, combat, races/classes) unpackaged, living entirely in +`samples/SharpMud.Samples.Classic` as a reference implementation a consumer +reads and reimplements from scratch in their own repo. + +In practice, that reference implementation is doing more work than a +consumer should have to redo. Reading the actual code in +`samples/SharpMud.Samples.Classic` (not just its file list) shows two very +different kinds of content sitting in one project: + +- **Genuinely flavor-specific content**: `Race`/`CharacterClass` (D&D-style + enum values via `LayeredCraft.OptimizedEnums`), `StatsBehavior` (the six + D&D attributes + mana/stamina/level/XP), `HubWorldBuilder` (hand-built + world content), `Program.cs` (the composition root). +- **Generic-shaped RPG scaffolding that already has no coupling to the + flavor-specific content above**: `CombatantBehavior` (HP/`ArmorClass`/ + damage range/XP reward — a `Behavior` any `Thing` can carry, already + documented as "independent of `StatsBehavior` so an NPC doesn't need a + full character sheet just to throw a punch"), `ICombatResolver`/ + `CombatResolver` (d20-vs-AC resolution, reads only `CombatantBehavior`), + `ICombatManager`/`CombatManager` (tick-driven encounter tracking), + `AttackCommand`/`FleeCommand`. Verified directly against the current + source: none of these extracted types reference `Race`, `CharacterClass`, + or any D&D-specific attribute. `CombatManager`'s only two touches of + `StatsBehavior` (awarding XP on a kill, applying the death-penalty XP/HP + cut) are already `FindBehavior()` calls guarded by + `if (stats is not null)` — i.e. the existing code already treats + `StatsBehavior` as an optional companion, not a hard dependency. + +That second group is the actual "core game logic" complaint: it's reusable +in shape, but stuck unpackaged in one specific flavor's sample, so a +consumer building any RPG-shaped ruleset — not just a D&D reskin — +re-derives combat/encounter-tracking/attack-command wiring from scratch. + +A research pass against WheelMUD (the prior art this whole project is +modeled on, see [wheelmud-findings.md](../research/wheelmud-findings.md)) +found no precedent for a three-tier split on the *rules* dimension: +`WarriorRogueMage` is WheelMUD's only ruleset, ever, and its stats/skills/ +combat are one monolithic assembly with no further split — WheelMUD never +had a second ruleset to force that question. (WheelMUD does keep a real, +validated split between `Core` and `WheelMUD.Universe`, but `Universe` +turned out — checked directly against its actual file list and `.csproj` +references — to be a library of generic, ruleset-agnostic *item-type* +behaviors, `WeaponBehavior`/`PotionBehavior`/`ContainerBehavior`/etc., not +world content as this repo's own findings doc summary previously implied — +corrected in the same change as this ADR. `Universe` references only +`Core`/`Data`/`Effects`, never `WarriorRogueMage`, and vice versa — two +independent siblings composed together only at `Main`. Sharp-mud already +made this exact call at the `Engine` tier directly, via `WearableBehavior`/ +`EquippedBehavior`, rather than needing a separate package for it.) + +One more piece of WheelMUD prior art is directly relevant here and *is* +worth adopting, unlike the three-tier rules split: `WheelMUD.Core.DiceService` +(a `Random`-backed dice roller — `DiceService.Instance.GetDie(sides).Roll()`) +lives in WheelMUD's engine tier, not `WarriorRogueMage` — WheelMUD treats +"roll dice" as an engine-level concern, distinct from any ruleset's specific +rules. It's a static singleton (the exact anti-pattern this repo's own +research doc already rejected — see `wheelmud-findings.md` §10), has no +multi-die/modifier notation (`2d6+3`), and isn't an interface despite the +name suggesting one. sharp-mud already has the DI-friendly primitive this +should build on — `IRandomSource`, already constructor-injected into +`CombatResolver`/`FleeCommand` today — but nothing sits on top of it for +"roll N dice of M sides plus a modifier"; call sites just do raw +`random.Next(min, max)`. A small dice-rolling abstraction over `IRandomSource` +belongs in `SharpMud.Ruleset.Rpg` (generic RPG mechanic, not bare randomness, +so not `Engine`; not ruleset-flavor-specific, so not `Basic`/Classic) — see +Decision Outcome. + +So this ADR is genuinely new ground for this project, not an adoption of +already-proven prior art, and is scoped narrowly to the rules dimension: +should there be a packaged tier between `SharpMud.Engine` and a concrete +ruleset, specifically for RPG-shaped scaffolding (stats/combat/leveling), +the way `Microsoft.EntityFrameworkCore.Relational` sits between +`Microsoft.EntityFrameworkCore` and a concrete provider (`.Sqlite`/ +`.SqlServer`/...)? + +## Decision Drivers + +- Primary goal (stated directly): make the engine genuinely easy to extend + for someone building *their own* ruleset — not just technically possible + via raw `Thing`/`Behavior` primitives, but with real combat/encounter + scaffolding available off the shelf. +- Secondary, related goal: a true "`dotnet add package`, a few lines in + `Program.cs`, run a basic game" quick-start — the same shape as EF Core's + `UseSqlite(...)` one-liner. Note this requires a concrete, runnable leaf + package, not the abstract scaffolding tier alone — `Relational` isn't a + database either. +- Don't design the scaffolding boundary against a single consumer. WheelMUD + never had a second ruleset to validate `WarriorRogueMage`'s shape against; + this ADR should have at least two (a new minimal package plus the + existing Classic sample) before the contracts are treated as settled. +- Don't turn game-balance-sensitive content (dice formulas, race/class + stat blocks, hand-built world content) into a public SemVer compatibility + promise — that content changes far more often than infrastructure code, + and `SharpMud.Persistence`'s "public API surface is a real compatibility + promise" cost (ADR-0006's Negative Consequences) shouldn't extend to + content that isn't infrastructure. +- Reuse the existing low extraction cost where it already exists — + `CombatantBehavior`/`ICombatResolver`/`ICombatManager`/`AttackCommand`/ + `FleeCommand` are already flavor-agnostic in practice; this is largely a + move, not a rewrite, modulo the `StatsBehavior` XP/death-penalty seam. +- Standing repo rules still apply: composition over inheritance (new + scaffolding types are `Behavior`s, not a class hierarchy), + `SharpMud.Engine` never references ruleset-specific types, DI-only wiring, + no MEF/dynamic loading. + +## Considered Options + +1. **Status quo** — ruleset logic stays entirely unpackaged in + `samples/SharpMud.Samples.Classic`, per ADR-0006 as originally written. +2. **Promote the generic-shaped pieces directly into `SharpMud.Engine`**, + the same way `WanderingBehavior` earned its way in (it only needed + `NpcBehavior`/`ExitBehavior`, no ruleset data). +3. **Package Classic itself as an installable starter/composite product** + (`SharpMud.Ruleset.Classic` or similar) — a consumer `dotnet add + package`s the actual D&D-flavored game and overrides pieces of it. +4. **A dedicated scaffolding package (`SharpMud.Ruleset.Rpg`) between + `Engine` and concrete rulesets, plus a new minimal concrete leaf package + (`SharpMud.Ruleset.Basic`) built on it, with Classic remaining an + unpackaged sample rebuilt on the same scaffolding** (chosen). + +## Decision Outcome + +Chosen option: **4**. Three real tiers: + +``` +SharpMud.Engine unchanged — Thing/Behavior/events, zero ruleset knowledge + +SharpMud.Ruleset.Rpg NEW, packaged — CombatantBehavior, ICombatResolver/CombatResolver, + ICombatManager/CombatManager, AttackCommand/FleeCommand, moved + in from samples/SharpMud.Samples.Classic (see Context — these + these extracted types' actual combat logic has no flavor-specific coupling + today, but CombatManager itself carries two seams that must be + decoupled before the move, not moved as-is): + - XP-award/death-penalty touches into StatsBehavior — likely via + a combat-outcome game event through the existing ThingEvents + pipeline (Thing.Add/Remove already use this pattern for + cancellable events; exact mechanism is a plan-level decision). + - CombatManager's constructor takes a concrete `Thing hubRoom` and + hard-codes it as the respawn destination on defeat — a + Classic-specific world/respawn policy, not generic combat logic. + Needs the same kind of seam (plausibly the same combat-outcome + event above also owning "where does the loser respawn," rather + than a second bespoke mechanism — exact shape is a plan-level + decision) so Basic/a custom ruleset aren't forced to have a + "hub room" concept at all. + Once both seams are decoupled, this package has zero reference to + any concrete ruleset's stat/race/class/world-content types. Also + gains a small dice-rolling abstraction over the existing + Engine-level IRandomSource (DI-registered, not a WheelMUD-style + static singleton — see Context) covering the "N dice of M sides + plus a modifier" shape CombatResolver/FleeCommand currently do + with raw random.Next(...) calls. Ships its own DI registration + entry point (an AddSharpMudRpgRuleset(...)-shaped extension) + reproducing today's manual Program.cs wiring (ICombatResolver, + ICombatManager registered as both itself and ITickable off the + same instance, the mapping contributor, the dice service) — Basic, + Classic, and a custom consumer all call this instead of each + hand-rolling the same scaffolding verbatim. This entry point must + also register AttackCommand/FleeCommand in a way that composes + with Basic/Classic/a consumer's own commands, not replace them -- + Hosting's AddSharpMudRuleset(...) takes a single callback, so two + independent calls (one for Rpg's commands, one for the consumer's) + would silently clobber each other via DI's last-registration-wins + resolution. Command registration is a real part of this package's + public contract, not just a plan-level implementation detail; exact + mechanism (forwarding callback vs. additive registry) is left to the + plan. Not runnable/playable on its own, same as + Microsoft.EntityFrameworkCore.Relational isn't a database. + **Persistence mapping moves with it**: `CombatantBehavior`'s EF + Core configuration (today `Configurations/ + CombatantBehaviorConfiguration.cs`, applied via + `ClassicBehaviorMappingContributor.ConfigureBehaviors`'s + `ApplyConfigurationsFromAssembly(typeof(ClassicBehaviorMappingContributor).Assembly)` + — an assembly-scan scoped to the Classic assembly only, per + `docs/persistence.md`'s `IBehaviorMappingContributor` seam) has to + move to an `SharpMud.Ruleset.Rpg`-owned + `IBehaviorMappingContributor` once `CombatantBehavior` lives in a + different assembly, or a saved world containing `CombatantBehavior` + hits an unmapped TPH discriminator subtype the moment Classic stops + owning that configuration. **This is a real package-boundary + tradeoff, not a free move**: `IBehaviorMappingContributor` is + defined in `SharpMud.Persistence`, so `SharpMud.Ruleset.Rpg` takes a + dependency on `SharpMud.Persistence` to implement it — meaning a + consumer who wants Rpg's combat scaffolding purely in-memory, with + no EF Core involved at all, still pulls in `Persistence`. Accepted + here rather than splitting persistence mapping into a separate + companion package, since no consumer has asked for persistence-free + combat scaffolding and speculatively splitting for a hypothetical + one repeats the mistake this ADR's Considered Options already + reasoned against elsewhere — revisit if that actually comes up. + +SharpMud.Ruleset.Basic NEW, packaged, deliberately minimal — a concrete flavor built on + SharpMud.Ruleset.Rpg: a plain numeric stat block (no Race/ + CharacterClass), simple d20-ish combat via the scaffolding's + resolver, a tiny default IWorldBuilder (a room or two, including + at least one Thing with CombatantBehavior as a fightable NPC — + the Goal above promises a fresh character can walk around and + fight something, which empty rooms alone don't satisfy), and its + own IPlayerFactory (mirrors ClassicPlayerFactory) so Hosting's + PlayerLogin/LoginFlow can actually create a fresh CLI/Telnet + player — without it the quick-start fails at first login, not + just "nothing to see." This is the actual "dotnet add package + + a few Program.cs lines = a playable game" experience, analogous + to UseSqlite(...). + +samples/SharpMud.Samples.Classic Stays a sample, unpackaged. Rebuilt on SharpMud.Ruleset.Rpg + instead of owning CombatantBehavior/CombatResolver/etc. + directly — keeps its own StatsBehavior/Race/CharacterClass/ + HubWorldBuilder as flavor-specific content demonstrating a + richer game than Basic, on the same scaffolding. +``` + +Package naming follows ADR-0006's convention (no `LayeredCraft.` prefix, +package ID matches project/assembly name 1:1) — `SharpMud.Ruleset.Rpg` and +`SharpMud.Ruleset.Basic` are working names, open to bikeshedding at +implementation time, not a load-bearing part of this decision. + +`Ruleset.Basic` and Classic together give the `Rpg` scaffolding's contracts +(`ICombatResolver` chiefly) two genuinely different concrete consumers to +validate against before the package ships — Classic's dice-vs-AC combat and +Basic's own (deliberately similarly-shaped, since it's meant to be simple) +combat, both exercising the same `ICombatResolver` contract, is the closest +this repo can get to WheelMUD-style validation without inventing a third, +speculative ruleset just to prove a point. + +### Positive Consequences + +- Directly answers the original complaint: a consumer building their own + ruleset gets working combat/encounter-tracking/attack-command scaffolding + for free instead of re-deriving it, the same value a `Behavior`-composition + model already provides for engine-level concerns. +- A genuine NuGet quick-start becomes possible (`Ruleset.Basic`), matching + the "few lines in `Program.cs`, run a basic game" goal directly, without + turning content that changes often (Classic's dice formulas, race/class + values, hub-world content) into a versioned public API. +- Extraction cost is lower than a fresh design would suggest — most of the + scaffolding tier's core types are already flavor-agnostic in the existing + codebase. Real design/implementation work remains, though, and shouldn't + be minimized: decoupling `CombatManager`'s `StatsBehavior` touches and its + hard-coded `hubRoom` respawn destination, moving `CombatantBehavior`'s EF + Core mapping into a `Ruleset.Rpg`-owned `IBehaviorMappingContributor`, and + building the new dice-rolling abstraction over `IRandomSource` are all real + work this ADR commits to, not incidental cleanup. +- Two real consumers (`Basic`, Classic) exercise `Rpg`'s contracts before + they're locked in, rather than one. + +### Negative Consequences + +- Two new packages added to the already-lockstep-versioned set from + ADR-0006/0007 — more public API surface to maintain, and an open question + (deliberately not settled here — see Open Items below) about whether + either belongs in the `SharpMud` meta-package, which ADR-0007 already + narrowed once over unwanted transitive dependencies. +- `SharpMud.Ruleset.Rpg`'s contracts (`ICombatResolver`, whatever mechanism + replaces the direct `StatsBehavior` touches) become a real compatibility + promise the moment they're published — the same category of cost ADR-0006 + already flagged for `SharpMud.Persistence`, now extended to a third + conceptual layer. +- `Ruleset.Basic`, however minimal, is still a concrete published ruleset — + its default stat numbers/combat math are a smaller but real compatibility + promise, the same category of cost Option 3 was rejected for, just + deliberately kept small by being genuinely minimal. +- This is genuinely new territory beyond WheelMUD's validated prior art — + even with two consumers instead of one, the risk that `Rpg`'s boundary is + drawn in the wrong place is real, just lower than designing against Classic + alone. +- `SharpMud.Ruleset.Rpg` taking on a `SharpMud.Persistence` dependency (see + Decision Outcome/Open Items) means there's no persistence-free path to + Rpg's combat scaffolding — a consumer who wants in-memory-only combat, no + EF Core involved at all, still pulls in `Persistence` transitively. + Accepted for now since no consumer has asked for that, but a real + consumer-facing cost, not a free extraction. + +## Pros and Cons of the Options + +### Option 1 — Status quo (sample-only) + +- Good, because it's zero new packages, zero new public API surface, and + matches WheelMUD's own actual (never-needed-a-split) precedent exactly. +- Bad, because it leaves the actual, present complaint unaddressed — the + sample is genuinely unpleasant to build a different ruleset on top of + today, not just hypothetically. + +### Option 2 — Promote generic pieces directly into `SharpMud.Engine` + +- Good, because it needs no new package/project, reusing the `Engine` + distribution channel that already exists. +- Bad, because `engine-vs-ruleset.md`'s "zero ruleset knowledge" line for + `Engine` has been clean so far (`WanderingBehavior`'s promotion only + needed `NpcBehavior`/`ExitBehavior` — no combat, no stats, no leveling + concept at all); `CombatantBehavior`/`ICombatResolver` assume "this is an + RPG with hit points and damage," which is a materially bigger concession + than anything `Engine` has absorbed before, for consumers who might want + a MUD with no combat system whatsoever. + +### Option 3 — Package Classic itself as an installable starter product + +- Good, because it's the simplest possible "install and get exactly this + game" experience — no new abstraction to design at all. +- Bad, because it turns genuinely volatile content (dice formulas, + race/class stat blocks, hand-built world content — the things most likely + to change release-to-release) into a public SemVer compatibility promise. +- Bad, because it only serves a consumer who wants *this specific* D&D-ish + game — it does nothing for the stated primary goal of making it easy to + build a *different* ruleset on the engine; that consumer skips the + package entirely and is back to the Option 1 experience. + +### Option 4 — Scaffolding tier + minimal concrete leaf + Classic as sample (chosen) + +See Decision Outcome above. + +- Good, because it's the only option that serves both stated goals (easy to + extend the engine for a custom ruleset, and a genuine one-line quick-start) + without also publishing volatile game-balance content as a compatibility + promise. +- Good, because the extraction cost is verified low against the actual + current code, not assumed. +- Bad, because it's the most new package surface area of any option (two + new packages, not zero or one), and the newest, least-precedented design + decision in this project's history — no external prior art validates the + three-tier shape specifically, only the general "shared scaffolding under + multiple concrete leaves" idea from EF Core's own ecosystem. + +## Links + +- [engine-vs-ruleset.md](../engine-vs-ruleset.md) — the `Engine`/ruleset + split this ADR adds a tier to, not replaces +- [ADR-0006](0006-nuget-package-distribution.md) — this ADR refines + ADR-0006's "ruleset logic lives in `samples/`, not a package" framing by + carving out a packaged scaffolding + minimal-leaf tier specifically for + reusable RPG shape, while keeping concrete flavor content (Classic) + unpackaged; ADR-0006's granular-package/meta-package/`Hosting` mechanics + are otherwise unchanged and this does not supersede it +- [ADR-0007](0007-narrow-meta-package-scope.md) — the meta-package scoping + precedent this ADR's Open Items below will need to weigh + `Ruleset.Rpg`/`Ruleset.Basic` against +- [wheelmud-findings.md](../research/wheelmud-findings.md) — prior art + reviewed for this decision; corrected in the same change (its `Universe` + description, and a new Decisions entry for `DiceService`/`Die` — adopted + in spirit as DI-based dice rolling on top of `IRandomSource`, not adopted + as a static singleton) + +## Open Items + +- Whether `SharpMud.Ruleset.Rpg`/`SharpMud.Ruleset.Basic` belong in the + `SharpMud` meta-package (narrowed by ADR-0007 to Engine + Hosting + + Persistence) is not decided here — a consumer who wants a *different* + ruleset shape shouldn't get RPG-specific scaffolding pulled in by default, + echoing ADR-0007's own reasoning almost exactly. Likely answer is "no," + but that's a decision for whoever implements this, not asserted here. +- The exact mechanism for decoupling `CombatManager`'s XP-award/death-penalty + logic from `StatsBehavior` (a game event through `ThingEvents`, a small + callback interface, or something else) is left to the implementation plan, + not fixed by this ADR. +- The exact mechanism for decoupling `CombatManager`'s hard-coded `hubRoom` + respawn destination — just as blocking as the `StatsBehavior` seam above + for moving `CombatManager` into a generic package, and plausibly the same + mechanism, but left to the implementation plan, not fixed by this ADR. +- The exact mechanism for composing `AttackCommand`/`FleeCommand` + registration with a consumer's own commands (a forwarding callback through + `AddSharpMudRpgRuleset(...)`, or an additive `ICommandRegistry`) — command + registration is part of `Ruleset.Rpg`'s public contract per the Decision + Outcome above, just as blocking as the other two seams, and left to the + implementation plan, not fixed by this ADR. +- `SharpMud.Ruleset.Rpg` taking a dependency on `SharpMud.Persistence` (to + implement `IBehaviorMappingContributor` for `CombatantBehavior`) means + there's no persistence-free path to Rpg's combat scaffolding — accepted + for now (see Decision Outcome) since no consumer has asked for one; + revisit with a companion persistence-mapping package if that changes. diff --git a/docs/adr/README.md b/docs/adr/README.md index b2c6dd7..b25e9c4 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -67,3 +67,4 @@ the mechanics: numbering, status, and the index. | [0005](0005-security-role-model-and-moderation-commands.md) | Security Role Model + Moderation Commands | Accepted | | [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 | Proposed | diff --git a/docs/engine-vs-ruleset.md b/docs/engine-vs-ruleset.md index 819ce5e..4707541 100644 --- a/docs/engine-vs-ruleset.md +++ b/docs/engine-vs-ruleset.md @@ -271,3 +271,12 @@ revisit if guard logic keeps growing. packages plus `SharpMud.Samples.Classic` as a reference consumer under `samples/`. Implemented — this doc's project-structure listing above reflects the current layout. +- `SharpMud.Samples.Classic` owning combat/stats scaffolding directly (not + just its own D&D-specific content) is addressed by + [ADR-0008](adr/0008-ruleset-scaffolding-tier.md) — a new + `SharpMud.Ruleset.Rpg` package for the reusable scaffolding, a new minimal + `SharpMud.Ruleset.Basic` package as a concrete quick-start leaf sibling to + Classic — both built directly on `SharpMud.Ruleset.Rpg`, neither depending + on the other — with Classic staying a still-unpackaged, richer sample. Not + yet implemented — this doc's project-structure listing above does not yet + reflect this. diff --git a/docs/plans/0008-ruleset-scaffolding-tier.md b/docs/plans/0008-ruleset-scaffolding-tier.md new file mode 100644 index 0000000..eefdaee --- /dev/null +++ b/docs/plans/0008-ruleset-scaffolding-tier.md @@ -0,0 +1,279 @@ +# [PLAN-0008] A Reusable RPG Scaffolding Tier Between `Engine` and Concrete Rulesets + +**Implements:** ADR-0008 (link once that ADR is `Accepted` — do not move this +plan to `In Progress` while it still reads `Proposed`) + +**Status:** Not Started + +**Last updated:** 2026-07-22 + +## Goal + +A consumer can `dotnet add package SharpMud.Ruleset.Basic` (plus `Engine`/ +`Hosting`/`Persistence.*`/a transport adapter), write a small `Program.cs`, +and run a real, if minimal, playable MUD with working combat — without +writing any combat/stats/encounter code themselves. Separately, a consumer +building a *different* ruleset can reference `SharpMud.Ruleset.Rpg` alone and +get the same combat/encounter scaffolding without `Basic`'s specific numbers. +`samples/SharpMud.Samples.Classic` still works exactly as it does today from +a player's perspective, now built on `SharpMud.Ruleset.Rpg` instead of owning +combat scaffolding directly. + +## Scope + +In scope: +- Extract `CombatantBehavior`/`ICombatResolver`/`CombatResolver`/ + `ICombatManager`/`CombatManager`/`AttackCommand`/`FleeCommand` into a new + `SharpMud.Ruleset.Rpg` package. +- Decouple `CombatManager`'s XP-award/death-penalty logic from + `StatsBehavior` (exact mechanism — game event vs. callback interface — is + this plan's to decide, per ADR-0008's Open Items). +- Decouple `CombatManager`'s constructor-injected `Thing hubRoom` respawn + destination — a Classic-specific world/respawn policy hard-coded into what + should be generic combat logic. Plausibly the same combat-outcome + mechanism above also owns "where does the loser respawn," rather than a + second bespoke seam — exact shape is this plan's to decide. +- Move `CombatantBehavior`'s EF Core mapping (`CombatantBehaviorConfiguration`) + into an `SharpMud.Ruleset.Rpg`-owned `IBehaviorMappingContributor`, since + today's `ClassicBehaviorMappingContributor.ConfigureBehaviors` only scans + its own (Classic) assembly for `IEntityTypeConfiguration<>` types — once + `CombatantBehavior` lives in a different assembly, that scan silently stops + finding its configuration and a saved world containing it hits an unmapped + TPH discriminator subtype. +- A small dice-rolling abstraction in `SharpMud.Ruleset.Rpg`, built on the + existing `IRandomSource` (constructor-injected, DI-registered — not a + WheelMUD-style static singleton), covering "N dice of M sides plus a + modifier." Replace `CombatResolver`/`FleeCommand`'s raw `random.Next(...)` + calls with it where it's a straightforward swap. +- Build a new, minimal `SharpMud.Ruleset.Basic` package on top of + `SharpMud.Ruleset.Rpg`: a plain numeric stat block, a default + `IWorldBuilder` (a room or two), DI registration extension + (`AddSharpMudBasicRuleset(...)`). +- Rebuild `samples/SharpMud.Samples.Classic` against `SharpMud.Ruleset.Rpg` + instead of owning the extracted types directly. + +Explicitly deferred / out of scope for this plan: +- Any additional ruleset "flavors" beyond `Basic` (`Freeform`/`Tactical` + from ADR-0008's discussion) — speculative, not committed to. +- Deciding whether `Ruleset.Rpg`/`Ruleset.Basic` join the `SharpMud` + meta-package (ADR-0008 Open Items) — treat as a follow-up ADR-0007-style + narrow decision if/when it comes up, not settled here. +- A leveling/progression contract (`ILevelingStrategy` or similar) beyond + whatever XP-award mechanism this plan lands on — only build it if the + decoupling work actually needs it. + +## Tasks + +- [ ] New `src/SharpMud.Ruleset.Rpg` project + - [ ] Move `CombatantBehavior`, `ICombatResolver`/`CombatResolver`, + `ICombatManager`/`CombatManager`, `AttackCommand`/`FleeCommand` in + from `samples/SharpMud.Samples.Classic` + - [ ] Design and implement the `StatsBehavior` decoupling (game event + through `ThingEvents`, or a small callback interface — pick one, + document why in a code comment per `documentation.md`'s bar for + non-obvious decisions) + - [ ] Design and implement the `hubRoom`/respawn-destination decoupling — + consider folding into the same mechanism as the `StatsBehavior` + decoupling above rather than a second bespoke seam + - [ ] While touching this: today's death-penalty handling only resets + `StatsBehavior.CurrentHitPoints`, but `CombatResolver` actually reads + and writes damage against `CombatantBehavior.CurrentHitPoints` — a + respawned character's `CombatantBehavior` HP stays at/below 0, so the + very next hit lands and instantly re-triggers "defeated" regardless + of the roll. This is a real, pre-existing bug (not introduced by this + extraction), but the decoupled respawn mechanism must reset + `CombatantBehavior.CurrentHitPoints` too, not just carry the + `StatsBehavior`-only reset forward into the new package + - [ ] Move `CombatantBehaviorConfiguration` + a new `SharpMud.Ruleset.Rpg`- + owned `IBehaviorMappingContributor` in from + `ClassicBehaviorMappingContributor`; verify `Basic`/Classic's own DI + registration actually wires this contributor up (`Persistence` + discovers contributors via DI, per `docs/persistence.md`) + - [ ] Dice-rolling abstraction over `IRandomSource` (DI-registered + interface + implementation, not a static singleton — "N dice of M + sides plus a modifier"); swap `CombatResolver`/`FleeCommand`'s raw + `random.Next(...)` calls to use it + - [ ] A package-level DI registration entry point (`AddSharpMudRpgRuleset(...)` + or equivalent) that reproduces today's manual wiring in + `samples/SharpMud.Samples.Classic/Program.cs` (`ICombatResolver`, + `ICombatManager` registered as both itself *and* `ITickable` off the + same instance, `IBehaviorMappingContributor`, the dice service) — + without this, `Basic`/Classic/a custom ruleset each hand-roll the + exact scaffolding this ADR exists to provide, verbatim + - [ ] Define how `AttackCommand`/`FleeCommand` registration composes with + a consumer's own commands. `Hosting`'s `AddSharpMudRuleset(...)` takes + a single callback — calling it a second time (once for Rpg's commands, + again for `Basic`/Classic's own) registers `ICommandRegistry` as a + singleton twice, and DI resolution returns only the *last* + registration, silently dropping the first call's commands entirely. + This isn't optional wiring detail — without a real answer, either + Rpg's `attack`/`flee` or the consumer's own commands go missing. + Plausible shapes: `AddSharpMudRpgRuleset(...)` itself takes and + forwards a consumer callback (so there's still only one + `AddSharpMudRuleset` call total), or `ICommandRegistry` registration + becomes additive across multiple sources instead of one factory — + exact mechanism is this plan's to decide, same as the other seams + above + - [ ] `csproj`: `Directory.Packages.props` entry, package metadata matching + the existing `SharpMud.*` packages +- [ ] New `src/SharpMud.Ruleset.Basic` project + - [ ] Minimal concrete stats behavior (plain numeric attributes, no + Race/CharacterClass) + - [ ] Default `IWorldBuilder` implementation (small, enough to walk + around) — must include at least one `Thing` with `CombatantBehavior` + (an NPC) as a valid `attack` target, not just empty rooms; the Goal + above promises a fresh character can "walk around and fight + something," which a world with nothing to fight doesn't satisfy + - [ ] `AddSharpMudBasicRuleset(...)` DI extension, options callback for the + tunable numbers (starting HP, etc.) + - [ ] Basic's new stats behavior needs its own `IEntityTypeConfiguration<>` + + `IBehaviorMappingContributor`, wired from `AddSharpMudBasicRuleset(...)` + — `GameDbContext.OnModelCreating` only discovers engine configs plus + registered contributors (`docs/persistence.md`), so without this a + Basic world/player carrying the new behavior hits the same unmapped + TPH subtype problem already caught for `CombatantBehavior` + - [ ] A Basic `IPlayerFactory` implementation (mirrors `ClassicPlayerFactory` + wrapping `HubWorldBuilder.CreatePlayer`) that creates a `Thing` with + `PlayerBehavior` plus Basic's stats/combat behaviors, wired from + `AddSharpMudBasicRuleset(...)` via `AddSharpMudPlayerFactory()` — + `PlayerLogin`/`LoginFlow` constructor-inject `IPlayerFactory`, so + without this a fresh CLI/Telnet player can't be created at all and + the quick-start fails at first login, not just at "no content to see" +- [ ] Rebuild `samples/SharpMud.Samples.Classic` + - [ ] Remove the now-extracted types; reference `SharpMud.Ruleset.Rpg` + instead + - [ ] Verify `StatsBehavior`/`Race`/`CharacterClass`/`HubWorldBuilder` + still compose correctly against the new decoupling mechanism +- [ ] Tests + - [ ] `SharpMud.Ruleset.Rpg.Tests` — mirrors today's Classic combat test + coverage, moved and adjusted for the decoupling change + - [ ] `SharpMud.Ruleset.Basic.Tests` — new coverage for the minimal + concrete ruleset + - [ ] `SharpMud.Samples.Classic.Tests` — updated for the new dependency + shape, same behavioral coverage as before +- [ ] Docs + - [ ] Update `engine-vs-ruleset.md`'s project-structure listing to reflect + the new packages (currently only has ADR-0008's forward-reference) + - [ ] Update `architecture.md` — it owns the solution layout and + direct-dependency summary; the new `Ruleset.Rpg`/`Ruleset.Basic` + projects and their dependency direction belong there too, not just + in `engine-vs-ruleset.md` + - [ ] Update `combat.md`/`character.md` subsystem docs if their described + "current state" changes + - [ ] Write the actual quick-start guidance the ADR's headline goal + promises — a README/docsite getting-started page walking through + `dotnet add package SharpMud.Ruleset.Basic` (+ `Engine`/`Hosting`/a + persistence provider/a transport adapter) through a runnable basic + game, plus package-consumption docs for `SharpMud.Ruleset.Rpg` + itself for a consumer building a different ruleset on it. Without + this, "few lines in `Program.cs`, run a basic game" is proven by an + internal manual test (see Verification) but never actually + documented for a real external consumer to follow. + - [ ] `docs/adr/README.md`: once implementation is complete and matches + ADR-0008 as written (or any divergence is reconciled), confirm the + ADR's `Status` reads `Accepted` — it must already be `Accepted` + *before* this plan moves to `In Progress` (see this plan's header + and `docs/plans/README.md`'s lifecycle rule); this task is a final + consistency check, not the acceptance step itself + +## Critical files + +New: +- `src/SharpMud.Ruleset.Rpg/*` (project + moved types + decoupling mechanism) +- `src/SharpMud.Ruleset.Basic/*` (project + minimal stats/world/DI extension) +- `tests/SharpMud.Ruleset.Rpg.Tests/*` +- `tests/SharpMud.Ruleset.Basic.Tests/*` + +Modified: +- `samples/SharpMud.Samples.Classic/*` — removes extracted types, adds + `SharpMud.Ruleset.Rpg` reference +- `tests/SharpMud.Samples.Classic.Tests/*` +- `Directory.Packages.props`, `SharpMud.slnx` +- `docs/architecture.md`, `docs/engine-vs-ruleset.md`, `docs/combat.md`, + `docs/character.md` (as needed), `docs/adr/README.md` +- `src/SharpMud.Hosting/ServiceCollectionExtensions.cs`, + `tests/SharpMud.Hosting.Tests/*` — **conditional** on which + command-composition design (above) gets chosen: touched if + `ICommandRegistry` registration becomes additive across multiple sources, + untouched if `AddSharpMudRpgRuleset(...)` instead takes and forwards a + consumer callback through the existing single-registration shape + +## Test plan + +Unit coverage mirrors today's Classic combat tests, relocated to +`SharpMud.Ruleset.Rpg.Tests`: `CombatResolver` hit/miss/damage math, +`CombatManager` encounter lifecycle (start/end/linkdead-freeze/defeat +handling), `AttackCommand`/`FleeCommand` guard and success paths — same +`AutoFixture`/`NSubstitute` patterns already established +(`testing.md`). The dice-rolling abstraction is new public scaffolding +behavior in its own right, not just a `CombatResolver` implementation +detail — it needs direct tests of its own (dice-count/sides/modifier +math, and validation/error behavior for invalid input like zero dice or +zero-sided dice), not incidental coverage via `CombatResolver`'s tests. +New coverage in `SharpMud.Ruleset.Basic.Tests` for the minimal stat block, +default world builder, and its own EF Core mapping's round-trip. Also add a +test proving `AddSharpMudBasicRuleset(...)` registers `IPlayerFactory` and +that it produces a `Thing` with `PlayerBehavior` plus Basic's stats/combat +behaviors — without this, a broken player factory only surfaces as a +first-login failure, not a test failure. `SharpMud.Samples.Classic.Tests` +should need no *behavioral* changes, only reference/namespace updates, +**except** for the intentional `CombatantBehavior.CurrentHitPoints` +respawn-reset fix below — that one behavioral change is expected and +required, not a signal of an extraction regression. Any *other* behavioral +change is still that signal. Add a persistence +round-trip test (save/load a `Thing` carrying `CombatantBehavior` through +`SharpMud.Persistence`) specifically to catch the TPH-mapping seam above +regressing — this is exactly the kind of gap that passes every unit test +but fails at actual save/load time. Add a defeat/respawn test asserting +`CombatantBehavior.CurrentHitPoints` is actually reset (not just +`StatsBehavior`'s) and the respawned character can be hit again without an +instant re-defeat, to catch the HP-reset bug above regressing. Add a +DI/composition test proving built-in commands, Rpg's `attack`/`flee`, and a +consumer's own registered command all end up in the same resolved +`ICommandRegistry` — this is the seam most likely to silently regress +(one registration source clobbering another) without a test that would +actually catch it. This coverage must prove `AttackCommand`'s actual public +command shape survives, not just "a command exists": today `Verb` is +`kill`, with `attack` registered as an alias, and Verification below +promises both stay compatible — assert the resolved `ICommandRegistry` +still resolves both `kill` and `attack`, not just one representative verb. + +## Verification + +Manual smoke test per this repo's established pattern for anything +session/gameplay-facing (`testing.md`): run `samples/SharpMud.Samples.Classic` +end-to-end, confirm `kill`/`attack`/`flee` behave identically to before the +extraction, **except** for the intentional death/respawn HP-reset fix above — +a respawned character's `CombatantBehavior.CurrentHitPoints` should now +actually reset, not stay at/below 0 and instantly re-trigger "defeated" on +the next hit; the smoke test should confirm the fix, not treat the old bug +as the expected baseline. Separately, build a throwaway minimal `Program.cs` against just +`SharpMud.Ruleset.Basic` (+ `Engine`/`Hosting`/a persistence provider/a +transport) and confirm a fresh character can log in, actually issue `attack` +against the default world's NPC and see combat resolve (hit/miss/damage, +not just "command not recognized"), and flee — this is the actual "few +lines, runnable basic game with working combat" claim ADR-0008 makes, and +needs a real run exercising the command-registration seam above, not just +passing unit tests, to confirm. + +## Open questions / blockers + +- Exact decoupling mechanism for `CombatManager`'s `StatsBehavior` touches + (game event vs. callback interface) — first task to resolve once this + plan moves to `In Progress`; blocks everything downstream in + `SharpMud.Ruleset.Rpg`. +- Exact decoupling mechanism for `CombatManager`'s hard-coded `hubRoom` + respawn destination — just as blocking as the `StatsBehavior` seam above + for moving `CombatManager` into a generic package, and plausibly resolved + by the same mechanism; listed here explicitly rather than only as a + nested task, since it's equally a blocker, not a follow-on detail. +- Package naming (`SharpMud.Ruleset.Rpg`/`SharpMud.Ruleset.Basic`) is a + working name per ADR-0008 — confirm before publishing, cheap to change + before first release, expensive after (lockstep versioning, per ADR-0006). +- Exact mechanism for composing `AttackCommand`/`FleeCommand` registration + with a consumer's own commands — `Hosting`'s `AddSharpMudRuleset(...)` + takes a single callback, so a naive second call for Rpg's commands + silently clobbers the consumer's via DI's last-registration-wins + resolution; either Rpg's or the consumer's commands go missing without a + real answer here. Just as blocking as the seams above for a usable + `AddSharpMudRpgRuleset(...)` entry point. diff --git a/docs/plans/README.md b/docs/plans/README.md index 67b935e..b7f19b9 100644 --- a/docs/plans/README.md +++ b/docs/plans/README.md @@ -61,3 +61,4 @@ isn't settled yet — don't do that. | [0004](0004-session-state-machine-and-reconnect.md) | [ADR-0004](../adr/0004-session-state-machine-and-reconnect.md) | Done | | [0005](0005-security-role-model-and-moderation-commands.md) | [ADR-0005](../adr/0005-security-role-model-and-moderation-commands.md) | Not Started | | [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) | Not Started | diff --git a/docs/research/wheelmud-findings.md b/docs/research/wheelmud-findings.md index e86732a..4dad962 100644 --- a/docs/research/wheelmud-findings.md +++ b/docs/research/wheelmud-findings.md @@ -18,7 +18,17 @@ directly rather than just "considered." 13 assemblies. The load-bearing split: `WheelMUD.Core` (engine) is completely separate from `WarriorRogueMage` (a sample ruleset), and `WheelMUD.Universe` -(default world content) is separate from both. +is a third, independent sibling — checked directly against its actual file +list and `.csproj`: it's a library of generic, ruleset-agnostic *item-type* +behaviors (`WeaponBehavior`/`ArmorBehavior`/`PotionBehavior`/`ContainerBehavior`/ +`CurrencyBehavior`/`PortalBehavior`/`LocksUnlocksBehavior`/etc.), not default +world/area content as an earlier version of this doc summarized. `Universe` +references only `Core`/`Data`/`Effects`, never `WarriorRogueMage`, and vice +versa; both are composed together only at `Main`. See +[ADR-0008](../adr/0008-ruleset-scaffolding-tier.md)'s Decisions for what this +means for sharp-mud (short version: nothing directly — sharp-mud already made +the equivalent call at the `Engine` tier itself, via `WearableBehavior`/ +`EquippedBehavior`, rather than a separate package). ``` src/ @@ -31,7 +41,7 @@ src/ ├── Server/ WheelMUD.Server — telnet networking, MCCP/MXP/NAWS │ └── Telnet/ ├── Data/, Data.RavenDb/ persistence (RavenDB document store) -├── Universe/ default world/area content bootstrap +├── Universe/ generic item-type behaviors (weapon/armor/potion/container/...) ├── WarriorRogueMage/ sample ruleset (stats/skills/races/combat) built entirely on Core ├── Main/, ServerHarness/ entry point / dev harness └── Tests/ @@ -305,9 +315,10 @@ exports at priority 100 - a downstream game overriding one command exports at **Not adopted** - `System.ComponentModel.Composition` is legacy tech, largely superseded in modern .NET. We get the important property (ruleset assemblies -plug into the engine without the engine referencing them) from a reflection -scan over loaded assemblies plus standard DI registration instead - see -Decisions. +plug into the engine without the engine referencing them) from explicit, +package-level DI extension methods a consumer calls at startup +(`AddSharpMudRuleset(...)`, `AddSharpMudRpgRuleset(...)`, +`AddSharpMudBasicRuleset(...)`) instead - see Decisions. ## 9. Combat @@ -368,17 +379,21 @@ update for the full design): `MultipleParentsBehavior` mechanism described in §6, since the exit Thing genuinely lives in both rooms' child lists at once. -2. **Ruleset plugins load via assembly-scan + DI, not MEF.** We get the - "engine doesn't reference ruleset code" property from reflection over - loaded assemblies (find types tagged `[GameAction]`/`[Behavior]` and - register them with the DI container at startup) instead of MEF's - `CompositionContainer`/`DirectoryCatalog`. This keeps the extensibility - property WheelMUD is built for while staying on the DI story - `docs/architecture.md` already committed to. True hot-swap-a-DLL-without- - rebuilding distribution (MEF's `DirectoryCatalog`) is not implemented yet - - `AssemblyLoadContext`-based dynamic loading can be added later if genuine - third-party redistribution (not just a separate project reference) is - needed. +2. **Ruleset plugins load via manual DI registration, not MEF and not + reflection/attribute-scanning.** We get the "engine doesn't reference + ruleset code" property from package-level DI extension methods a consumer + calls explicitly (`AddSharpMudRuleset(...)`, and per ADR-0008 + `AddSharpMudRpgRuleset(...)`/`AddSharpMudBasicRuleset(...)`) instead of + MEF's `CompositionContainer`/`DirectoryCatalog` or reflection over loaded + assemblies for tagged types. This keeps the extensibility property + WheelMUD is built for while staying on the explicit, no-MEF/no-dynamic- + loading DI story `docs/architecture.md` already committed to. True + hot-swap-a-DLL-without-rebuilding distribution (MEF's `DirectoryCatalog`) + is not implemented and not planned — `AssemblyLoadContext`-based dynamic + loading could be added later if genuine third-party redistribution (not + just a separate project reference) is ever needed, but manual DI + registration is the committed approach, not an interim step toward + scanning. Additionally, the event system is simplified to one generic `Publish(TEvent evt, EventScope scope) where TEvent : GameEvent` @@ -390,7 +405,7 @@ don't require adding new delegate pairs to a growing interface. 3. **`Server/Telnet/`'s IAC/Q-Method negotiation is adopted, its byte-parser class hierarchy is not** - see - [ADR-0002](../adr/0002-telnet-protocol-negotiation.md) (status: Proposed) + [ADR-0002](../adr/0002-telnet-protocol-negotiation.md) (status: Accepted) for the full record. The RFC-1143 four-state negotiation tracking is adopted near-verbatim; the 5-class persistent byte-parser state machine is replaced with a single-call parser, since sharp-mud's read loop is already @@ -410,6 +425,20 @@ don't require adding new delegate pairs to a growing interface. [ADR-0005](../adr/0005-security-role-model-and-moderation-commands.md)/ [PLAN-0005](../plans/0005-security-role-model-and-moderation-commands.md). +5. **`Core.DiceService`/`Die`'s dice-rolling concept is adopted, its static + singleton is not** - see + [ADR-0008](../adr/0008-ruleset-scaffolding-tier.md) for the full record. + WheelMUD treats "roll N-sided dice" as an engine-level concern + (`DiceService` lives in `WheelMUD.Core`, not `WarriorRogueMage`), which + sharp-mud agrees with in spirit, but `DiceService.Instance.GetDie(sides)` + is exactly the `XManager.Instance` static-singleton pattern §10 already + rejected, and has no multi-die/modifier (`2d6+3`) notation. sharp-mud + instead builds a small DI-registered dice-rolling abstraction ("N dice of + M sides plus a modifier") over the existing `IRandomSource` primitive, and + places it in `SharpMud.Ruleset.Rpg` rather than `SharpMud.Engine` - it's a + generic RPG mechanic, not bare randomness, so it doesn't belong at the + fully ruleset-agnostic engine tier the way WheelMUD's does. + Going forward, further reconciliation against WheelMUD (moderation/admin tooling, session reconnect, world-building commands, and more) is tracked as a sequenced roadmap in [ADR-0001](../adr/0001-wheelmud-reconciliation-roadmap.md)