From 87fe97c689827360d654dd3ce502ad37f68ffeb1 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 09:34:21 -0400 Subject: [PATCH 1/6] feat: reusable RPG scaffolding tier - Ruleset.Rpg + Ruleset.Basic (PLAN-0008) Implements ADR-0008: extracts CombatantBehavior/CombatResolver/CombatManager/ AttackCommand/FleeCommand out of the Classic sample into a new SharpMud.Ruleset.Rpg package, decoupling its stats-behavior/respawn touches behind a new ICombatOutcomeHandler seam, adding a DI-registered IDiceRoller over IRandomSource, and fixing a pre-existing bug where a respawned character's CombatantBehavior.CurrentHitPoints was never reset. Adds a new minimal SharpMud.Ruleset.Basic package built on Ruleset.Rpg for a true "dotnet add package, few lines, run a basic game" quick-start. Classic is rebuilt on Ruleset.Rpg instead of owning combat scaffolding directly. Also: dependabot now ignores the conditional/range-pinned packages it can't reliably update, Directory.Packages.props centralizes the lockstep net10.0/net11.0 Microsoft.* version ranges into one property per TFM, and the docsite gains Rulesets/Customizing pages. Co-Authored-By: Claude Sonnet 5 --- .github/dependabot.yml | 13 ++ Directory.Packages.props | 119 ++++++----- README.md | 18 +- SharpMud.slnx | 4 + docs/README.md | 1 + docs/adr/0008-ruleset-scaffolding-tier.md | 2 +- docs/adr/README.md | 2 +- docs/architecture.md | 25 ++- docs/character.md | 5 +- docs/combat.md | 110 +++++++--- docs/engine-vs-ruleset.md | 37 +++- docs/getting-started.md | 102 ++++++++++ docs/plans/0008-ruleset-scaffolding-tier.md | 131 ++++++------ docs/plans/README.md | 2 +- docsite/docs/customizing.md | 94 +++++++++ docsite/docs/getting-started.md | 14 +- docsite/docs/index.md | 15 +- docsite/docs/rulesets.md | 191 ++++++++++++++++++ docsite/zensical.toml | 4 +- .../ClassicBehaviorMappingContributor.cs | 8 +- .../ClassicCombatOutcomeHandler.cs | 60 ++++++ .../ClassicCommands.cs | 16 -- .../HubWorldBuilder.cs | 1 + samples/SharpMud.Samples.Classic/Program.cs | 18 +- .../SharpMud.Samples.Classic.csproj | 1 + src/SharpMud.Hosting/SharpMud.Hosting.csproj | 6 + .../BasicBehaviorMappingContributor.cs | 16 ++ .../BasicCombatOutcomeHandler.cs | 57 ++++++ .../BasicPlayerFactory.cs | 42 ++++ .../BasicRulesetOptions.cs | 14 ++ .../BasicStatsBehavior.cs | 13 ++ .../BasicWorldBuilder.cs | 83 ++++++++ .../BasicStatsBehaviorConfiguration.cs | 12 ++ .../ServiceCollectionExtensions.cs | 41 ++++ .../SharpMud.Ruleset.Basic.csproj | 17 ++ .../SharpMud.Ruleset.Rpg}/AttackCommand.cs | 15 +- .../SharpMud.Ruleset.Rpg}/CombatManager.cs | 57 +++--- .../SharpMud.Ruleset.Rpg}/CombatResolver.cs | 20 +- .../CombatantBehavior.cs | 12 +- .../CombatantBehaviorConfiguration.cs | 2 +- src/SharpMud.Ruleset.Rpg/DiceRoller.cs | 25 +++ .../SharpMud.Ruleset.Rpg}/FleeCommand.cs | 23 ++- .../SharpMud.Ruleset.Rpg}/ICombatManager.cs | 2 +- .../ICombatOutcomeHandler.cs | 32 +++ .../SharpMud.Ruleset.Rpg}/ICombatResolver.cs | 3 +- src/SharpMud.Ruleset.Rpg/IDiceRoller.cs | 14 ++ .../RpgBehaviorMappingContributor.cs | 16 ++ .../ServiceCollectionExtensions.cs | 63 ++++++ .../SharpMud.Ruleset.Rpg.csproj | 20 ++ .../SharpMud.Persistence.Tests.csproj | 1 + .../TestKit/TestDbContextFactory.cs | 3 +- .../ThingRepositoryTests.cs | 1 + .../BasicPlayerFactoryTests.cs | 33 +++ .../BasicWorldBuilderTests.cs | 31 +++ .../PersistenceRoundTripTests.cs | 47 +++++ .../ServiceCollectionExtensionsTests.cs | 50 +++++ .../SharpMud.Ruleset.Basic.Tests.csproj | 48 +++++ .../Attributes/BasicAutoDataAttribute.cs | 12 ++ .../TestKit/BaseFixtureFactory.cs | 22 ++ .../TestKit/TestDbContextFactory.cs | 31 +++ .../xunit.runner.json | 3 + .../Combat/CombatManagerTests.cs | 34 ++-- .../Combat/CombatResolverTests.cs | 17 +- .../Combat/DiceRollerTests.cs | 58 ++++++ .../Commands/AttackCommandTests.cs | 72 +++++++ .../Commands/FleeCommandTests.cs | 96 +++++++++ .../ServiceCollectionExtensionsTests.cs | 64 ++++++ .../SharpMud.Ruleset.Rpg.Tests.csproj | 46 +++++ .../Attributes/RpgAutoDataAttribute.cs | 12 ++ .../TestKit/BaseFixtureFactory.cs | 22 ++ .../xunit.runner.json | 3 + .../ClassicCombatOutcomeHandlerTests.cs | 53 +++++ 72 files changed, 2083 insertions(+), 274 deletions(-) create mode 100644 docs/getting-started.md create mode 100644 docsite/docs/customizing.md create mode 100644 docsite/docs/rulesets.md create mode 100644 samples/SharpMud.Samples.Classic/ClassicCombatOutcomeHandler.cs delete mode 100644 samples/SharpMud.Samples.Classic/ClassicCommands.cs create mode 100644 src/SharpMud.Ruleset.Basic/BasicBehaviorMappingContributor.cs create mode 100644 src/SharpMud.Ruleset.Basic/BasicCombatOutcomeHandler.cs create mode 100644 src/SharpMud.Ruleset.Basic/BasicPlayerFactory.cs create mode 100644 src/SharpMud.Ruleset.Basic/BasicRulesetOptions.cs create mode 100644 src/SharpMud.Ruleset.Basic/BasicStatsBehavior.cs create mode 100644 src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs create mode 100644 src/SharpMud.Ruleset.Basic/Configurations/BasicStatsBehaviorConfiguration.cs create mode 100644 src/SharpMud.Ruleset.Basic/ServiceCollectionExtensions.cs create mode 100644 src/SharpMud.Ruleset.Basic/SharpMud.Ruleset.Basic.csproj rename {samples/SharpMud.Samples.Classic => src/SharpMud.Ruleset.Rpg}/AttackCommand.cs (73%) rename {samples/SharpMud.Samples.Classic => src/SharpMud.Ruleset.Rpg}/CombatManager.cs (67%) rename {samples/SharpMud.Samples.Classic => src/SharpMud.Ruleset.Rpg}/CombatResolver.cs (58%) rename {samples/SharpMud.Samples.Classic => src/SharpMud.Ruleset.Rpg}/CombatantBehavior.cs (53%) rename {samples/SharpMud.Samples.Classic => src/SharpMud.Ruleset.Rpg}/Configurations/CombatantBehaviorConfiguration.cs (86%) create mode 100644 src/SharpMud.Ruleset.Rpg/DiceRoller.cs rename {samples/SharpMud.Samples.Classic => src/SharpMud.Ruleset.Rpg}/FleeCommand.cs (69%) rename {samples/SharpMud.Samples.Classic => src/SharpMud.Ruleset.Rpg}/ICombatManager.cs (94%) create mode 100644 src/SharpMud.Ruleset.Rpg/ICombatOutcomeHandler.cs rename {samples/SharpMud.Samples.Classic => src/SharpMud.Ruleset.Rpg}/ICombatResolver.cs (59%) create mode 100644 src/SharpMud.Ruleset.Rpg/IDiceRoller.cs create mode 100644 src/SharpMud.Ruleset.Rpg/RpgBehaviorMappingContributor.cs create mode 100644 src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs create mode 100644 src/SharpMud.Ruleset.Rpg/SharpMud.Ruleset.Rpg.csproj create mode 100644 tests/SharpMud.Ruleset.Basic.Tests/BasicPlayerFactoryTests.cs create mode 100644 tests/SharpMud.Ruleset.Basic.Tests/BasicWorldBuilderTests.cs create mode 100644 tests/SharpMud.Ruleset.Basic.Tests/PersistenceRoundTripTests.cs create mode 100644 tests/SharpMud.Ruleset.Basic.Tests/ServiceCollectionExtensionsTests.cs create mode 100644 tests/SharpMud.Ruleset.Basic.Tests/SharpMud.Ruleset.Basic.Tests.csproj create mode 100644 tests/SharpMud.Ruleset.Basic.Tests/TestKit/Attributes/BasicAutoDataAttribute.cs create mode 100644 tests/SharpMud.Ruleset.Basic.Tests/TestKit/BaseFixtureFactory.cs create mode 100644 tests/SharpMud.Ruleset.Basic.Tests/TestKit/TestDbContextFactory.cs create mode 100644 tests/SharpMud.Ruleset.Basic.Tests/xunit.runner.json rename tests/{SharpMud.Samples.Classic.Tests => SharpMud.Ruleset.Rpg.Tests}/Combat/CombatManagerTests.cs (76%) rename tests/{SharpMud.Samples.Classic.Tests => SharpMud.Ruleset.Rpg.Tests}/Combat/CombatResolverTests.cs (83%) create mode 100644 tests/SharpMud.Ruleset.Rpg.Tests/Combat/DiceRollerTests.cs create mode 100644 tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs create mode 100644 tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs create mode 100644 tests/SharpMud.Ruleset.Rpg.Tests/ServiceCollectionExtensionsTests.cs create mode 100644 tests/SharpMud.Ruleset.Rpg.Tests/SharpMud.Ruleset.Rpg.Tests.csproj create mode 100644 tests/SharpMud.Ruleset.Rpg.Tests/TestKit/Attributes/RpgAutoDataAttribute.cs create mode 100644 tests/SharpMud.Ruleset.Rpg.Tests/TestKit/BaseFixtureFactory.cs create mode 100644 tests/SharpMud.Ruleset.Rpg.Tests/xunit.runner.json create mode 100644 tests/SharpMud.Samples.Classic.Tests/ClassicCombatOutcomeHandlerTests.cs diff --git a/.github/dependabot.yml b/.github/dependabot.yml index c327a78..83d690d 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -34,6 +34,19 @@ updates: - "major" patterns: - "*" + # These are pinned as version ranges inside a TargetFramework-conditional + # ItemGroup in Directory.Packages.props (parallel net10.0-stable/ + # net11.0-preview lines - see the comment there). Dependabot can't + # reliably resolve/update a conditional range, so it's ignored here and + # bumped by hand instead. + ignore: + - dependency-name: "Microsoft.EntityFrameworkCore.Relational" + - dependency-name: "Microsoft.EntityFrameworkCore.Sqlite" + - dependency-name: "Microsoft.Extensions.Identity.Core" + - dependency-name: "Microsoft.Extensions.DependencyInjection" + - dependency-name: "Microsoft.Extensions.Hosting" + - dependency-name: "Microsoft.Extensions.Configuration" + - dependency-name: "EntityFrameworkCore.DynamoDb" - package-ecosystem: "dotnet-sdk" directory: "/" diff --git a/Directory.Packages.props b/Directory.Packages.props index 0aec333..fefa37a 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -9,17 +9,6 @@ Microsoft.Data.Sqlite.Core 10.0.9 pulls in - flagged NU1903 high severity (GHSA-2m69-gcr7-jv3q). --> - - - - + + + + [10.0.10, 11.0.0) + [11.0.0-preview.6.26359.118, 12.0.0-a) + - - - - + + + + - - + + + + + - - - - - - - + + + + + + + + + + diff --git a/src/SharpMud.Ruleset.Basic/BasicBehaviorMappingContributor.cs b/src/SharpMud.Ruleset.Basic/BasicBehaviorMappingContributor.cs new file mode 100644 index 0000000..3999d2b --- /dev/null +++ b/src/SharpMud.Ruleset.Basic/BasicBehaviorMappingContributor.cs @@ -0,0 +1,16 @@ +using Microsoft.EntityFrameworkCore; +using SharpMud.Persistence; + +namespace SharpMud.Ruleset.Basic; + +// Registers this package's own EF Core mapping for BasicStatsBehavior - +// scoped to this assembly's own IEntityTypeConfiguration<> types, same +// pattern as RpgBehaviorMappingContributor/ClassicBehaviorMappingContributor. +// Without this, a Basic world/player carrying BasicStatsBehavior hits the +// same unmapped TPH discriminator subtype problem CombatantBehavior would +// have without RpgBehaviorMappingContributor. +public sealed class BasicBehaviorMappingContributor : IBehaviorMappingContributor +{ + public void ConfigureBehaviors(ModelBuilder modelBuilder) => + modelBuilder.ApplyConfigurationsFromAssembly(typeof(BasicBehaviorMappingContributor).Assembly); +} diff --git a/src/SharpMud.Ruleset.Basic/BasicCombatOutcomeHandler.cs b/src/SharpMud.Ruleset.Basic/BasicCombatOutcomeHandler.cs new file mode 100644 index 0000000..2d21d32 --- /dev/null +++ b/src/SharpMud.Ruleset.Basic/BasicCombatOutcomeHandler.cs @@ -0,0 +1,57 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; +using SharpMud.Hosting; +using SharpMud.Ruleset.Rpg; + +namespace SharpMud.Ruleset.Basic; + +/// +/// Basic's - awards on a win, applies a flat XP-loss/ +/// HP-halving death penalty on a loss (same shape as Classic's, deliberately +/// simple), and always respawns at the world's starting room. Basic has no +/// "hub room" concept of its own - +/// already is one. +/// +public sealed class BasicCombatOutcomeHandler : ICombatOutcomeHandler +{ + private readonly WorldContext _worldContext; + + public BasicCombatOutcomeHandler(WorldContext worldContext) + { + _worldContext = worldContext; + } + + public async Task OnVictoryAsync(Thing victor, Thing defeated, CancellationToken ct) + { + var stats = victor.FindBehavior(); + var combatant = defeated.FindBehavior(); + if (stats is null || combatant is null) + return; + + stats.Experience += combatant.ExperienceReward; + + var session = victor.FindBehavior()?.Session; + if (session is not null) + await session.WriteLineAsync($"You gain {combatant.ExperienceReward} experience.", ct); + } + + public async Task OnDefeatAsync(Thing defeated, Thing victor, CancellationToken ct) + { + var stats = defeated.FindBehavior(); + if (stats is not null) + { + // XP-loss death penalty (docs/combat.md decision) - exact + // percentage is still an open item; 10% is a placeholder, + // matching Classic's. + long xpLoss = (long)(stats.Experience * 0.10); + stats.Experience = Math.Max(0, stats.Experience - xpLoss); + + var session = defeated.FindBehavior()?.Session; + if (session is not null) + await session.WriteLineAsync($"You lose {xpLoss} experience and awaken back in the clearing.", ct); + } + + return _worldContext.StartingRoom; + } +} diff --git a/src/SharpMud.Ruleset.Basic/BasicPlayerFactory.cs b/src/SharpMud.Ruleset.Basic/BasicPlayerFactory.cs new file mode 100644 index 0000000..62cdcca --- /dev/null +++ b/src/SharpMud.Ruleset.Basic/BasicPlayerFactory.cs @@ -0,0 +1,42 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; +using SharpMud.Hosting; +using SharpMud.Ruleset.Rpg; + +namespace SharpMud.Ruleset.Basic; + +/// +/// The Basic ruleset's - without this, / (which constructor-inject +/// ) can't create a fresh CLI/Telnet player at +/// all, and the quick-start fails at first login. +/// +public sealed class BasicPlayerFactory : IPlayerFactory +{ + private readonly BasicRulesetOptions _options; + + public BasicPlayerFactory(BasicRulesetOptions options) + { + _options = options; + } + + public Thing CreatePlayer(World world, string username, string passwordHash, Thing startingRoom) + { + var player = new Thing { Id = ThingId.New(), Name = username }; + player.Behaviors.Add(new PlayerBehavior { Username = username, PasswordHash = passwordHash }); + player.Behaviors.Add(new EquippedBehavior()); + player.Behaviors.Add(new BasicStatsBehavior()); + player.Behaviors.Add(new CombatantBehavior + { + MaxHitPoints = _options.StartingHitPoints, + CurrentHitPoints = _options.StartingHitPoints, + ArmorClass = _options.StartingArmorClass, + DamageMin = _options.StartingDamageMin, + DamageMax = _options.StartingDamageMax, + }); + + startingRoom.Add(player); + world.Register(player); + return player; + } +} diff --git a/src/SharpMud.Ruleset.Basic/BasicRulesetOptions.cs b/src/SharpMud.Ruleset.Basic/BasicRulesetOptions.cs new file mode 100644 index 0000000..0f51ea7 --- /dev/null +++ b/src/SharpMud.Ruleset.Basic/BasicRulesetOptions.cs @@ -0,0 +1,14 @@ +namespace SharpMud.Ruleset.Basic; + +// Plain mutable options class configured via AddSharpMudBasicRuleset(...)'s +// callback, not IOptions/appsettings.json-bound - same shape as Engine's +// GameLoopOptions. These are the tunable starting numbers for a fresh +// player character; the default default world's NPC keeps its own fixed +// stats regardless (small, deliberately simple content, not a tunable). +public sealed class BasicRulesetOptions +{ + public int StartingHitPoints { get; set; } = 20; + public int StartingArmorClass { get; set; } = 10; + public int StartingDamageMin { get; set; } = 1; + public int StartingDamageMax { get; set; } = 4; +} diff --git a/src/SharpMud.Ruleset.Basic/BasicStatsBehavior.cs b/src/SharpMud.Ruleset.Basic/BasicStatsBehavior.cs new file mode 100644 index 0000000..34d4223 --- /dev/null +++ b/src/SharpMud.Ruleset.Basic/BasicStatsBehavior.cs @@ -0,0 +1,13 @@ +using SharpMud.Engine.Core; + +namespace SharpMud.Ruleset.Basic; + +// Deliberately minimal - plain numeric attributes only, no Race/CharacterClass +// (that's Classic-flavored content, not what Basic promises). Attached +// alongside SharpMud.Engine's PlayerBehavior to make a Thing a character; +// SharpMud.Ruleset.Rpg's CombatantBehavior handles the actual combat numbers. +public sealed class BasicStatsBehavior : Behavior +{ + public int Level { get; set; } = 1; + public long Experience { get; set; } +} diff --git a/src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs b/src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs new file mode 100644 index 0000000..b5b4c20 --- /dev/null +++ b/src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs @@ -0,0 +1,83 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; +using SharpMud.Hosting; +using SharpMud.Ruleset.Rpg; + +namespace SharpMud.Ruleset.Basic; + +/// +/// The default small world a fresh SharpMud.Ruleset.Basic consumer +/// gets for free - two rooms and one fightable NPC, enough to walk around +/// and issue attack/flee against something without writing any +/// world content of their own. A real game still wants its own ; this is the quick-start default. +/// +public sealed class BasicWorldBuilder : IWorldBuilder +{ + // Fixed, not ThingId.New() - so a fresh boot can ask the repository + // "does this already exist?" instead of always rebuilding. See + // docs/persistence.md. + public static readonly ThingId AreaId = new(Guid.Parse("00000000-0000-0000-0000-000000000002")); + + public ThingId RootId => AreaId; + + public (World World, Thing StartingRoom) Build() + { + var world = new World(); + + var area = new Thing { Id = AreaId, Name = "The Basic World" }; + area.Behaviors.Add(new AreaBehavior()); + world.Register(area); + + var clearing = CreateRoom(world, area, "Clearing", + "A quiet clearing ringed by tall grass. A worn path leads north."); + var watchtower = CreateRoom(world, area, "Old Watchtower", + "A crumbling stone watchtower, long abandoned. Something rustles nearby."); + + Connect(world, clearing, watchtower, Direction.North); + + var boar = new Thing { Id = ThingId.New(), Name = "wild boar" }; + boar.Behaviors.Add(new NpcBehavior()); + boar.Behaviors.Add(new CombatantBehavior + { + MaxHitPoints = 8, + CurrentHitPoints = 8, + ArmorClass = 8, + DamageMin = 1, + DamageMax = 3, + ExperienceReward = 10, + }); + watchtower.Add(boar); + world.Register(boar); + + return (world, clearing); + } + + public Thing FindStartingRoom(Thing root) => + root.Children.FirstOrDefault(c => c.HasBehavior() && c.Name == "Clearing") + ?? root.Children.First(c => c.HasBehavior()); + + private static Thing CreateRoom(World world, Thing area, string name, string description) + { + var room = new Thing { Id = ThingId.New(), Name = name, Description = description }; + room.Behaviors.Add(new RoomBehavior()); + area.Add(room); + 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/src/SharpMud.Ruleset.Basic/Configurations/BasicStatsBehaviorConfiguration.cs b/src/SharpMud.Ruleset.Basic/Configurations/BasicStatsBehaviorConfiguration.cs new file mode 100644 index 0000000..8eab19b --- /dev/null +++ b/src/SharpMud.Ruleset.Basic/Configurations/BasicStatsBehaviorConfiguration.cs @@ -0,0 +1,12 @@ +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Metadata.Builders; + +namespace SharpMud.Ruleset.Basic.Configurations; + +public sealed class BasicStatsBehaviorConfiguration : IEntityTypeConfiguration +{ + public void Configure(EntityTypeBuilder builder) + { + // All plain int/long properties - default mapping. + } +} diff --git a/src/SharpMud.Ruleset.Basic/ServiceCollectionExtensions.cs b/src/SharpMud.Ruleset.Basic/ServiceCollectionExtensions.cs new file mode 100644 index 0000000..6bb2469 --- /dev/null +++ b/src/SharpMud.Ruleset.Basic/ServiceCollectionExtensions.cs @@ -0,0 +1,41 @@ +using Microsoft.Extensions.DependencyInjection; +using SharpMud.Engine.Commands; +using SharpMud.Hosting; +using SharpMud.Persistence; +using SharpMud.Ruleset.Rpg; + +namespace SharpMud.Ruleset.Basic; + +public static class ServiceCollectionExtensions +{ + /// + /// Registers everything a consumer needs for a runnable, playable basic + /// game on top of SharpMud.Ruleset.Rpg's combat scaffolding - + /// this package's stats behavior mapping, default world, player + /// factory, and combat-outcome handler. Combined with Engine/ + /// Hosting/a persistence provider/a transport adapter, this is + /// the actual "dotnet add package, a few lines in + /// Program.cs, run a basic game" quick-start + /// (docs/adr/0008-ruleset-scaffolding-tier.md). + /// + /// The consumer's . + /// Tunes the starting numbers for a fresh player character - see . + /// Forwarded to AddSharpMudRpgRuleset(...) - a consumer's own commands, registered alongside kill/attack/flee. + public static IServiceCollection AddSharpMudBasicRuleset( + this IServiceCollection services, + Action? configureOptions = null, + Action? registerConsumerCommands = null) + { + var options = new BasicRulesetOptions(); + configureOptions?.Invoke(options); + services.AddSingleton(options); + + services.AddSingleton(); + services.AddSharpMudWorld(); + services.AddSharpMudPlayerFactory(); + + services.AddSharpMudRpgRuleset(registerConsumerCommands); + + return services; + } +} diff --git a/src/SharpMud.Ruleset.Basic/SharpMud.Ruleset.Basic.csproj b/src/SharpMud.Ruleset.Basic/SharpMud.Ruleset.Basic.csproj new file mode 100644 index 0000000..54a8eec --- /dev/null +++ b/src/SharpMud.Ruleset.Basic/SharpMud.Ruleset.Basic.csproj @@ -0,0 +1,17 @@ + + + + + + + + + + + net10.0;net11.0 + enable + enable + Minimal, deliberately simple concrete ruleset built on SharpMud.Ruleset.Rpg - a plain numeric stat block, a small default world with a fightable NPC, and AddSharpMudBasicRuleset(...) for a true "dotnet add package, few lines in Program.cs, run a basic game" quick-start. + + + diff --git a/samples/SharpMud.Samples.Classic/AttackCommand.cs b/src/SharpMud.Ruleset.Rpg/AttackCommand.cs similarity index 73% rename from samples/SharpMud.Samples.Classic/AttackCommand.cs rename to src/SharpMud.Ruleset.Rpg/AttackCommand.cs index a270776..44250f8 100644 --- a/samples/SharpMud.Samples.Classic/AttackCommand.cs +++ b/src/SharpMud.Ruleset.Rpg/AttackCommand.cs @@ -1,10 +1,17 @@ using SharpMud.Engine.Behaviors; using SharpMud.Engine.Commands; -namespace SharpMud.Samples.Classic; +namespace SharpMud.Ruleset.Rpg; -public sealed class AttackCommand(ICombatManager combatManager) : ICommand +public sealed class AttackCommand : ICommand { + private readonly ICombatManager _combatManager; + + public AttackCommand(ICombatManager combatManager) + { + _combatManager = combatManager; + } + public string Verb => "kill"; public IReadOnlyList Aliases { get; } = ["attack"]; @@ -13,7 +20,7 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) if (await CommandGuards.RequireArgsAsync(ctx, "Kill what?", ct)) return; - if (combatManager.IsInCombat(ctx.Actor.Id)) + if (_combatManager.IsInCombat(ctx.Actor.Id)) { await ctx.Session.WriteLineAsync("You are already fighting!", ct); return; @@ -31,7 +38,7 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) return; } - combatManager.StartEncounter(ctx.Actor, target); + _combatManager.StartEncounter(ctx.Actor, target); await ctx.Session.WriteLineAsync($"You attack {target.Name}!", ct); } } diff --git a/samples/SharpMud.Samples.Classic/CombatManager.cs b/src/SharpMud.Ruleset.Rpg/CombatManager.cs similarity index 67% rename from samples/SharpMud.Samples.Classic/CombatManager.cs rename to src/SharpMud.Ruleset.Rpg/CombatManager.cs index f5306db..654e9a0 100644 --- a/samples/SharpMud.Samples.Classic/CombatManager.cs +++ b/src/SharpMud.Ruleset.Rpg/CombatManager.cs @@ -5,16 +5,30 @@ using SharpMud.Engine.Sessions; using SharpMud.Engine.Ticking; -namespace SharpMud.Samples.Classic; +namespace SharpMud.Ruleset.Rpg; // Registered once with IGameLoop and resolves every active encounter each // tick - simpler Host wiring than one ITickable per encounter, and all // world-state mutation happens on the single game-loop "thread" (sequential // awaits in GameLoop.RunAsync), so there's no concurrent-mutation risk. -public sealed class CombatManager(ICombatResolver resolver, Thing hubRoom) : ICombatManager, ITickable +// +// Combat-outcome side effects (XP awards, death penalties, respawn +// destination) are delegated to ICombatOutcomeHandler rather than touching a +// concrete ruleset's stats behavior or a hard-coded room directly - this +// package has zero reference to any concrete ruleset's types (see +// docs/adr/0008-ruleset-scaffolding-tier.md's Decision Outcome). +public sealed class CombatManager : ICombatManager, ITickable { + private readonly ICombatResolver _resolver; + private readonly ICombatOutcomeHandler _outcomeHandler; private readonly Dictionary _encounters = []; + public CombatManager(ICombatResolver resolver, ICombatOutcomeHandler outcomeHandler) + { + _resolver = resolver; + _outcomeHandler = outcomeHandler; + } + public bool IsInCombat(ThingId thingId) => _encounters.ContainsKey(thingId); public void StartEncounter(Thing attacker, Thing defender) => @@ -52,7 +66,7 @@ public async Task OnTickAsync(TickContext ctx, CancellationToken ct) // Not Linkdead (checked above), so Session is the live, connected session. var session = attackerBehavior.Session!; - var attackResult = resolver.ResolveRound(encounter.Attacker, encounter.Defender); + var attackResult = _resolver.ResolveRound(encounter.Attacker, encounter.Defender); await session.WriteLineAsync( attackResult.Hit ? $"You hit {encounter.Defender.Name} for {attackResult.Damage} damage." @@ -66,7 +80,7 @@ await session.WriteLineAsync( } // Classic mutual combat: the defender strikes back the same round. - var counterResult = resolver.ResolveRound(encounter.Defender, encounter.Attacker); + var counterResult = _resolver.ResolveRound(encounter.Defender, encounter.Attacker); await session.WriteLineAsync( counterResult.Hit ? $"{encounter.Defender.Name} hits you for {counterResult.Damage} damage." @@ -82,13 +96,7 @@ private async Task HandleDefenderDefeatedAsync(CombatEncounter encounter, ISessi { await session.WriteLineAsync($"You have slain {encounter.Defender.Name}!", ct); - var combatant = encounter.Defender.FindBehavior()!; - var stats = encounter.Attacker.FindBehavior(); - if (stats is not null) - { - stats.Experience += combatant.ExperienceReward; - await session.WriteLineAsync($"You gain {combatant.ExperienceReward} experience.", ct); - } + await _outcomeHandler.OnVictoryAsync(encounter.Attacker, encounter.Defender, ct); encounter.Defender.Parent?.Remove(encounter.Defender); _encounters.Remove(encounter.Attacker.Id); @@ -97,28 +105,27 @@ private async Task HandleDefenderDefeatedAsync(CombatEncounter encounter, ISessi private async Task HandleAttackerDefeatedAsync(CombatEncounter encounter, ISession session, CancellationToken ct) { var attacker = encounter.Attacker; - var stats = attacker.FindBehavior(); - // XP-loss death penalty (docs/combat.md decision) - exact percentage - // is still an open item; 10% is a placeholder. - long xpLoss = 0; - if (stats is not null) - { - xpLoss = (long)(stats.Experience * 0.10); - stats.Experience = Math.Max(0, stats.Experience - xpLoss); + // Real, pre-existing bug fixed here: CombatResolver reads/writes + // damage against CombatantBehavior.CurrentHitPoints, not any + // ruleset-specific stats behavior. A respawn that only reset the + // latter left CombatantBehavior.CurrentHitPoints at/below 0, so the + // very next hit instantly re-triggered "defeated" regardless of the + // roll. This reset is generic (CombatantBehavior is this package's + // own type) so it happens here, unconditionally, before the + // ruleset-specific outcome handler runs. + var combatant = attacker.FindBehavior()!; + combatant.CurrentHitPoints = combatant.MaxHitPoints; - // Respawn HP fraction is also an open item; 50% is a placeholder. - stats.CurrentHitPoints = Math.Max(1, stats.MaxHitPoints / 2); - } + var destination = await _outcomeHandler.OnDefeatAsync(attacker, encounter.Defender, ct); await session.WriteLineAsync($"{encounter.Defender.Name} has slain you!", ct); - await session.WriteLineAsync($"You lose {xpLoss} experience and awaken back in town.", ct); _encounters.Remove(attacker.Id); attacker.Parent?.Remove(attacker); - hubRoom.Add(attacker); + destination.Add(attacker); - await LookCommand.SendRoomDescriptionAsync(attacker, hubRoom, ct); + await LookCommand.SendRoomDescriptionAsync(attacker, destination, ct); } } diff --git a/samples/SharpMud.Samples.Classic/CombatResolver.cs b/src/SharpMud.Ruleset.Rpg/CombatResolver.cs similarity index 58% rename from samples/SharpMud.Samples.Classic/CombatResolver.cs rename to src/SharpMud.Ruleset.Rpg/CombatResolver.cs index d7bfc5c..a25f414 100644 --- a/samples/SharpMud.Samples.Classic/CombatResolver.cs +++ b/src/SharpMud.Ruleset.Rpg/CombatResolver.cs @@ -1,23 +1,35 @@ using SharpMud.Engine.Core; -namespace SharpMud.Samples.Classic; +namespace SharpMud.Ruleset.Rpg; // Diku/Circle-style d20-vs-AC roll (docs/combat.md decision). Level/skill // to-hit modifiers and the exact damage formula are still open items there - // this is currently an unmodified d20 roll against the defender's AC. -public sealed class CombatResolver(IRandomSource random) : ICombatResolver +public sealed class CombatResolver : ICombatResolver { + private readonly IDiceRoller _dice; + private readonly IRandomSource _random; + + public CombatResolver(IDiceRoller dice, IRandomSource random) + { + _dice = dice; + _random = random; + } + public CombatRoundResult ResolveRound(Thing attacker, Thing defender) { var attackerCombatant = attacker.FindBehavior()!; var defenderCombatant = defender.FindBehavior()!; - var toHitRoll = random.Next(1, 20); + var toHitRoll = _dice.Roll(1, 20); if (toHitRoll < defenderCombatant.ArmorClass) return new CombatRoundResult(false, 0, false); + // Damage range isn't dice notation (arbitrary min/max, not 1-based + // per-die), so this stays a direct IRandomSource roll rather than + // going through IDiceRoller. var (min, max) = attackerCombatant.DamageRange; - var damage = random.Next(min, max); + var damage = _random.Next(min, max); defenderCombatant.CurrentHitPoints -= damage; return new CombatRoundResult(true, damage, defenderCombatant.CurrentHitPoints <= 0); diff --git a/samples/SharpMud.Samples.Classic/CombatantBehavior.cs b/src/SharpMud.Ruleset.Rpg/CombatantBehavior.cs similarity index 53% rename from samples/SharpMud.Samples.Classic/CombatantBehavior.cs rename to src/SharpMud.Ruleset.Rpg/CombatantBehavior.cs index 6a21d4c..eb28961 100644 --- a/samples/SharpMud.Samples.Classic/CombatantBehavior.cs +++ b/src/SharpMud.Ruleset.Rpg/CombatantBehavior.cs @@ -1,11 +1,13 @@ using SharpMud.Engine.Core; -namespace SharpMud.Samples.Classic; +namespace SharpMud.Ruleset.Rpg; -// What used to be the ICombatant interface Player/Npc implemented - now a -// behavior any Thing can carry (a hostile plant, a turret - anything the -// ruleset wants to be able to fight), independent of StatsBehavior so an NPC -// doesn't need a full character sheet just to throw a punch. +/// +/// HP/armor-class/damage-range/XP-reward - a any +/// can carry (a hostile plant, a turret - anything the +/// ruleset wants to be able to fight), independent of any stats behavior so +/// an NPC doesn't need a full character sheet just to throw a punch. +/// public sealed class CombatantBehavior : Behavior { public int MaxHitPoints { get; set; } diff --git a/samples/SharpMud.Samples.Classic/Configurations/CombatantBehaviorConfiguration.cs b/src/SharpMud.Ruleset.Rpg/Configurations/CombatantBehaviorConfiguration.cs similarity index 86% rename from samples/SharpMud.Samples.Classic/Configurations/CombatantBehaviorConfiguration.cs rename to src/SharpMud.Ruleset.Rpg/Configurations/CombatantBehaviorConfiguration.cs index 6c95798..7a57024 100644 --- a/samples/SharpMud.Samples.Classic/Configurations/CombatantBehaviorConfiguration.cs +++ b/src/SharpMud.Ruleset.Rpg/Configurations/CombatantBehaviorConfiguration.cs @@ -1,7 +1,7 @@ using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Metadata.Builders; -namespace SharpMud.Samples.Classic.Configurations; +namespace SharpMud.Ruleset.Rpg.Configurations; public sealed class CombatantBehaviorConfiguration : IEntityTypeConfiguration { diff --git a/src/SharpMud.Ruleset.Rpg/DiceRoller.cs b/src/SharpMud.Ruleset.Rpg/DiceRoller.cs new file mode 100644 index 0000000..f3007c6 --- /dev/null +++ b/src/SharpMud.Ruleset.Rpg/DiceRoller.cs @@ -0,0 +1,25 @@ +using SharpMud.Engine.Core; + +namespace SharpMud.Ruleset.Rpg; + +public sealed class DiceRoller : IDiceRoller +{ + private readonly IRandomSource _random; + + public DiceRoller(IRandomSource random) + { + _random = random; + } + + public int Roll(int diceCount, int sides, int modifier = 0) + { + ArgumentOutOfRangeException.ThrowIfLessThan(diceCount, 1); + ArgumentOutOfRangeException.ThrowIfLessThan(sides, 1); + + var total = modifier; + for (var i = 0; i < diceCount; i++) + total += _random.Next(1, sides); + + return total; + } +} diff --git a/samples/SharpMud.Samples.Classic/FleeCommand.cs b/src/SharpMud.Ruleset.Rpg/FleeCommand.cs similarity index 69% rename from samples/SharpMud.Samples.Classic/FleeCommand.cs rename to src/SharpMud.Ruleset.Rpg/FleeCommand.cs index 9cd727e..079b260 100644 --- a/samples/SharpMud.Samples.Classic/FleeCommand.cs +++ b/src/SharpMud.Ruleset.Rpg/FleeCommand.cs @@ -3,16 +3,27 @@ using SharpMud.Engine.Commands.Builtin; using SharpMud.Engine.Core; -namespace SharpMud.Samples.Classic; +namespace SharpMud.Ruleset.Rpg; -public sealed class FleeCommand(ICombatManager combatManager, IRandomSource random) : ICommand +public sealed class FleeCommand : ICommand { + private readonly ICombatManager _combatManager; + private readonly IDiceRoller _dice; + private readonly IRandomSource _random; + + public FleeCommand(ICombatManager combatManager, IDiceRoller dice, IRandomSource random) + { + _combatManager = combatManager; + _dice = dice; + _random = random; + } + public string Verb => "flee"; public IReadOnlyList Aliases { get; } = []; public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) { - if (!combatManager.TryGetEncounter(ctx.Actor.Id, out _)) + if (!_combatManager.TryGetEncounter(ctx.Actor.Id, out _)) { await ctx.Session.WriteLineAsync("You aren't fighting anything.", ct); return; @@ -27,17 +38,17 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) // Success chance: DEX-influenced per docs/combat.md, but the exact // formula is still an open item there. Flat 60% until it's defined. - var success = random.Next(1, 100) <= 60; + var success = _dice.Roll(1, 100) <= 60; if (!success) { await ctx.Session.WriteLineAsync("You fail to escape!", ct); return; } - var exit = exits[random.Next(0, exits.Count - 1)]; + var exit = exits[_random.Next(0, exits.Count - 1)]; var destination = exit.Destination; - combatManager.EndEncounter(ctx.Actor.Id); + _combatManager.EndEncounter(ctx.Actor.Id); await ctx.Session.WriteLineAsync($"You flee {exit.Direction.ToDisplayString()}!", ct); await RoomBroadcast.ToOccupantsAsync( diff --git a/samples/SharpMud.Samples.Classic/ICombatManager.cs b/src/SharpMud.Ruleset.Rpg/ICombatManager.cs similarity index 94% rename from samples/SharpMud.Samples.Classic/ICombatManager.cs rename to src/SharpMud.Ruleset.Rpg/ICombatManager.cs index 044f309..8484d60 100644 --- a/samples/SharpMud.Samples.Classic/ICombatManager.cs +++ b/src/SharpMud.Ruleset.Rpg/ICombatManager.cs @@ -1,7 +1,7 @@ using System.Diagnostics.CodeAnalysis; using SharpMud.Engine.Core; -namespace SharpMud.Samples.Classic; +namespace SharpMud.Ruleset.Rpg; public sealed class CombatEncounter { diff --git a/src/SharpMud.Ruleset.Rpg/ICombatOutcomeHandler.cs b/src/SharpMud.Ruleset.Rpg/ICombatOutcomeHandler.cs new file mode 100644 index 0000000..8bd106a --- /dev/null +++ b/src/SharpMud.Ruleset.Rpg/ICombatOutcomeHandler.cs @@ -0,0 +1,32 @@ +using SharpMud.Engine.Core; + +namespace SharpMud.Ruleset.Rpg; + +/// +/// A ruleset's hook into combat outcomes - owns +/// generic encounter bookkeeping (round resolution, freezing on linkdead, +/// resetting ) but has no +/// concept of a ruleset's own stats/leveling behavior or where a defeated +/// character should respawn. Implemented once per ruleset (e.g. Classic +/// touches its StatsBehavior's XP; a ruleset with no leveling concept +/// at all can no-op the reward side) and registered via +/// AddSharpMudRpgRuleset<TCombatOutcomeHandler>(...). See +/// docs/adr/0008-ruleset-scaffolding-tier.md's Decision Outcome for why this +/// mechanism replaces CombatManager's prior direct StatsBehavior +/// touches and hard-coded respawn room. +/// +public interface ICombatOutcomeHandler +{ + /// Called when defeats - the hook for awarding XP/rewards. + Task OnVictoryAsync(Thing victor, Thing defeated, CancellationToken ct); + + /// + /// Called when loses the encounter to + /// - the hook for a death penalty (XP loss, + /// stats-specific HP reset) and for deciding the respawn destination. + /// is already reset by + /// before this is called, regardless of what + /// this method does. + /// + Task OnDefeatAsync(Thing defeated, Thing victor, CancellationToken ct); +} diff --git a/samples/SharpMud.Samples.Classic/ICombatResolver.cs b/src/SharpMud.Ruleset.Rpg/ICombatResolver.cs similarity index 59% rename from samples/SharpMud.Samples.Classic/ICombatResolver.cs rename to src/SharpMud.Ruleset.Rpg/ICombatResolver.cs index 8102e23..50c289c 100644 --- a/samples/SharpMud.Samples.Classic/ICombatResolver.cs +++ b/src/SharpMud.Ruleset.Rpg/ICombatResolver.cs @@ -1,9 +1,10 @@ using SharpMud.Engine.Core; -namespace SharpMud.Samples.Classic; +namespace SharpMud.Ruleset.Rpg; public sealed record CombatRoundResult(bool Hit, int Damage, bool DefenderDefeated); +/// Resolves a single round of combat between two -carrying Things. public interface ICombatResolver { CombatRoundResult ResolveRound(Thing attacker, Thing defender); diff --git a/src/SharpMud.Ruleset.Rpg/IDiceRoller.cs b/src/SharpMud.Ruleset.Rpg/IDiceRoller.cs new file mode 100644 index 0000000..0ff0085 --- /dev/null +++ b/src/SharpMud.Ruleset.Rpg/IDiceRoller.cs @@ -0,0 +1,14 @@ +namespace SharpMud.Ruleset.Rpg; + +/// +/// "N dice of M sides plus a modifier" over the engine's - a generic RPG mechanic, not +/// bare randomness (so it doesn't belong in Engine) and not tied to any one +/// ruleset's specific formulas (so it doesn't belong in a concrete leaf +/// package either). See docs/adr/0008-ruleset-scaffolding-tier.md. +/// +public interface IDiceRoller +{ + /// Rolls dice of sides each and adds . + int Roll(int diceCount, int sides, int modifier = 0); +} diff --git a/src/SharpMud.Ruleset.Rpg/RpgBehaviorMappingContributor.cs b/src/SharpMud.Ruleset.Rpg/RpgBehaviorMappingContributor.cs new file mode 100644 index 0000000..78ec936 --- /dev/null +++ b/src/SharpMud.Ruleset.Rpg/RpgBehaviorMappingContributor.cs @@ -0,0 +1,16 @@ +using Microsoft.EntityFrameworkCore; +using SharpMud.Persistence; + +namespace SharpMud.Ruleset.Rpg; + +// Registers this package's own EF Core mapping for CombatantBehavior - +// Persistence never references SharpMud.Ruleset.Rpg directly, per +// docs/persistence.md. Scoped to this assembly's own +// IEntityTypeConfiguration<> types, same pattern as +// ClassicBehaviorMappingContributor - a consumer's own contributor scans +// its own assembly, never this one's. +public sealed class RpgBehaviorMappingContributor : IBehaviorMappingContributor +{ + public void ConfigureBehaviors(ModelBuilder modelBuilder) => + modelBuilder.ApplyConfigurationsFromAssembly(typeof(RpgBehaviorMappingContributor).Assembly); +} diff --git a/src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs b/src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs new file mode 100644 index 0000000..8be25f0 --- /dev/null +++ b/src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs @@ -0,0 +1,63 @@ +using Microsoft.Extensions.DependencyInjection; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Core; +using SharpMud.Engine.Ticking; +using SharpMud.Hosting; +using SharpMud.Persistence; + +namespace SharpMud.Ruleset.Rpg; + +public static class ServiceCollectionExtensions +{ + /// + /// Registers this package's combat scaffolding - , (as both + /// itself and , off the same instance), the dice + /// service, this package's , + /// and the kill/attack/flee commands - reproducing + /// what every consumer previously had to hand-wire in their own + /// Program.cs. is + /// the consumer's own implementation + /// (XP awards, death penalty, respawn destination). + /// + /// The consumer's . + /// + /// Optional callback for the consumer's own commands, invoked after this + /// package's own commands are registered. This package calls the + /// underlying Hosting.AddSharpMudRuleset(...) exactly once + /// internally - a consumer must not call it again themselves, since + /// DI's last-registration-wins resolution for + /// would silently drop whichever call came first (see + /// docs/adr/0008-ruleset-scaffolding-tier.md's Decision Outcome). + /// + public static IServiceCollection AddSharpMudRpgRuleset( + this IServiceCollection services, + Action? registerConsumerCommands = null) + where TCombatOutcomeHandler : class, ICombatOutcomeHandler + { + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + + // Registered once as ICombatManager and once as ITickable, same + // underlying instance - CombatManager both drives the kill/flee + // commands and advances active encounters each tick. + services.AddSingleton(sp => + new CombatManager(sp.GetRequiredService(), sp.GetRequiredService())); + services.AddSingleton(sp => (ITickable)sp.GetRequiredService()); + + services.AddSharpMudRuleset((sp, registry) => + { + registry.Register(new AttackCommand(sp.GetRequiredService())); + registry.Register(new FleeCommand( + sp.GetRequiredService(), + sp.GetRequiredService(), + sp.GetRequiredService())); + + registerConsumerCommands?.Invoke(sp, registry); + }); + + return services; + } +} diff --git a/src/SharpMud.Ruleset.Rpg/SharpMud.Ruleset.Rpg.csproj b/src/SharpMud.Ruleset.Rpg/SharpMud.Ruleset.Rpg.csproj new file mode 100644 index 0000000..21edae2 --- /dev/null +++ b/src/SharpMud.Ruleset.Rpg/SharpMud.Ruleset.Rpg.csproj @@ -0,0 +1,20 @@ + + + + + + + + + + + net10.0;net11.0 + enable + enable + Reusable RPG scaffolding between SharpMud.Engine and a concrete ruleset - CombatantBehavior, combat resolution/encounter tracking, attack/flee commands, and a dice-rolling abstraction over IRandomSource. + + + diff --git a/tests/SharpMud.Persistence.Tests/SharpMud.Persistence.Tests.csproj b/tests/SharpMud.Persistence.Tests/SharpMud.Persistence.Tests.csproj index eb990ec..2bf8194 100644 --- a/tests/SharpMud.Persistence.Tests/SharpMud.Persistence.Tests.csproj +++ b/tests/SharpMud.Persistence.Tests/SharpMud.Persistence.Tests.csproj @@ -42,6 +42,7 @@ + diff --git a/tests/SharpMud.Persistence.Tests/TestKit/TestDbContextFactory.cs b/tests/SharpMud.Persistence.Tests/TestKit/TestDbContextFactory.cs index 6d715f5..2bec471 100644 --- a/tests/SharpMud.Persistence.Tests/TestKit/TestDbContextFactory.cs +++ b/tests/SharpMud.Persistence.Tests/TestKit/TestDbContextFactory.cs @@ -1,4 +1,5 @@ using Microsoft.EntityFrameworkCore; +using SharpMud.Ruleset.Rpg; using SharpMud.Samples.Classic; namespace SharpMud.Persistence.Tests.TestKit; @@ -16,7 +17,7 @@ public GameDbContext CreateDbContext() var options = new DbContextOptionsBuilder() .UseSqlite($"Data Source={_dbPath}") .Options; - var context = new GameDbContext(options, [new ClassicBehaviorMappingContributor()]); + var context = new GameDbContext(options, [new ClassicBehaviorMappingContributor(), new RpgBehaviorMappingContributor()]); context.Database.EnsureCreated(); return context; } diff --git a/tests/SharpMud.Persistence.Tests/ThingRepositoryTests.cs b/tests/SharpMud.Persistence.Tests/ThingRepositoryTests.cs index 60e0ec6..23c47cc 100644 --- a/tests/SharpMud.Persistence.Tests/ThingRepositoryTests.cs +++ b/tests/SharpMud.Persistence.Tests/ThingRepositoryTests.cs @@ -1,6 +1,7 @@ using SharpMud.Engine.Behaviors; using SharpMud.Engine.Core; using SharpMud.Persistence.Tests.TestKit; +using SharpMud.Ruleset.Rpg; using SharpMud.Samples.Classic; namespace SharpMud.Persistence.Tests; diff --git a/tests/SharpMud.Ruleset.Basic.Tests/BasicPlayerFactoryTests.cs b/tests/SharpMud.Ruleset.Basic.Tests/BasicPlayerFactoryTests.cs new file mode 100644 index 0000000..12678a8 --- /dev/null +++ b/tests/SharpMud.Ruleset.Basic.Tests/BasicPlayerFactoryTests.cs @@ -0,0 +1,33 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; +using SharpMud.Ruleset.Rpg; + +namespace SharpMud.Ruleset.Basic.Tests; + +public sealed class BasicPlayerFactoryTests +{ + [Fact] + public void CreatePlayer_ReturnsThingWithPlayerAndBasicStatsAndCombatantBehaviors() + { + var options = new BasicRulesetOptions { StartingHitPoints = 15, StartingArmorClass = 9, StartingDamageMin = 2, StartingDamageMax = 5 }; + var sut = new BasicPlayerFactory(options); + var world = new World(); + var startingRoom = new Thing { Id = ThingId.New(), Name = "Clearing" }; + startingRoom.Behaviors.Add(new RoomBehavior()); + + var player = sut.CreatePlayer(world, "Adventurer", "hash", startingRoom); + + player.HasBehavior().Should().BeTrue(); + player.HasBehavior().Should().BeTrue(); + + var combatant = player.FindBehavior(); + combatant.Should().NotBeNull(); + combatant!.MaxHitPoints.Should().Be(15); + combatant.ArmorClass.Should().Be(9); + combatant.DamageMin.Should().Be(2); + combatant.DamageMax.Should().Be(5); + + player.Parent.Should().Be(startingRoom); + world.GetThing(player.Id).Should().Be(player); + } +} diff --git a/tests/SharpMud.Ruleset.Basic.Tests/BasicWorldBuilderTests.cs b/tests/SharpMud.Ruleset.Basic.Tests/BasicWorldBuilderTests.cs new file mode 100644 index 0000000..b7c855b --- /dev/null +++ b/tests/SharpMud.Ruleset.Basic.Tests/BasicWorldBuilderTests.cs @@ -0,0 +1,31 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Ruleset.Rpg; + +namespace SharpMud.Ruleset.Basic.Tests; + +public sealed class BasicWorldBuilderTests +{ + [Fact] + public void Build_ReturnsWorldWithAtLeastOneFightableNpc() + { + var sut = new BasicWorldBuilder(); + + var (world, startingRoom) = sut.Build(); + + var fightableNpcs = world.AllWithBehavior().Where(t => t.HasBehavior()); + fightableNpcs.Should().NotBeEmpty("a fresh character must be able to walk around and fight something"); + startingRoom.HasBehavior().Should().BeTrue(); + } + + [Fact] + public void FindStartingRoom_ReturnsTheClearing_AfterReload() + { + var sut = new BasicWorldBuilder(); + var (_, startingRoom) = sut.Build(); + var area = startingRoom.Parent!; + + var found = sut.FindStartingRoom(area); + + found.Should().Be(startingRoom); + } +} diff --git a/tests/SharpMud.Ruleset.Basic.Tests/PersistenceRoundTripTests.cs b/tests/SharpMud.Ruleset.Basic.Tests/PersistenceRoundTripTests.cs new file mode 100644 index 0000000..7f05c8b --- /dev/null +++ b/tests/SharpMud.Ruleset.Basic.Tests/PersistenceRoundTripTests.cs @@ -0,0 +1,47 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; +using SharpMud.Persistence; +using SharpMud.Ruleset.Basic.Tests.TestKit; +using SharpMud.Ruleset.Rpg; + +namespace SharpMud.Ruleset.Basic.Tests; + +public sealed class PersistenceRoundTripTests : IDisposable +{ + private readonly TestDbContextFactory _factory = new(); + private readonly ThingRepository _sut; + + public PersistenceRoundTripTests() + { + _sut = new ThingRepository(_factory); + } + + public void Dispose() => _factory.Dispose(); + + // Without BasicBehaviorMappingContributor actually being registered and + // discovered, a Basic world/player carrying BasicStatsBehavior hits an + // unmapped TPH discriminator subtype - only a real save/load round trip + // catches this, not a unit test against the behavior alone. + [Fact] + public async Task SaveTreeAsync_ThenLoadTreeAsync_RoundTripsPlayerWithBasicStatsAndCombatant() + { + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + player.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash" }); + player.Behaviors.Add(new BasicStatsBehavior { Level = 2, Experience = 150 }); + player.Behaviors.Add(new CombatantBehavior { MaxHitPoints = 20, CurrentHitPoints = 12, ArmorClass = 10 }); + + await _sut.SaveTreeAsync(player, TestContext.Current.CancellationToken); + + var loaded = await _sut.LoadTreeAsync(player.Id, TestContext.Current.CancellationToken); + + loaded.Should().NotBeNull(); + var stats = loaded!.FindBehavior(); + stats.Should().NotBeNull(); + stats!.Level.Should().Be(2); + stats.Experience.Should().Be(150); + + var combatant = loaded.FindBehavior(); + combatant.Should().NotBeNull(); + combatant!.CurrentHitPoints.Should().Be(12); + } +} diff --git a/tests/SharpMud.Ruleset.Basic.Tests/ServiceCollectionExtensionsTests.cs b/tests/SharpMud.Ruleset.Basic.Tests/ServiceCollectionExtensionsTests.cs new file mode 100644 index 0000000..d71ef6c --- /dev/null +++ b/tests/SharpMud.Ruleset.Basic.Tests/ServiceCollectionExtensionsTests.cs @@ -0,0 +1,50 @@ +using Microsoft.Extensions.DependencyInjection; +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; +using SharpMud.Hosting; +using SharpMud.Ruleset.Rpg; + +namespace SharpMud.Ruleset.Basic.Tests; + +public sealed class ServiceCollectionExtensionsTests +{ + // Without a real IPlayerFactory registration, LoginFlow/PlayerLogin + // (which constructor-inject IPlayerFactory) can't create a fresh + // CLI/Telnet player at all - the quick-start would fail at first login, + // not just "nothing to see". + [Fact] + public void AddSharpMudBasicRuleset_RegistersPlayerFactory_ThatProducesAPlayableCharacter() + { + var services = new ServiceCollection(); + services.AddSingleton(Substitute.For()); + + services.AddSharpMudBasicRuleset(); + + var provider = services.BuildServiceProvider(); + var factory = provider.GetRequiredService(); + + var world = new World(); + var startingRoom = new Thing { Id = ThingId.New(), Name = "Clearing" }; + startingRoom.Behaviors.Add(new RoomBehavior()); + + var player = factory.CreatePlayer(world, "Adventurer", "hash", startingRoom); + + player.HasBehavior().Should().BeTrue(); + player.HasBehavior().Should().BeTrue(); + player.HasBehavior().Should().BeTrue(); + } + + [Fact] + public void AddSharpMudBasicRuleset_AppliesConfigureOptionsCallback() + { + var services = new ServiceCollection(); + services.AddSingleton(Substitute.For()); + + services.AddSharpMudBasicRuleset(options => options.StartingHitPoints = 42); + + var provider = services.BuildServiceProvider(); + var options = provider.GetRequiredService(); + + options.StartingHitPoints.Should().Be(42); + } +} diff --git a/tests/SharpMud.Ruleset.Basic.Tests/SharpMud.Ruleset.Basic.Tests.csproj b/tests/SharpMud.Ruleset.Basic.Tests/SharpMud.Ruleset.Basic.Tests.csproj new file mode 100644 index 0000000..b9b9642 --- /dev/null +++ b/tests/SharpMud.Ruleset.Basic.Tests/SharpMud.Ruleset.Basic.Tests.csproj @@ -0,0 +1,48 @@ + + + + enable + enable + Exe + SharpMud.Ruleset.Basic.Tests + net11.0 + false + true + + true + true + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/SharpMud.Ruleset.Basic.Tests/TestKit/Attributes/BasicAutoDataAttribute.cs b/tests/SharpMud.Ruleset.Basic.Tests/TestKit/Attributes/BasicAutoDataAttribute.cs new file mode 100644 index 0000000..a598059 --- /dev/null +++ b/tests/SharpMud.Ruleset.Basic.Tests/TestKit/Attributes/BasicAutoDataAttribute.cs @@ -0,0 +1,12 @@ +using AutoFixture; +using AutoFixture.Xunit3; + +namespace SharpMud.Ruleset.Basic.Tests.TestKit.Attributes; + +public sealed class BasicAutoDataAttribute() : AutoDataAttribute(CreateFixture) +{ + internal static IFixture CreateFixture() => BaseFixtureFactory.CreateFixture(); +} + +public sealed class InlineBasicAutoDataAttribute(params object[] values) + : InlineAutoDataAttribute(BasicAutoDataAttribute.CreateFixture, values); diff --git a/tests/SharpMud.Ruleset.Basic.Tests/TestKit/BaseFixtureFactory.cs b/tests/SharpMud.Ruleset.Basic.Tests/TestKit/BaseFixtureFactory.cs new file mode 100644 index 0000000..278c6a0 --- /dev/null +++ b/tests/SharpMud.Ruleset.Basic.Tests/TestKit/BaseFixtureFactory.cs @@ -0,0 +1,22 @@ +using AutoFixture; +using AutoFixture.AutoNSubstitute; + +namespace SharpMud.Ruleset.Basic.Tests.TestKit; + +public static class BaseFixtureFactory +{ + public static IFixture CreateFixture(Action? customizeAction = null) + { + var fixture = new Fixture(); + + fixture.Behaviors.OfType().ToList() + .ForEach(b => fixture.Behaviors.Remove(b)); + fixture.Behaviors.Add(new OmitOnRecursionBehavior()); + + fixture.Customize(new AutoNSubstituteCustomization { ConfigureMembers = true }); + + customizeAction?.Invoke(fixture); + + return fixture; + } +} diff --git a/tests/SharpMud.Ruleset.Basic.Tests/TestKit/TestDbContextFactory.cs b/tests/SharpMud.Ruleset.Basic.Tests/TestKit/TestDbContextFactory.cs new file mode 100644 index 0000000..70cfc5e --- /dev/null +++ b/tests/SharpMud.Ruleset.Basic.Tests/TestKit/TestDbContextFactory.cs @@ -0,0 +1,31 @@ +using Microsoft.EntityFrameworkCore; +using SharpMud.Persistence; +using SharpMud.Ruleset.Rpg; + +namespace SharpMud.Ruleset.Basic.Tests.TestKit; + +// A temp-file SQLite DB, not in-memory - matches +// SharpMud.Persistence.Tests.TestKit.TestDbContextFactory's reasoning. +public sealed class TestDbContextFactory : IDbContextFactory, IDisposable +{ + private readonly string _dbPath = Path.Combine(Path.GetTempPath(), $"sharpmud-basic-test-{Guid.NewGuid()}.db"); + + public GameDbContext CreateDbContext() + { + var options = new DbContextOptionsBuilder() + .UseSqlite($"Data Source={_dbPath}") + .Options; + var context = new GameDbContext(options, [new BasicBehaviorMappingContributor(), new RpgBehaviorMappingContributor()]); + context.Database.EnsureCreated(); + return context; + } + + public Task CreateDbContextAsync(CancellationToken ct = default) => + Task.FromResult(CreateDbContext()); + + public void Dispose() + { + if (File.Exists(_dbPath)) + File.Delete(_dbPath); + } +} diff --git a/tests/SharpMud.Ruleset.Basic.Tests/xunit.runner.json b/tests/SharpMud.Ruleset.Basic.Tests/xunit.runner.json new file mode 100644 index 0000000..86c7ea0 --- /dev/null +++ b/tests/SharpMud.Ruleset.Basic.Tests/xunit.runner.json @@ -0,0 +1,3 @@ +{ + "$schema": "https://xunit.net/schema/current/xunit.runner.schema.json" +} diff --git a/tests/SharpMud.Samples.Classic.Tests/Combat/CombatManagerTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs similarity index 76% rename from tests/SharpMud.Samples.Classic.Tests/Combat/CombatManagerTests.cs rename to tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs index 6394a54..05b24c7 100644 --- a/tests/SharpMud.Samples.Classic.Tests/Combat/CombatManagerTests.cs +++ b/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs @@ -3,21 +3,20 @@ using SharpMud.Engine.Sessions; using SharpMud.Engine.Ticking; -namespace SharpMud.Samples.Classic.Tests.Combat; +namespace SharpMud.Ruleset.Rpg.Tests.Combat; public sealed class CombatManagerTests { [Fact] - public async Task OnTickAsync_AwardsXpAndEndsEncounter_WhenPlayerDefeatsNpc() + public async Task OnTickAsync_NotifiesOutcomeHandlerAndEndsEncounter_WhenPlayerDefeatsNpc() { var resolver = Substitute.For(); + var outcomeHandler = Substitute.For(); var session = Substitute.For(); - var hubRoom = new Thing { Id = ThingId.New(), Name = "Hub" }; var room = new Thing { Id = ThingId.New(), Name = "Room" }; var player = new Thing { Id = ThingId.New(), Name = "Hero" }; player.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash", Session = session }); - player.Behaviors.Add(new StatsBehavior()); room.Add(player); var npc = new Thing { Id = ThingId.New(), Name = "cave rat" }; @@ -27,28 +26,29 @@ public async Task OnTickAsync_AwardsXpAndEndsEncounter_WhenPlayerDefeatsNpc() resolver.ResolveRound(player, npc).Returns(new CombatRoundResult(true, 6, true)); - var sut = new CombatManager(resolver, hubRoom); + var sut = new CombatManager(resolver, outcomeHandler); sut.StartEncounter(player, npc); await sut.OnTickAsync(new TickContext(DateTimeOffset.UtcNow), TestContext.Current.CancellationToken); - player.FindBehavior()!.Experience.Should().Be(10); + await outcomeHandler.Received(1).OnVictoryAsync(player, npc, TestContext.Current.CancellationToken); room.Children.Should().NotContain(npc); resolver.DidNotReceive().ResolveRound(npc, player); sut.IsInCombat(player.Id).Should().BeFalse(); } [Fact] - public async Task OnTickAsync_RespawnsPlayerWithXpLoss_WhenNpcDefeatsPlayer() + public async Task OnTickAsync_ResetsCombatantHitPointsAndRespawnsAtHandlerDestination_WhenNpcDefeatsPlayer() { var resolver = Substitute.For(); + var outcomeHandler = Substitute.For(); var session = Substitute.For(); var hubRoom = new Thing { Id = ThingId.New(), Name = "Hub" }; var room = new Thing { Id = ThingId.New(), Name = "Room" }; var player = new Thing { Id = ThingId.New(), Name = "Hero" }; player.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash", Session = session }); - player.Behaviors.Add(new StatsBehavior { Experience = 100, MaxHitPoints = 20 }); + player.Behaviors.Add(new CombatantBehavior { MaxHitPoints = 20, CurrentHitPoints = -5 }); room.Add(player); var npc = new Thing { Id = ThingId.New(), Name = "cave rat" }; @@ -58,14 +58,18 @@ public async Task OnTickAsync_RespawnsPlayerWithXpLoss_WhenNpcDefeatsPlayer() resolver.ResolveRound(player, npc).Returns(new CombatRoundResult(false, 0, false)); resolver.ResolveRound(npc, player).Returns(new CombatRoundResult(true, 999, true)); + outcomeHandler.OnDefeatAsync(player, npc, TestContext.Current.CancellationToken).Returns(hubRoom); - var sut = new CombatManager(resolver, hubRoom); + var sut = new CombatManager(resolver, outcomeHandler); sut.StartEncounter(player, npc); await sut.OnTickAsync(new TickContext(DateTimeOffset.UtcNow), TestContext.Current.CancellationToken); - player.FindBehavior()!.Experience.Should().Be(90); - player.FindBehavior()!.CurrentHitPoints.Should().Be(10); + // Regression coverage for the pre-existing bug: a respawned + // character's CombatantBehavior.CurrentHitPoints must actually + // reset, not stay at/below 0 and instantly re-trigger "defeated" on + // the next hit. + player.FindBehavior()!.CurrentHitPoints.Should().Be(20); player.Parent.Should().Be(hubRoom); sut.IsInCombat(player.Id).Should().BeFalse(); } @@ -74,8 +78,8 @@ public async Task OnTickAsync_RespawnsPlayerWithXpLoss_WhenNpcDefeatsPlayer() public async Task OnTickAsync_FreezesEncounter_WhenAttackerLinkdeadWithinGraceWindow() { var resolver = Substitute.For(); + var outcomeHandler = Substitute.For(); var session = Substitute.For(); - var hubRoom = new Thing { Id = ThingId.New(), Name = "Hub" }; var room = new Thing { Id = ThingId.New(), Name = "Room" }; var player = new Thing { Id = ThingId.New(), Name = "Hero" }; @@ -89,7 +93,7 @@ public async Task OnTickAsync_FreezesEncounter_WhenAttackerLinkdeadWithinGraceWi npc.Behaviors.Add(new CombatantBehavior { ExperienceReward = 10, CurrentHitPoints = 6 }); room.Add(npc); - var sut = new CombatManager(resolver, hubRoom); + var sut = new CombatManager(resolver, outcomeHandler); sut.StartEncounter(player, npc); await sut.OnTickAsync(new TickContext(DateTimeOffset.UtcNow), TestContext.Current.CancellationToken); @@ -103,8 +107,8 @@ public async Task OnTickAsync_FreezesEncounter_WhenAttackerLinkdeadWithinGraceWi public async Task OnTickAsync_AbandonsEncounter_WhenAttackerLinkdeadPastGraceWindow() { var resolver = Substitute.For(); + var outcomeHandler = Substitute.For(); var session = Substitute.For(); - var hubRoom = new Thing { Id = ThingId.New(), Name = "Hub" }; var room = new Thing { Id = ThingId.New(), Name = "Room" }; var player = new Thing { Id = ThingId.New(), Name = "Hero" }; @@ -118,7 +122,7 @@ public async Task OnTickAsync_AbandonsEncounter_WhenAttackerLinkdeadPastGraceWin npc.Behaviors.Add(new CombatantBehavior { ExperienceReward = 10, CurrentHitPoints = 6 }); room.Add(npc); - var sut = new CombatManager(resolver, hubRoom); + var sut = new CombatManager(resolver, outcomeHandler); sut.StartEncounter(player, npc); await sut.OnTickAsync(new TickContext(DateTimeOffset.UtcNow), TestContext.Current.CancellationToken); diff --git a/tests/SharpMud.Samples.Classic.Tests/Combat/CombatResolverTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatResolverTests.cs similarity index 83% rename from tests/SharpMud.Samples.Classic.Tests/Combat/CombatResolverTests.cs rename to tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatResolverTests.cs index 0922ff8..e2e4b5a 100644 --- a/tests/SharpMud.Samples.Classic.Tests/Combat/CombatResolverTests.cs +++ b/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatResolverTests.cs @@ -1,20 +1,21 @@ using SharpMud.Engine.Core; -namespace SharpMud.Samples.Classic.Tests.Combat; +namespace SharpMud.Ruleset.Rpg.Tests.Combat; public sealed class CombatResolverTests { [Fact] public void ResolveRound_AppliesDamageAndReportsHit_WhenToHitRollMeetsArmorClass() { + var dice = Substitute.For(); var random = Substitute.For(); - random.Next(1, 20).Returns(10); + dice.Roll(1, 20).Returns(10); random.Next(2, 5).Returns(3); var attacker = MakeCombatant("Attacker", damageMin: 2, damageMax: 5); var defender = MakeCombatant("Defender", armorClass: 10, hitPoints: 10); - var sut = new CombatResolver(random); + var sut = new CombatResolver(dice, random); var result = sut.ResolveRound(attacker, defender); @@ -27,13 +28,14 @@ public void ResolveRound_AppliesDamageAndReportsHit_WhenToHitRollMeetsArmorClass [Fact] public void ResolveRound_ReportsMissAndAppliesNoDamage_WhenToHitRollIsBelowArmorClass() { + var dice = Substitute.For(); var random = Substitute.For(); - random.Next(1, 20).Returns(5); + dice.Roll(1, 20).Returns(5); var attacker = MakeCombatant("Attacker"); var defender = MakeCombatant("Defender", armorClass: 10, hitPoints: 10); - var sut = new CombatResolver(random); + var sut = new CombatResolver(dice, random); var result = sut.ResolveRound(attacker, defender); @@ -45,14 +47,15 @@ public void ResolveRound_ReportsMissAndAppliesNoDamage_WhenToHitRollIsBelowArmor [Fact] public void ResolveRound_ReportsDefenderDefeated_WhenDamageDropsHitPointsToZeroOrBelow() { + var dice = Substitute.For(); var random = Substitute.For(); - random.Next(1, 20).Returns(20); + dice.Roll(1, 20).Returns(20); random.Next(1, 4).Returns(4); var attacker = MakeCombatant("Attacker", damageMin: 1, damageMax: 4); var defender = MakeCombatant("Defender", armorClass: 10, hitPoints: 3); - var sut = new CombatResolver(random); + var sut = new CombatResolver(dice, random); var result = sut.ResolveRound(attacker, defender); diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/Combat/DiceRollerTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/Combat/DiceRollerTests.cs new file mode 100644 index 0000000..6b0ce4b --- /dev/null +++ b/tests/SharpMud.Ruleset.Rpg.Tests/Combat/DiceRollerTests.cs @@ -0,0 +1,58 @@ +using SharpMud.Engine.Core; + +namespace SharpMud.Ruleset.Rpg.Tests.Combat; + +public sealed class DiceRollerTests +{ + [Fact] + public void Roll_SumsEachDieResultPlusModifier() + { + var random = Substitute.For(); + random.Next(1, 6).Returns(3, 5, 2); + + var sut = new DiceRoller(random); + + var result = sut.Roll(3, 6, modifier: 4); + + result.Should().Be(3 + 5 + 2 + 4); + } + + [Fact] + public void Roll_DefaultsModifierToZero() + { + var random = Substitute.For(); + random.Next(1, 20).Returns(15); + + var sut = new DiceRoller(random); + + var result = sut.Roll(1, 20); + + result.Should().Be(15); + } + + [Theory] + [InlineData(0, 6)] + [InlineData(-1, 6)] + public void Roll_Throws_WhenDiceCountIsLessThanOne(int diceCount, int sides) + { + var random = Substitute.For(); + var sut = new DiceRoller(random); + + var act = () => sut.Roll(diceCount, sides); + + act.Should().Throw(); + } + + [Theory] + [InlineData(1, 0)] + [InlineData(1, -1)] + public void Roll_Throws_WhenSidesIsLessThanOne(int diceCount, int sides) + { + var random = Substitute.For(); + var sut = new DiceRoller(random); + + var act = () => sut.Roll(diceCount, sides); + + act.Should().Throw(); + } +} diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs new file mode 100644 index 0000000..e3e3ed6 --- /dev/null +++ b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs @@ -0,0 +1,72 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Ruleset.Rpg.Tests.Commands; + +public sealed class AttackCommandTests +{ + [Fact] + public async Task ExecuteAsync_StartsEncounter_WhenTargetExists() + { + var combatManager = Substitute.For(); + var session = Substitute.For(); + + var room = new Thing { Id = ThingId.New(), Name = "Room" }; + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + room.Add(player); + + var npc = new Thing { Id = ThingId.New(), Name = "cave rat" }; + npc.Behaviors.Add(new NpcBehavior()); + npc.Behaviors.Add(new CombatantBehavior()); + room.Add(npc); + + var sut = new AttackCommand(combatManager); + var ctx = new CommandContext(player, room, ["cave", "rat"], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + combatManager.Received(1).StartEncounter(player, npc); + await session.Received(1).WriteLineAsync("You attack cave rat!", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_SendsNotHereMessage_WhenNoMatchingCombatantInRoom() + { + var combatManager = Substitute.For(); + var session = Substitute.For(); + + var room = new Thing { Id = ThingId.New(), Name = "Room" }; + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + room.Add(player); + + var sut = new AttackCommand(combatManager); + var ctx = new CommandContext(player, room, ["dragon"], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + combatManager.DidNotReceiveWithAnyArgs().StartEncounter(default!, default!); + await session.Received(1).WriteLineAsync("You don't see that here.", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_SendsAlreadyFightingMessage_WhenActorAlreadyInCombat() + { + var combatManager = Substitute.For(); + var session = Substitute.For(); + combatManager.IsInCombat(Arg.Any()).Returns(true); + + var room = new Thing { Id = ThingId.New(), Name = "Room" }; + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + room.Add(player); + + var sut = new AttackCommand(combatManager); + var ctx = new CommandContext(player, room, ["cave", "rat"], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + combatManager.DidNotReceiveWithAnyArgs().StartEncounter(default!, default!); + await session.Received(1).WriteLineAsync("You are already fighting!", Arg.Any()); + } +} diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs new file mode 100644 index 0000000..eb6ddc5 --- /dev/null +++ b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs @@ -0,0 +1,96 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Ruleset.Rpg.Tests.Commands; + +public sealed class FleeCommandTests +{ + [Fact] + public async Task ExecuteAsync_MovesActorAndEndsEncounter_WhenRollSucceeds() + { + var combatManager = Substitute.For(); + var dice = Substitute.For(); + var random = Substitute.For(); + var session = Substitute.For(); + var encounter = new CombatEncounter { Attacker = new Thing { Id = ThingId.New(), Name = "x" }, Defender = new Thing { Id = ThingId.New(), Name = "y" } }; + combatManager.TryGetEncounter(Arg.Any(), out Arg.Any()) + .Returns(x => { x[1] = encounter; return true; }); + dice.Roll(1, 100).Returns(1); + random.Next(0, 0).Returns(0); + + var origin = new Thing { Id = ThingId.New(), Name = "Origin" }; + var destination = new Thing { Id = ThingId.New(), Name = "Destination", Description = "A quiet place." }; + var exit = new Thing { Id = ThingId.New(), Name = "north" }; + exit.Behaviors.Add(new ExitBehavior { Direction = Direction.North, Destination = destination }); + origin.Add(exit); + + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + player.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash", Session = session }); + origin.Add(player); + + var sut = new FleeCommand(combatManager, dice, random); + var ctx = new CommandContext(player, origin, [], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + combatManager.Received(1).EndEncounter(player.Id); + player.Parent.Should().Be(destination); + origin.Children.Should().NotContain(player); + } + + [Fact] + public async Task ExecuteAsync_SendsFailureMessage_WhenRollFails() + { + var combatManager = Substitute.For(); + var dice = Substitute.For(); + var random = Substitute.For(); + var session = Substitute.For(); + var encounter = new CombatEncounter { Attacker = new Thing { Id = ThingId.New(), Name = "x" }, Defender = new Thing { Id = ThingId.New(), Name = "y" } }; + combatManager.TryGetEncounter(Arg.Any(), out Arg.Any()) + .Returns(x => { x[1] = encounter; return true; }); + dice.Roll(1, 100).Returns(100); + + var origin = new Thing { Id = ThingId.New(), Name = "Origin" }; + var destination = new Thing { Id = ThingId.New(), Name = "Destination" }; + var exit = new Thing { Id = ThingId.New(), Name = "north" }; + exit.Behaviors.Add(new ExitBehavior { Direction = Direction.North, Destination = destination }); + origin.Add(exit); + + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + player.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash", Session = session }); + origin.Add(player); + + var sut = new FleeCommand(combatManager, dice, random); + var ctx = new CommandContext(player, origin, [], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + combatManager.DidNotReceiveWithAnyArgs().EndEncounter(default!); + player.Parent.Should().Be(origin); + await session.Received(1).WriteLineAsync("You fail to escape!", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_SendsNotFightingMessage_WhenNoActiveEncounter() + { + var combatManager = Substitute.For(); + var dice = Substitute.For(); + var random = Substitute.For(); + var session = Substitute.For(); + combatManager.TryGetEncounter(Arg.Any(), out Arg.Any()).Returns(false); + + var room = new Thing { Id = ThingId.New(), Name = "Room" }; + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + player.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash", Session = session }); + room.Add(player); + + var sut = new FleeCommand(combatManager, dice, random); + var ctx = new CommandContext(player, room, [], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync("You aren't fighting anything.", Arg.Any()); + } +} diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/ServiceCollectionExtensionsTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/ServiceCollectionExtensionsTests.cs new file mode 100644 index 0000000..3f8cc35 --- /dev/null +++ b/tests/SharpMud.Ruleset.Rpg.Tests/ServiceCollectionExtensionsTests.cs @@ -0,0 +1,64 @@ +using Microsoft.Extensions.DependencyInjection; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Core; +using SharpMud.Engine.Ticking; + +namespace SharpMud.Ruleset.Rpg.Tests; + +public sealed class ServiceCollectionExtensionsTests +{ + // Proves builtin commands, this package's kill/attack/flee, and a + // consumer's own registered command all end up in the same resolved + // ICommandRegistry - the seam most likely to silently regress (one + // registration source clobbering another) per + // docs/adr/0008-ruleset-scaffolding-tier.md. + [Fact] + public void AddSharpMudRpgRuleset_ComposesBuiltinRpgAndConsumerCommands_IntoOneRegistry() + { + var services = new ServiceCollection(); + services.AddSingleton(Substitute.For()); + + services.AddSharpMudRpgRuleset((_, registry) => + registry.Register(new FakeConsumerCommand())); + + var provider = services.BuildServiceProvider(); + var registry = provider.GetRequiredService(); + + registry.TryResolve("look", out _).Should().BeTrue("built-in commands must still be registered"); + registry.TryResolve("kill", out var killCommand).Should().BeTrue(); + registry.TryResolve("attack", out var attackAliasCommand).Should().BeTrue(); + killCommand.Should().BeSameAs(attackAliasCommand, "attack is kill's alias, not a separate command"); + registry.TryResolve("flee", out _).Should().BeTrue(); + registry.TryResolve("dance", out _).Should().BeTrue("a consumer's own command must not be clobbered"); + } + + [Fact] + public void AddSharpMudRpgRuleset_RegistersCombatManagerAsBothItselfAndTickable_OffTheSameInstance() + { + var services = new ServiceCollection(); + services.AddSingleton(Substitute.For()); + + services.AddSharpMudRpgRuleset(); + + var provider = services.BuildServiceProvider(); + var combatManager = provider.GetRequiredService(); + var tickable = provider.GetRequiredService(); + + tickable.Should().BeSameAs(combatManager); + } + + private sealed class FakeCombatOutcomeHandler : ICombatOutcomeHandler + { + public Task OnVictoryAsync(Thing victor, Thing defeated, CancellationToken ct) => Task.CompletedTask; + + public Task OnDefeatAsync(Thing defeated, Thing victor, CancellationToken ct) => Task.FromResult(defeated); + } + + private sealed class FakeConsumerCommand : ICommand + { + public string Verb => "dance"; + public IReadOnlyList Aliases { get; } = []; + + public Task ExecuteAsync(CommandContext ctx, CancellationToken ct) => Task.CompletedTask; + } +} diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/SharpMud.Ruleset.Rpg.Tests.csproj b/tests/SharpMud.Ruleset.Rpg.Tests/SharpMud.Ruleset.Rpg.Tests.csproj new file mode 100644 index 0000000..ad35a61 --- /dev/null +++ b/tests/SharpMud.Ruleset.Rpg.Tests/SharpMud.Ruleset.Rpg.Tests.csproj @@ -0,0 +1,46 @@ + + + + enable + enable + Exe + SharpMud.Ruleset.Rpg.Tests + net11.0 + false + true + + true + true + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/TestKit/Attributes/RpgAutoDataAttribute.cs b/tests/SharpMud.Ruleset.Rpg.Tests/TestKit/Attributes/RpgAutoDataAttribute.cs new file mode 100644 index 0000000..747aa80 --- /dev/null +++ b/tests/SharpMud.Ruleset.Rpg.Tests/TestKit/Attributes/RpgAutoDataAttribute.cs @@ -0,0 +1,12 @@ +using AutoFixture; +using AutoFixture.Xunit3; + +namespace SharpMud.Ruleset.Rpg.Tests.TestKit.Attributes; + +public sealed class RpgAutoDataAttribute() : AutoDataAttribute(CreateFixture) +{ + internal static IFixture CreateFixture() => BaseFixtureFactory.CreateFixture(); +} + +public sealed class InlineRpgAutoDataAttribute(params object[] values) + : InlineAutoDataAttribute(RpgAutoDataAttribute.CreateFixture, values); diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/TestKit/BaseFixtureFactory.cs b/tests/SharpMud.Ruleset.Rpg.Tests/TestKit/BaseFixtureFactory.cs new file mode 100644 index 0000000..d7bd71f --- /dev/null +++ b/tests/SharpMud.Ruleset.Rpg.Tests/TestKit/BaseFixtureFactory.cs @@ -0,0 +1,22 @@ +using AutoFixture; +using AutoFixture.AutoNSubstitute; + +namespace SharpMud.Ruleset.Rpg.Tests.TestKit; + +public static class BaseFixtureFactory +{ + public static IFixture CreateFixture(Action? customizeAction = null) + { + var fixture = new Fixture(); + + fixture.Behaviors.OfType().ToList() + .ForEach(b => fixture.Behaviors.Remove(b)); + fixture.Behaviors.Add(new OmitOnRecursionBehavior()); + + fixture.Customize(new AutoNSubstituteCustomization { ConfigureMembers = true }); + + customizeAction?.Invoke(fixture); + + return fixture; + } +} diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/xunit.runner.json b/tests/SharpMud.Ruleset.Rpg.Tests/xunit.runner.json new file mode 100644 index 0000000..86c7ea0 --- /dev/null +++ b/tests/SharpMud.Ruleset.Rpg.Tests/xunit.runner.json @@ -0,0 +1,3 @@ +{ + "$schema": "https://xunit.net/schema/current/xunit.runner.schema.json" +} diff --git a/tests/SharpMud.Samples.Classic.Tests/ClassicCombatOutcomeHandlerTests.cs b/tests/SharpMud.Samples.Classic.Tests/ClassicCombatOutcomeHandlerTests.cs new file mode 100644 index 0000000..37f4262 --- /dev/null +++ b/tests/SharpMud.Samples.Classic.Tests/ClassicCombatOutcomeHandlerTests.cs @@ -0,0 +1,53 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; +using SharpMud.Hosting; +using SharpMud.Ruleset.Rpg; + +namespace SharpMud.Samples.Classic.Tests; + +public sealed class ClassicCombatOutcomeHandlerTests +{ + [Fact] + public async Task OnVictoryAsync_AwardsExperienceFromCombatantReward() + { + var session = Substitute.For(); + var victor = new Thing { Id = ThingId.New(), Name = "Hero" }; + victor.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash", Session = session }); + victor.Behaviors.Add(new StatsBehavior { Experience = 0 }); + + var defeated = new Thing { Id = ThingId.New(), Name = "cave rat" }; + defeated.Behaviors.Add(new CombatantBehavior { ExperienceReward = 10 }); + + var worldContext = new WorldContext(); + var sut = new ClassicCombatOutcomeHandler(worldContext); + + await sut.OnVictoryAsync(victor, defeated, TestContext.Current.CancellationToken); + + victor.FindBehavior()!.Experience.Should().Be(10); + } + + [Fact] + public async Task OnDefeatAsync_AppliesXpLossAndHitPointHalving_AndReturnsHubRoom() + { + var session = Substitute.For(); + var defeated = new Thing { Id = ThingId.New(), Name = "Hero" }; + defeated.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash", Session = session }); + defeated.Behaviors.Add(new StatsBehavior { Experience = 100, MaxHitPoints = 20 }); + + var victor = new Thing { Id = ThingId.New(), Name = "cave rat" }; + + var hubRoom = new Thing { Id = ThingId.New(), Name = "Hub" }; + hubRoom.Behaviors.Add(new RoomBehavior()); + var worldContext = new WorldContext(); + worldContext.Initialize(new World(), hubRoom, hubRoom); + + var sut = new ClassicCombatOutcomeHandler(worldContext); + + var destination = await sut.OnDefeatAsync(defeated, victor, TestContext.Current.CancellationToken); + + destination.Should().Be(hubRoom); + defeated.FindBehavior()!.Experience.Should().Be(90); + defeated.FindBehavior()!.CurrentHitPoints.Should().Be(10); + } +} From 231ae287b21b1c723d98bc76955682bde2749ba9 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 10:37:39 -0400 Subject: [PATCH 2/6] fix: address PR #18 review feedback on ruleset-scaffolding-tier Fixes two real death-penalty bugs found by review: CombatManager reset CombatantBehavior.CurrentHitPoints to full before the outcome handler ran, but neither ClassicCombatOutcomeHandler nor BasicCombatOutcomeHandler actually halved it back down, so the documented "respawn at half HP" penalty was silently a no-op for both rulesets. Also fixes a player-facing message-order regression (defeat message now precedes the outcome handler's own messages again, matching pre-extraction behavior), adds a guard so AttackCommand fails cleanly instead of crashing at tick time when the actor has no CombatantBehavior, adds fail-fast validation to BasicRulesetOptions, and adds XML doc comments across the new public surface in both packages. Also: docs/getting-started.md's package install commands now include --prerelease (alpha packages, none published as stable yet), and several docs/*.md sections left stale by the extraction (combat.md's code sketch, engine-vs-ruleset.md's ruleset-behaviors listing, a small ADR-0008 typo) are brought current. ADR-0008's Open Items are updated to record the decisions actually made during implementation (meta-package exclusion, the ICombatOutcomeHandler mechanism, and the forwarding-callback command composition), rather than left as still-open questions. Co-Authored-By: Claude Sonnet 5 --- docs/adr/0008-ruleset-scaffolding-tier.md | 46 ++++++------ docs/combat.md | 75 ++++++++++--------- docs/engine-vs-ruleset.md | 39 ++++++---- docs/getting-started.md | 13 ++-- .../ClassicCombatOutcomeHandler.cs | 28 +++++-- .../BasicBehaviorMappingContributor.cs | 7 ++ .../BasicCombatOutcomeHandler.cs | 13 ++++ .../BasicPlayerFactory.cs | 8 ++ .../BasicRulesetOptions.cs | 21 ++++++ .../BasicStatsBehavior.cs | 9 +++ .../BasicWorldBuilder.cs | 4 + .../ServiceCollectionExtensions.cs | 4 +- src/SharpMud.Ruleset.Rpg/AttackCommand.cs | 27 +++++++ src/SharpMud.Ruleset.Rpg/CombatManager.cs | 33 +++++++- src/SharpMud.Ruleset.Rpg/CombatResolver.cs | 10 +++ src/SharpMud.Ruleset.Rpg/CombatantBehavior.cs | 12 +++ src/SharpMud.Ruleset.Rpg/DiceRoller.cs | 9 +++ src/SharpMud.Ruleset.Rpg/FleeCommand.cs | 14 ++++ src/SharpMud.Ruleset.Rpg/ICombatManager.cs | 17 +++++ .../ICombatOutcomeHandler.cs | 12 ++- src/SharpMud.Ruleset.Rpg/ICombatResolver.cs | 10 +++ .../RpgBehaviorMappingContributor.cs | 7 ++ .../ServiceCollectionExtensions.cs | 1 + .../BasicRulesetOptionsTests.cs | 48 ++++++++++++ .../ServiceCollectionExtensionsTests.cs | 14 ++++ .../Combat/CombatManagerTests.cs | 39 ++++++++++ .../Commands/AttackCommandTests.cs | 26 +++++++ 27 files changed, 456 insertions(+), 90 deletions(-) create mode 100644 tests/SharpMud.Ruleset.Basic.Tests/BasicRulesetOptionsTests.cs diff --git a/docs/adr/0008-ruleset-scaffolding-tier.md b/docs/adr/0008-ruleset-scaffolding-tier.md index 49fb850..ed55ea3 100644 --- a/docs/adr/0008-ruleset-scaffolding-tier.md +++ b/docs/adr/0008-ruleset-scaffolding-tier.md @@ -140,7 +140,7 @@ SharpMud.Engine unchanged — Thing/Behavior/events, zero ruleset kno SharpMud.Ruleset.Rpg NEW, packaged — CombatantBehavior, ICombatResolver/CombatResolver, ICombatManager/CombatManager, AttackCommand/FleeCommand, moved - in from samples/SharpMud.Samples.Classic (see Context — these + in from samples/SharpMud.Samples.Classic (see Context — 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): @@ -355,26 +355,30 @@ See Decision Outcome above. ## 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. +- ~~Whether `SharpMud.Ruleset.Rpg`/`SharpMud.Ruleset.Basic` belong in the + `SharpMud` meta-package~~ — **resolved during implementation: no.** Same + reasoning ADR-0007 already established for provider/transport packages — + a consumer who wants a *different* ruleset shape shouldn't get + RPG-specific scaffolding pulled in by default just for referencing the + meta-package. `SharpMud` stays Engine + Hosting + Persistence only; + `Ruleset.Rpg`/`Ruleset.Basic` are always an explicit, separate reference. + Revisit only if a real consumer need for a bundled "engine + Rpg" + meta-package surfaces — none has. +- ~~The exact mechanism for decoupling `CombatManager`'s XP-award/death-penalty + logic from `StatsBehavior`~~ — **resolved during implementation:** a + callback interface, `ICombatOutcomeHandler` (`OnVictoryAsync`/ + `OnDefeatAsync`), not a `ThingEvents` game event. See PLAN-0008. +- ~~The exact mechanism for decoupling `CombatManager`'s hard-coded `hubRoom` + respawn destination~~ — **resolved during implementation:** the same + `ICombatOutcomeHandler.OnDefeatAsync` above also returns the respawn + `Thing`, confirming the ADR's own "plausibly the same mechanism" guess. +- ~~The exact mechanism for composing `AttackCommand`/`FleeCommand` + registration with a consumer's own commands~~ — **resolved during + implementation:** the forwarding-callback shape. + `AddSharpMudRpgRuleset(registerConsumerCommands)` + calls `Hosting`'s `AddSharpMudRuleset(...)` exactly once internally; + `ICommandRegistry` registration did not need to become additive, and + `SharpMud.Hosting` was not changed. - `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 diff --git a/docs/combat.md b/docs/combat.md index a8b8ca7..c5af4ae 100644 --- a/docs/combat.md +++ b/docs/combat.md @@ -31,29 +31,28 @@ world content. Simple round-based combat (Diku/Circle-style), per SPEC.md: auto-attack on the global tick, hit/miss/damage messages, minimal per-round input required once engaged. **v1 scope is player-vs-NPC only** — no PvP verb or aggression -rules exist yet, so every encounter is keyed by the attacking player. +rules exist yet, so every encounter is keyed by the attacking `Thing`. -Implemented shape (`src/SharpMud.Engine/Combat/`) differs from the original +Implemented shape (`src/SharpMud.Ruleset.Rpg/`) differs from the original sketch in two ways: `ITickable.OnTick` is `Task OnTickAsync(...)`, not `void` -(it needs to `await` `ISession` writes each round — same reasoning as -`IWorld.MovePlayer` becoming `MovePlayerAsync`), and there's one +(it needs to `await` `ISession` writes each round), and there's one `CombatManager` registered with `IGameLoop`, not one `ITickable` per -encounter — it owns a `Dictionary` and resolves +encounter — it owns a `Dictionary` and resolves every active encounter each tick: ```csharp public sealed class CombatEncounter { - public required Player Attacker { get; init; } - public required Npc Defender { get; init; } + public required Thing Attacker { get; init; } + public required Thing Defender { get; init; } } public interface ICombatManager { - bool IsInCombat(PlayerId playerId); - void StartEncounter(Player attacker, Npc defender); - void EndEncounter(PlayerId playerId); - bool TryGetEncounter(PlayerId playerId, out CombatEncounter? encounter); + bool IsInCombat(ThingId thingId); + void StartEncounter(Thing attacker, Thing defender); + void EndEncounter(ThingId thingId); + bool TryGetEncounter(ThingId thingId, [MaybeNullWhen(false)] out CombatEncounter encounter); } public sealed class CombatManager(ICombatResolver resolver, ICombatOutcomeHandler outcomeHandler) @@ -63,8 +62,8 @@ public sealed class CombatManager(ICombatResolver resolver, ICombatOutcomeHandle } ``` -No `hubRoomId`/`IWorld` constructor parameter any more - `CombatManager` has -no respawn-destination or world-lookup concept of its own. `ICombatOutcomeHandler` +No `hubRoomId`/`IWorld` constructor parameter — `CombatManager` has no +respawn-destination or world-lookup concept of its own. `ICombatOutcomeHandler` (implemented per-ruleset) owns both the XP-award/death-penalty side effects and the respawn destination: @@ -76,53 +75,55 @@ public interface ICombatOutcomeHandler } ``` -`ICombatant` also grew two members beyond the original sketch -(`CurrentHitPoints`/`ArmorClass`/`DamageRange` only) — `Name` and -`MaxHitPoints`, both needed for round messages and death/respawn handling -that the doc implied but didn't spell out as interface members: +Any `Thing` that can fight carries `CombatantBehavior` - a plain-data +`Behavior`, not an interface a domain type implements (see +[engine-vs-ruleset.md](engine-vs-ruleset.md) for why composition replaced +the original class-hierarchy sketch entirely): ```csharp -public interface ICombatant +public sealed class CombatantBehavior : Behavior { - string Name { get; } - int CurrentHitPoints { get; set; } - int MaxHitPoints { get; } - int ArmorClass { get; } - (int Min, int Max) DamageRange { get; } + public int MaxHitPoints { get; set; } + public int CurrentHitPoints { get; set; } + public int ArmorClass { get; set; } + public int DamageMin { get; set; } + public int DamageMax { get; set; } + public int ExperienceReward { get; set; } + public (int Min, int Max) DamageRange => (DamageMin, DamageMax); } ``` -`Player` and `Npc` both implement `ICombatant`. - Hit/damage formula: Diku/Circle-style d20-vs-AC roll — attacker rolls d20 vs. defender Armor Class to hit; damage is a random roll within the attacker's `DamageRange`. **Currently implemented as an unmodified d20 roll** — no level/skill to-hit bonus yet (see Open Items; the modifier-scaling formula is still undecided, so the code has nothing to apply). -Formulas live in a dedicated `ICombatResolver` (pure, unit-testable, no I/O): +Formulas live in a dedicated `ICombatResolver` (pure, unit-testable, no I/O) +implemented by `CombatResolver`, which both computes **and applies** the +round (mutates the defender's `CombatantBehavior.CurrentHitPoints` directly): ```csharp public interface ICombatResolver { - CombatRoundResult ResolveRound(ICombatant attacker, ICombatant defender); + CombatRoundResult ResolveRound(Thing attacker, Thing defender); } public sealed record CombatRoundResult(bool Hit, int Damage, bool DefenderDefeated); ``` -`ResolveRound` both computes **and applies** the round (mutates -`defender.CurrentHitPoints` directly) — matching the original "resolve one -round: hit check → damage → apply → check death" description. - ## Sequence: Combat Round Resolves -1. Player types `"kill cave rat"` → `AttackCommand` resolves the target NPC - in the room (`ctx.CurrentRoom.Npcs` → `IWorld.GetNpc`, matched by - case-insensitive substring), calls `ICombatManager.StartEncounter`, sends - `"You attack cave rat!"` immediately (engagement is instant; resolution is - tick-gated). If the player is already in combat, the command instead sends - `"You are already fighting!"`. +1. Player types `"kill cave rat"` → `AttackCommand` matches the target among + the current room's children carrying both `NpcBehavior` and + `CombatantBehavior` (`ObjectMatcher.FindMatch`, case-insensitive), calls + `ICombatManager.StartEncounter`, sends `"You attack cave rat!"` + immediately (engagement is instant; resolution is tick-gated). If the + player is already in combat, the command instead sends `"You are already + fighting!"`; if the actor itself has no `CombatantBehavior` (a consumer's + own `IPlayerFactory` forgot to attach it), it sends `"You have no way to + fight."` instead of starting an encounter that would crash on the next + tick. 2. On the next global tick, `IGameLoop` calls `CombatManager.OnTickAsync`, which iterates every active encounter. 3. `ICombatResolver.ResolveRound(attacker, defender)` computes the player's diff --git a/docs/engine-vs-ruleset.md b/docs/engine-vs-ruleset.md index 412c17d..6fbf0ae 100644 --- a/docs/engine-vs-ruleset.md +++ b/docs/engine-vs-ruleset.md @@ -218,19 +218,29 @@ exit canceling a move, a full container canceling an item pickup). that the split holds for a whole feature, not just data classes: `SharpMud.Samples.Classic.Tests` never had to change for this to work. -## Ruleset-level behaviors (`SharpMud.Samples.Classic`) - -- `StatsBehavior` — the D&D-style attributes (`Strength`...`Charisma`), - `Race`, `CharacterClass`, `Level`, `Experience`, `MaxHitPoints`/ - `CurrentHitPoints`/etc. Everything [character.md](character.md) describes. -- `CombatantBehavior` — `ArmorClass`, `DamageMin`/`DamageMax`. What - [combat.md](combat.md)'s `ICombatant` used to be an interface `Player`/`Npc` - implemented is now a behavior any `Thing` can carry (a hostile plant, a - turret — anything the ruleset wants to fight). +## Ruleset-level behaviors + +Split across two tiers as of [ADR-0008](adr/0008-ruleset-scaffolding-tier.md) +(see that ADR/[rulesets tier listing above](#project-structure-revised) for +the full picture) — not all of this lives in `SharpMud.Samples.Classic` +anymore: + +- `StatsBehavior` (`SharpMud.Samples.Classic`, Classic-specific) — the + D&D-style attributes (`Strength`...`Charisma`), `Race`, `CharacterClass`, + `Level`, `Experience`, `MaxHitPoints`/`CurrentHitPoints`/etc. Everything + [character.md](character.md) describes. `SharpMud.Ruleset.Basic` has its + own, much simpler `BasicStatsBehavior` instead (just `Level`/`Experience`). +- `CombatantBehavior` (`SharpMud.Ruleset.Rpg`, shared scaffolding) — + `ArmorClass`, `DamageMin`/`DamageMax`. What [combat.md](combat.md)'s + `ICombatant` used to be an interface `Player`/`Npc` implemented is now a + behavior any `Thing` can carry (a hostile plant, a turret — anything a + ruleset wants to fight). - Combat resolution (`ICombatResolver`/`CombatResolver`), `ICombatManager`/ - `CombatManager`, and the `kill`/`attack`/`flee` commands all move here - unchanged in logic — only their dependency on `Player`/`Npc` becomes a - dependency on `Thing` + `FindBehavior()`. + `CombatManager`, and the `kill`/`attack`/`flee` commands (`SharpMud.Ruleset.Rpg`, + shared scaffolding) — their dependency on `Player`/`Npc` is a dependency on + `Thing` + `FindBehavior()`, same as `CombatantBehavior` + above. Classic and Basic both reference this package rather than owning + this logic themselves. ## Command pipeline changes @@ -238,8 +248,9 @@ exit canceling a move, a full container canceling an item pickup). `(Thing Actor, Thing CurrentRoom, ...)`. Commands that need ruleset data (`AttackCommand` needing `CombatantBehavior`) do `ctx.Actor.FindBehavior<...>()` and fail gracefully if absent — this is the actual mechanism that keeps -`AttackCommand` in the sample ruleset rather than `Engine`: it's the first -command to depend on a ruleset-specific behavior type. +`AttackCommand` in `SharpMud.Ruleset.Rpg` rather than `Engine`: it's the +first command to depend on a ruleset-shaped behavior type, not a +ruleset-agnostic one. Adopted from WheelMUD (see findings doc §3): a lightweight `CommandGuards` static helper covers repeated preconditions (`RequiresAtLeastOneArgument`, diff --git a/docs/getting-started.md b/docs/getting-started.md index 1d0cd92..61a19e8 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -13,13 +13,16 @@ below. Create a new console project and add: ``` -dotnet add package SharpMud.Engine -dotnet add package SharpMud.Hosting -dotnet add package SharpMud.Persistence.Sqlite -dotnet add package SharpMud.Ruleset.Basic -dotnet add package SharpMud.Adapters.Cli +dotnet add package SharpMud.Engine --prerelease +dotnet add package SharpMud.Hosting --prerelease +dotnet add package SharpMud.Persistence.Sqlite --prerelease +dotnet add package SharpMud.Ruleset.Basic --prerelease +dotnet add package SharpMud.Adapters.Cli --prerelease ``` +`--prerelease` is required until a stable 1.0 release ships — sharp-mud is +pre-1.0 and only prerelease packages are published so far. + (swap `SharpMud.Persistence.Sqlite` for `SharpMud.Persistence.DynamoDb` and/or `SharpMud.Adapters.Cli` for `SharpMud.Adapters.Telnet` as needed — see [persistence.md](persistence.md)/[networking.md](networking.md)). diff --git a/samples/SharpMud.Samples.Classic/ClassicCombatOutcomeHandler.cs b/samples/SharpMud.Samples.Classic/ClassicCombatOutcomeHandler.cs index 0e7d329..5a13614 100644 --- a/samples/SharpMud.Samples.Classic/ClassicCombatOutcomeHandler.cs +++ b/samples/SharpMud.Samples.Classic/ClassicCombatOutcomeHandler.cs @@ -9,20 +9,25 @@ namespace SharpMud.Samples.Classic; /// Classic's - awards /// on a win, applies the XP-loss/ /// HP-halving death penalty on a loss, and respawns the loser in the hub -/// (). is already reset by before runs - this -/// only owns the ruleset-specific touches. +/// (). +/// resets to full as a +/// safe baseline before calling - the actual +/// combat HP used by , not - so this handler halves it +/// again here to make the documented death penalty real, not just cosmetic +/// against a field combat no longer reads. /// public sealed class ClassicCombatOutcomeHandler : ICombatOutcomeHandler { private readonly WorldContext _worldContext; + /// Creates the handler against the shared (for the respawn destination). public ClassicCombatOutcomeHandler(WorldContext worldContext) { _worldContext = worldContext; } + /// public async Task OnVictoryAsync(Thing victor, Thing defeated, CancellationToken ct) { var stats = victor.FindBehavior(); @@ -37,8 +42,18 @@ public async Task OnVictoryAsync(Thing victor, Thing defeated, CancellationToken await session.WriteLineAsync($"You gain {combatant.ExperienceReward} experience.", ct); } + /// public async Task OnDefeatAsync(Thing defeated, Thing victor, CancellationToken ct) { + // Respawn HP fraction is an open item (docs/combat.md); 50% is a + // placeholder. This is the HP CombatResolver actually reads/writes - + // CombatManager already reset it to full before calling this method, + // so halving it here (not just StatsBehavior's copy below) is what + // makes the penalty real rather than cosmetic. + var combatant = defeated.FindBehavior(); + if (combatant is not null) + combatant.CurrentHitPoints = Math.Max(1, combatant.MaxHitPoints / 2); + var stats = defeated.FindBehavior(); if (stats is not null) { @@ -47,7 +62,10 @@ public async Task OnDefeatAsync(Thing defeated, Thing victor, Cancellatio long xpLoss = (long)(stats.Experience * 0.10); stats.Experience = Math.Max(0, stats.Experience - xpLoss); - // Respawn HP fraction is also an open item; 50% is a placeholder. + // StatsBehavior's own HP mirrors the character-sheet display + // value; kept in sync with CombatantBehavior's halving above so + // the two don't drift, even though CombatResolver never reads + // this copy directly. stats.CurrentHitPoints = Math.Max(1, stats.MaxHitPoints / 2); var session = defeated.FindBehavior()?.Session; diff --git a/src/SharpMud.Ruleset.Basic/BasicBehaviorMappingContributor.cs b/src/SharpMud.Ruleset.Basic/BasicBehaviorMappingContributor.cs index 3999d2b..2b392c5 100644 --- a/src/SharpMud.Ruleset.Basic/BasicBehaviorMappingContributor.cs +++ b/src/SharpMud.Ruleset.Basic/BasicBehaviorMappingContributor.cs @@ -9,8 +9,15 @@ namespace SharpMud.Ruleset.Basic; // Without this, a Basic world/player carrying BasicStatsBehavior hits the // same unmapped TPH discriminator subtype problem CombatantBehavior would // have without RpgBehaviorMappingContributor. +/// +/// This package's - registers 's EF Core mapping. Registered automatically +/// by AddSharpMudBasicRuleset(...); a consumer doesn't need to +/// register it themselves. +/// public sealed class BasicBehaviorMappingContributor : IBehaviorMappingContributor { + /// Applies every IEntityTypeConfiguration<T> in this assembly - currently just 's. public void ConfigureBehaviors(ModelBuilder modelBuilder) => modelBuilder.ApplyConfigurationsFromAssembly(typeof(BasicBehaviorMappingContributor).Assembly); } diff --git a/src/SharpMud.Ruleset.Basic/BasicCombatOutcomeHandler.cs b/src/SharpMud.Ruleset.Basic/BasicCombatOutcomeHandler.cs index 2d21d32..2c97ca7 100644 --- a/src/SharpMud.Ruleset.Basic/BasicCombatOutcomeHandler.cs +++ b/src/SharpMud.Ruleset.Basic/BasicCombatOutcomeHandler.cs @@ -17,11 +17,13 @@ public sealed class BasicCombatOutcomeHandler : ICombatOutcomeHandler { private readonly WorldContext _worldContext; + /// Creates the handler against the shared (for the respawn destination). public BasicCombatOutcomeHandler(WorldContext worldContext) { _worldContext = worldContext; } + /// public async Task OnVictoryAsync(Thing victor, Thing defeated, CancellationToken ct) { var stats = victor.FindBehavior(); @@ -36,8 +38,19 @@ public async Task OnVictoryAsync(Thing victor, Thing defeated, CancellationToken await session.WriteLineAsync($"You gain {combatant.ExperienceReward} experience.", ct); } + /// public async Task OnDefeatAsync(Thing defeated, Thing victor, CancellationToken ct) { + // Respawn HP fraction is an open item (docs/combat.md); 50% is a + // placeholder, matching Classic's. BasicStatsBehavior carries no HP + // of its own (just Level/Experience) - CombatantBehavior.CurrentHitPoints + // is the only combat HP a Basic character has, and CombatManager + // already reset it to full before calling this method, so halving + // it here is what makes the penalty real rather than a no-op. + var combatant = defeated.FindBehavior(); + if (combatant is not null) + combatant.CurrentHitPoints = Math.Max(1, combatant.MaxHitPoints / 2); + var stats = defeated.FindBehavior(); if (stats is not null) { diff --git a/src/SharpMud.Ruleset.Basic/BasicPlayerFactory.cs b/src/SharpMud.Ruleset.Basic/BasicPlayerFactory.cs index 62cdcca..bdddf78 100644 --- a/src/SharpMud.Ruleset.Basic/BasicPlayerFactory.cs +++ b/src/SharpMud.Ruleset.Basic/BasicPlayerFactory.cs @@ -15,11 +15,19 @@ public sealed class BasicPlayerFactory : IPlayerFactory { private readonly BasicRulesetOptions _options; + /// Creates the factory against the configured (starting HP/AC/damage). public BasicPlayerFactory(BasicRulesetOptions options) { _options = options; } + /// + /// Creates a player with , , , and + /// (seeded from ), adds it to , and registers it with . + /// public Thing CreatePlayer(World world, string username, string passwordHash, Thing startingRoom) { var player = new Thing { Id = ThingId.New(), Name = username }; diff --git a/src/SharpMud.Ruleset.Basic/BasicRulesetOptions.cs b/src/SharpMud.Ruleset.Basic/BasicRulesetOptions.cs index 0f51ea7..4321028 100644 --- a/src/SharpMud.Ruleset.Basic/BasicRulesetOptions.cs +++ b/src/SharpMud.Ruleset.Basic/BasicRulesetOptions.cs @@ -7,8 +7,29 @@ namespace SharpMud.Ruleset.Basic; // stats regardless (small, deliberately simple content, not a tunable). public sealed class BasicRulesetOptions { + /// A fresh character's starting (and max) hit points. Must be at least 1 - see . public int StartingHitPoints { get; set; } = 20; + + /// A fresh character's starting armor class. public int StartingArmorClass { get; set; } = 10; + + /// A fresh character's minimum damage per hit. Must be at least 1 - see . public int StartingDamageMin { get; set; } = 1; + + /// A fresh character's maximum damage per hit. Must be at least - see . public int StartingDamageMax { get; set; } = 4; + + /// + /// Fails fast at composition-root time on a combat-breaking configuration + /// (non-positive starting HP/damage, or a damage range with no valid + /// rolls) - called by AddSharpMudBasicRuleset(...) right after its + /// configureOptions callback runs, so a bad value surfaces at + /// startup instead of the first time a fight actually happens. + /// + public void Validate() + { + ArgumentOutOfRangeException.ThrowIfLessThan(StartingHitPoints, 1, nameof(StartingHitPoints)); + ArgumentOutOfRangeException.ThrowIfLessThan(StartingDamageMin, 1, nameof(StartingDamageMin)); + ArgumentOutOfRangeException.ThrowIfLessThan(StartingDamageMax, StartingDamageMin, nameof(StartingDamageMax)); + } } diff --git a/src/SharpMud.Ruleset.Basic/BasicStatsBehavior.cs b/src/SharpMud.Ruleset.Basic/BasicStatsBehavior.cs index 34d4223..30c3fb9 100644 --- a/src/SharpMud.Ruleset.Basic/BasicStatsBehavior.cs +++ b/src/SharpMud.Ruleset.Basic/BasicStatsBehavior.cs @@ -1,4 +1,5 @@ using SharpMud.Engine.Core; +using SharpMud.Ruleset.Rpg; namespace SharpMud.Ruleset.Basic; @@ -6,8 +7,16 @@ namespace SharpMud.Ruleset.Basic; // (that's Classic-flavored content, not what Basic promises). Attached // alongside SharpMud.Engine's PlayerBehavior to make a Thing a character; // SharpMud.Ruleset.Rpg's CombatantBehavior handles the actual combat numbers. +/// +/// Basic's minimal character-progression behavior - just level and +/// experience, no race/class/attributes. Combat HP lives on , not here. +/// public sealed class BasicStatsBehavior : Behavior { + /// The character's level. Not currently used to modify combat numbers - see docs/character.md. public int Level { get; set; } = 1; + + /// Accumulated experience, adjusted by on combat wins/losses. public long Experience { get; set; } } diff --git a/src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs b/src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs index b5b4c20..81532ca 100644 --- a/src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs +++ b/src/SharpMud.Ruleset.Basic/BasicWorldBuilder.cs @@ -17,10 +17,13 @@ public sealed class BasicWorldBuilder : IWorldBuilder // Fixed, not ThingId.New() - so a fresh boot can ask the repository // "does this already exist?" instead of always rebuilding. See // docs/persistence.md. + /// The fixed id of the default world's root area - stable across restarts so a persisted world can be found again. public static readonly ThingId AreaId = new(Guid.Parse("00000000-0000-0000-0000-000000000002")); + /// public ThingId RootId => AreaId; + /// Builds the default two-room world (a Clearing and an Old Watchtower) with one fightable NPC (a wild boar) in the watchtower. public (World World, Thing StartingRoom) Build() { var world = new World(); @@ -53,6 +56,7 @@ public sealed class BasicWorldBuilder : IWorldBuilder return (world, clearing); } + /// public Thing FindStartingRoom(Thing root) => root.Children.FirstOrDefault(c => c.HasBehavior() && c.Name == "Clearing") ?? root.Children.First(c => c.HasBehavior()); diff --git a/src/SharpMud.Ruleset.Basic/ServiceCollectionExtensions.cs b/src/SharpMud.Ruleset.Basic/ServiceCollectionExtensions.cs index 6bb2469..3659770 100644 --- a/src/SharpMud.Ruleset.Basic/ServiceCollectionExtensions.cs +++ b/src/SharpMud.Ruleset.Basic/ServiceCollectionExtensions.cs @@ -6,6 +6,7 @@ namespace SharpMud.Ruleset.Basic; +/// DI registration entry point for this package. public static class ServiceCollectionExtensions { /// @@ -19,7 +20,7 @@ public static class ServiceCollectionExtensions /// (docs/adr/0008-ruleset-scaffolding-tier.md). /// /// The consumer's . - /// Tunes the starting numbers for a fresh player character - see . + /// Tunes the starting numbers for a fresh player character - see . Validated immediately after this callback runs; an invalid combination throws at startup rather than at first combat. /// Forwarded to AddSharpMudRpgRuleset(...) - a consumer's own commands, registered alongside kill/attack/flee. public static IServiceCollection AddSharpMudBasicRuleset( this IServiceCollection services, @@ -28,6 +29,7 @@ public static IServiceCollection AddSharpMudBasicRuleset( { var options = new BasicRulesetOptions(); configureOptions?.Invoke(options); + options.Validate(); services.AddSingleton(options); services.AddSingleton(); diff --git a/src/SharpMud.Ruleset.Rpg/AttackCommand.cs b/src/SharpMud.Ruleset.Rpg/AttackCommand.cs index 44250f8..7533b5b 100644 --- a/src/SharpMud.Ruleset.Rpg/AttackCommand.cs +++ b/src/SharpMud.Ruleset.Rpg/AttackCommand.cs @@ -3,18 +3,34 @@ namespace SharpMud.Ruleset.Rpg; +/// +/// The kill/attack command - starts a combat encounter against +/// an NPC in the current room. Registered by +/// AddSharpMudRpgRuleset(...), not meant to be constructed directly +/// by a consumer. +/// public sealed class AttackCommand : ICommand { private readonly ICombatManager _combatManager; + /// Creates the command against the shared . public AttackCommand(ICombatManager combatManager) { _combatManager = combatManager; } + /// The canonical verb, kill. public string Verb => "kill"; + + /// Aliases for - just attack. public IReadOnlyList Aliases { get; } = ["attack"]; + /// + /// Guards (not already fighting, actor can fight, a matching NPC target + /// exists in the room), then starts the encounter via . Resolution happens on the + /// next game tick, not synchronously here. + /// public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) { if (await CommandGuards.RequireArgsAsync(ctx, "Kill what?", ct)) @@ -26,6 +42,17 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) return; } + // Every built-in IPlayerFactory attaches CombatantBehavior to a + // fresh player, but nothing enforces that for a consumer's own + // IPlayerFactory - without this guard, a player missing it would + // start an encounter here successfully and only fail later, at tick + // time, on CombatResolver's attacker.FindBehavior()!. + if (!ctx.Actor.HasBehavior()) + { + await ctx.Session.WriteLineAsync("You have no way to fight.", ct); + return; + } + var targetName = string.Join(' ', ctx.Args); var target = ObjectMatcher.FindMatch( ctx.CurrentRoom.Children.Where(c => c.HasBehavior() && c.HasBehavior()), diff --git a/src/SharpMud.Ruleset.Rpg/CombatManager.cs b/src/SharpMud.Ruleset.Rpg/CombatManager.cs index 654e9a0..b7497f6 100644 --- a/src/SharpMud.Ruleset.Rpg/CombatManager.cs +++ b/src/SharpMud.Ruleset.Rpg/CombatManager.cs @@ -17,28 +17,43 @@ namespace SharpMud.Ruleset.Rpg; // concrete ruleset's stats behavior or a hard-coded room directly - this // package has zero reference to any concrete ruleset's types (see // docs/adr/0008-ruleset-scaffolding-tier.md's Decision Outcome). +/// +/// Tracks active combat encounters and resolves them once per game tick - +/// the concrete implementation of , registered +/// by AddSharpMudRpgRuleset(...) as both +/// and off the same instance. Public (rather than +/// internal) specifically so a consumer can drive it directly against a +/// custom in their own tests, without +/// going through DI. +/// public sealed class CombatManager : ICombatManager, ITickable { private readonly ICombatResolver _resolver; private readonly ICombatOutcomeHandler _outcomeHandler; private readonly Dictionary _encounters = []; + /// Creates the manager against a resolver and a ruleset's outcome handler. public CombatManager(ICombatResolver resolver, ICombatOutcomeHandler outcomeHandler) { _resolver = resolver; _outcomeHandler = outcomeHandler; } + /// public bool IsInCombat(ThingId thingId) => _encounters.ContainsKey(thingId); + /// public void StartEncounter(Thing attacker, Thing defender) => _encounters[attacker.Id] = new CombatEncounter { Attacker = attacker, Defender = defender }; + /// public void EndEncounter(ThingId thingId) => _encounters.Remove(thingId); + /// public bool TryGetEncounter(ThingId thingId, [MaybeNullWhen(false)] out CombatEncounter encounter) => _encounters.TryGetValue(thingId, out encounter); + /// Resolves one round for every active encounter - see the class remarks above for the full sequence. public async Task OnTickAsync(TickContext ctx, CancellationToken ct) { foreach (var thingId in _encounters.Keys.ToArray()) @@ -112,15 +127,25 @@ private async Task HandleAttackerDefeatedAsync(CombatEncounter encounter, ISessi // latter left CombatantBehavior.CurrentHitPoints at/below 0, so the // very next hit instantly re-triggered "defeated" regardless of the // roll. This reset is generic (CombatantBehavior is this package's - // own type) so it happens here, unconditionally, before the - // ruleset-specific outcome handler runs. + // own type) so it happens here, as a safe full-HP baseline, before + // the ruleset-specific outcome handler runs - NOT as the final word. + // A ruleset that wants a real death penalty (e.g. respawn at half + // HP, not full) mutates CombatantBehavior.CurrentHitPoints itself + // inside OnDefeatAsync below, which runs after this and therefore + // wins - see ClassicCombatOutcomeHandler/BasicCombatOutcomeHandler. + // Without that override, "no penalty, full-HP respawn" is the + // correct default for a ruleset that doesn't want one. var combatant = attacker.FindBehavior()!; combatant.CurrentHitPoints = combatant.MaxHitPoints; - var destination = await _outcomeHandler.OnDefeatAsync(attacker, encounter.Defender, ct); - + // Generic defeat message first, then the ruleset-specific outcome + // handler (which may itself write its own messages, e.g. an XP-loss + // line) - same ordering as HandleDefenderDefeatedAsync above, and + // matches the message order from before this extraction. await session.WriteLineAsync($"{encounter.Defender.Name} has slain you!", ct); + var destination = await _outcomeHandler.OnDefeatAsync(attacker, encounter.Defender, ct); + _encounters.Remove(attacker.Id); attacker.Parent?.Remove(attacker); diff --git a/src/SharpMud.Ruleset.Rpg/CombatResolver.cs b/src/SharpMud.Ruleset.Rpg/CombatResolver.cs index a25f414..352c68c 100644 --- a/src/SharpMud.Ruleset.Rpg/CombatResolver.cs +++ b/src/SharpMud.Ruleset.Rpg/CombatResolver.cs @@ -5,17 +5,27 @@ namespace SharpMud.Ruleset.Rpg; // Diku/Circle-style d20-vs-AC roll (docs/combat.md decision). Level/skill // to-hit modifiers and the exact damage formula are still open items there - // this is currently an unmodified d20 roll against the defender's AC. +/// +/// The default - a Diku/Circle-style +/// unmodified d20-vs-armor-class roll, with damage rolled from the +/// attacker's . Public (rather +/// than internal) so a consumer can construct it directly in their own +/// tests, or explicitly replace the DI +/// registration with their own combat math. +/// public sealed class CombatResolver : ICombatResolver { private readonly IDiceRoller _dice; private readonly IRandomSource _random; + /// Creates the resolver against a dice roller and a raw randomness source (for the damage roll - see remarks on ). public CombatResolver(IDiceRoller dice, IRandomSource random) { _dice = dice; _random = random; } + /// public CombatRoundResult ResolveRound(Thing attacker, Thing defender) { var attackerCombatant = attacker.FindBehavior()!; diff --git a/src/SharpMud.Ruleset.Rpg/CombatantBehavior.cs b/src/SharpMud.Ruleset.Rpg/CombatantBehavior.cs index eb28961..88b7b68 100644 --- a/src/SharpMud.Ruleset.Rpg/CombatantBehavior.cs +++ b/src/SharpMud.Ruleset.Rpg/CombatantBehavior.cs @@ -10,12 +10,24 @@ namespace SharpMud.Ruleset.Rpg; /// public sealed class CombatantBehavior : Behavior { + /// Hit points at full health. public int MaxHitPoints { get; set; } + + /// Current hit points - the value actually reads/writes during combat. public int CurrentHitPoints { get; set; } + + /// Armor class - the to-hit roll must meet or exceed this to land a hit. public int ArmorClass { get; set; } + + /// Minimum damage rolled on a hit - see . public int DamageMin { get; set; } + + /// Maximum damage rolled on a hit - see . public int DamageMax { get; set; } + + /// XP awarded to the victor when this combatant is defeated - see . public int ExperienceReward { get; set; } + /// The (, ) pair, as read by . public (int Min, int Max) DamageRange => (DamageMin, DamageMax); } diff --git a/src/SharpMud.Ruleset.Rpg/DiceRoller.cs b/src/SharpMud.Ruleset.Rpg/DiceRoller.cs index f3007c6..519d831 100644 --- a/src/SharpMud.Ruleset.Rpg/DiceRoller.cs +++ b/src/SharpMud.Ruleset.Rpg/DiceRoller.cs @@ -2,15 +2,24 @@ namespace SharpMud.Ruleset.Rpg; +/// +/// The default - sums diceCount +/// independent rolls of over [1, +/// sides], plus a flat modifier. Public (rather than internal) so a +/// consumer can construct it directly in their own tests. +/// public sealed class DiceRoller : IDiceRoller { private readonly IRandomSource _random; + /// Creates the roller against the engine's randomness source. public DiceRoller(IRandomSource random) { _random = random; } + /// + /// or is less than 1. public int Roll(int diceCount, int sides, int modifier = 0) { ArgumentOutOfRangeException.ThrowIfLessThan(diceCount, 1); diff --git a/src/SharpMud.Ruleset.Rpg/FleeCommand.cs b/src/SharpMud.Ruleset.Rpg/FleeCommand.cs index 079b260..a8c936d 100644 --- a/src/SharpMud.Ruleset.Rpg/FleeCommand.cs +++ b/src/SharpMud.Ruleset.Rpg/FleeCommand.cs @@ -5,12 +5,18 @@ namespace SharpMud.Ruleset.Rpg; +/// +/// The flee command - attempts to escape an active combat encounter +/// through a random exit. Registered by AddSharpMudRpgRuleset(...), +/// not meant to be constructed directly by a consumer. +/// public sealed class FleeCommand : ICommand { private readonly ICombatManager _combatManager; private readonly IDiceRoller _dice; private readonly IRandomSource _random; + /// Creates the command against the shared and dice/randomness sources. public FleeCommand(ICombatManager combatManager, IDiceRoller dice, IRandomSource random) { _combatManager = combatManager; @@ -18,9 +24,17 @@ public FleeCommand(ICombatManager combatManager, IDiceRoller dice, IRandomSource _random = random; } + /// The canonical verb, flee. No aliases. public string Verb => "flee"; + + /// No aliases for . public IReadOnlyList Aliases { get; } = []; + /// + /// Guards (an active encounter exists, the current room has an exit), + /// rolls a flat success chance, and on success ends the encounter and + /// moves the actor through a random exit. + /// public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) { if (!_combatManager.TryGetEncounter(ctx.Actor.Id, out _)) diff --git a/src/SharpMud.Ruleset.Rpg/ICombatManager.cs b/src/SharpMud.Ruleset.Rpg/ICombatManager.cs index 8484d60..e9caf38 100644 --- a/src/SharpMud.Ruleset.Rpg/ICombatManager.cs +++ b/src/SharpMud.Ruleset.Rpg/ICombatManager.cs @@ -3,18 +3,35 @@ namespace SharpMud.Ruleset.Rpg; +/// An active combat encounter - the attacking and the it's fighting. public sealed class CombatEncounter { + /// The Thing that initiated the encounter (always the player, per the v1 scope note below). public required Thing Attacker { get; init; } + + /// The Thing being attacked. public required Thing Defender { get; init; } } // v1 scope is player-vs-NPC only - no PvP verb/aggression rules exist yet, // so an encounter is always keyed by the attacking Thing. +/// +/// Tracks and resolves active combat encounters. Implemented by ; a consumer typically only interacts with this +/// interface (e.g. a custom command checking ), +/// resolved from DI rather than constructed directly. +/// public interface ICombatManager { + /// Whether the given Thing is currently the attacker in an active encounter. bool IsInCombat(ThingId thingId); + + /// Starts (or replaces) the encounter keyed by . void StartEncounter(Thing attacker, Thing defender); + + /// Ends the encounter keyed by the given Thing, if one exists. void EndEncounter(ThingId thingId); + + /// Attempts to get the active encounter keyed by the given Thing. bool TryGetEncounter(ThingId thingId, [MaybeNullWhen(false)] out CombatEncounter encounter); } diff --git a/src/SharpMud.Ruleset.Rpg/ICombatOutcomeHandler.cs b/src/SharpMud.Ruleset.Rpg/ICombatOutcomeHandler.cs index 8bd106a..9904172 100644 --- a/src/SharpMud.Ruleset.Rpg/ICombatOutcomeHandler.cs +++ b/src/SharpMud.Ruleset.Rpg/ICombatOutcomeHandler.cs @@ -24,9 +24,15 @@ public interface ICombatOutcomeHandler /// Called when loses the encounter to /// - the hook for a death penalty (XP loss, /// stats-specific HP reset) and for deciding the respawn destination. - /// is already reset by - /// before this is called, regardless of what - /// this method does. + /// already reset 's + /// to full as a safe baseline before this is called - a "no penalty" + /// ruleset can rely on that and do nothing here, but a ruleset that + /// wants a real HP penalty (e.g. respawn at half HP) must mutate itself inside this + /// method, since it's the value actually + /// reads/writes in combat - see ClassicCombatOutcomeHandler/ + /// BasicCombatOutcomeHandler for worked examples. /// Task OnDefeatAsync(Thing defeated, Thing victor, CancellationToken ct); } diff --git a/src/SharpMud.Ruleset.Rpg/ICombatResolver.cs b/src/SharpMud.Ruleset.Rpg/ICombatResolver.cs index 50c289c..10023e6 100644 --- a/src/SharpMud.Ruleset.Rpg/ICombatResolver.cs +++ b/src/SharpMud.Ruleset.Rpg/ICombatResolver.cs @@ -2,10 +2,20 @@ namespace SharpMud.Ruleset.Rpg; +/// The outcome of one call. +/// Whether the attack landed. +/// Damage applied - always 0 when is . +/// Whether this round dropped the defender's to zero or below. public sealed record CombatRoundResult(bool Hit, int Damage, bool DefenderDefeated); /// Resolves a single round of combat between two -carrying Things. public interface ICombatResolver { + /// + /// Resolves and applies one round: rolls to hit, and on a hit rolls and + /// applies damage directly to 's . Both Things must already + /// carry . + /// CombatRoundResult ResolveRound(Thing attacker, Thing defender); } diff --git a/src/SharpMud.Ruleset.Rpg/RpgBehaviorMappingContributor.cs b/src/SharpMud.Ruleset.Rpg/RpgBehaviorMappingContributor.cs index 78ec936..34a75e2 100644 --- a/src/SharpMud.Ruleset.Rpg/RpgBehaviorMappingContributor.cs +++ b/src/SharpMud.Ruleset.Rpg/RpgBehaviorMappingContributor.cs @@ -9,8 +9,15 @@ namespace SharpMud.Ruleset.Rpg; // IEntityTypeConfiguration<> types, same pattern as // ClassicBehaviorMappingContributor - a consumer's own contributor scans // its own assembly, never this one's. +/// +/// This package's - registers 's EF Core mapping. Registered automatically by +/// AddSharpMudRpgRuleset(...); a consumer doesn't need to register it +/// themselves. +/// public sealed class RpgBehaviorMappingContributor : IBehaviorMappingContributor { + /// Applies every IEntityTypeConfiguration<T> in this assembly - currently just 's. public void ConfigureBehaviors(ModelBuilder modelBuilder) => modelBuilder.ApplyConfigurationsFromAssembly(typeof(RpgBehaviorMappingContributor).Assembly); } diff --git a/src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs b/src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs index 8be25f0..aa5146d 100644 --- a/src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs +++ b/src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs @@ -7,6 +7,7 @@ namespace SharpMud.Ruleset.Rpg; +/// DI registration entry point for this package. public static class ServiceCollectionExtensions { /// diff --git a/tests/SharpMud.Ruleset.Basic.Tests/BasicRulesetOptionsTests.cs b/tests/SharpMud.Ruleset.Basic.Tests/BasicRulesetOptionsTests.cs new file mode 100644 index 0000000..5e1923b --- /dev/null +++ b/tests/SharpMud.Ruleset.Basic.Tests/BasicRulesetOptionsTests.cs @@ -0,0 +1,48 @@ +namespace SharpMud.Ruleset.Basic.Tests; + +public sealed class BasicRulesetOptionsTests +{ + [Fact] + public void Validate_DoesNotThrow_ForDefaultOptions() + { + var sut = new BasicRulesetOptions(); + + var act = sut.Validate; + + act.Should().NotThrow(); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void Validate_Throws_WhenStartingHitPointsIsNotPositive(int hitPoints) + { + var sut = new BasicRulesetOptions { StartingHitPoints = hitPoints }; + + var act = sut.Validate; + + act.Should().Throw(); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void Validate_Throws_WhenStartingDamageMinIsNotPositive(int damageMin) + { + var sut = new BasicRulesetOptions { StartingDamageMin = damageMin }; + + var act = sut.Validate; + + act.Should().Throw(); + } + + [Fact] + public void Validate_Throws_WhenStartingDamageMaxIsLessThanStartingDamageMin() + { + var sut = new BasicRulesetOptions { StartingDamageMin = 5, StartingDamageMax = 4 }; + + var act = sut.Validate; + + act.Should().Throw(); + } +} diff --git a/tests/SharpMud.Ruleset.Basic.Tests/ServiceCollectionExtensionsTests.cs b/tests/SharpMud.Ruleset.Basic.Tests/ServiceCollectionExtensionsTests.cs index d71ef6c..f333ab2 100644 --- a/tests/SharpMud.Ruleset.Basic.Tests/ServiceCollectionExtensionsTests.cs +++ b/tests/SharpMud.Ruleset.Basic.Tests/ServiceCollectionExtensionsTests.cs @@ -47,4 +47,18 @@ public void AddSharpMudBasicRuleset_AppliesConfigureOptionsCallback() options.StartingHitPoints.Should().Be(42); } + + // Without this, an invalid StartingHitPoints/StartingDamageMin/Max only + // surfaces the first time a fight actually happens - IRandomSource.Next(min, max) + // throwing mid-combat, not a clear failure at startup. + [Fact] + public void AddSharpMudBasicRuleset_ThrowsAtCompositionRootTime_WhenOptionsAreInvalid() + { + var services = new ServiceCollection(); + services.AddSingleton(Substitute.For()); + + var act = () => services.AddSharpMudBasicRuleset(options => options.StartingHitPoints = 0); + + act.Should().Throw(); + } } diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs index 05b24c7..6951ab7 100644 --- a/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs +++ b/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs @@ -74,6 +74,45 @@ public async Task OnTickAsync_ResetsCombatantHitPointsAndRespawnsAtHandlerDestin sut.IsInCombat(player.Id).Should().BeFalse(); } + [Fact] + public async Task OnTickAsync_SendsDefeatMessageBeforeInvokingOutcomeHandler_WhenNpcDefeatsPlayer() + { + var resolver = Substitute.For(); + var outcomeHandler = Substitute.For(); + var session = Substitute.For(); + var hubRoom = new Thing { Id = ThingId.New(), Name = "Hub" }; + var callOrder = new List(); + + var room = new Thing { Id = ThingId.New(), Name = "Room" }; + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + player.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash", Session = session }); + player.Behaviors.Add(new CombatantBehavior { MaxHitPoints = 20, CurrentHitPoints = 20 }); + room.Add(player); + + var npc = new Thing { Id = ThingId.New(), Name = "cave rat" }; + npc.Behaviors.Add(new NpcBehavior()); + npc.Behaviors.Add(new CombatantBehavior { CurrentHitPoints = 6 }); + room.Add(npc); + + resolver.ResolveRound(player, npc).Returns(new CombatRoundResult(false, 0, false)); + resolver.ResolveRound(npc, player).Returns(new CombatRoundResult(true, 999, true)); + session.When(s => s.WriteLineAsync("cave rat has slain you!", Arg.Any())) + .Do(_ => callOrder.Add("defeat-message")); + outcomeHandler.OnDefeatAsync(player, npc, TestContext.Current.CancellationToken) + .Returns(hubRoom) + .AndDoes(_ => callOrder.Add("outcome-handler")); + + var sut = new CombatManager(resolver, outcomeHandler); + sut.StartEncounter(player, npc); + + await sut.OnTickAsync(new TickContext(DateTimeOffset.UtcNow), TestContext.Current.CancellationToken); + + // Matches the message order from before the ADR-0008 extraction - + // the generic "has slain you!" message must still arrive before any + // ruleset-specific outcome-handler messaging (e.g. an XP-loss line). + callOrder.Should().Equal("defeat-message", "outcome-handler"); + } + [Fact] public async Task OnTickAsync_FreezesEncounter_WhenAttackerLinkdeadWithinGraceWindow() { diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs index e3e3ed6..ebaf69a 100644 --- a/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs +++ b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs @@ -15,6 +15,7 @@ public async Task ExecuteAsync_StartsEncounter_WhenTargetExists() var room = new Thing { Id = ThingId.New(), Name = "Room" }; var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + player.Behaviors.Add(new CombatantBehavior()); room.Add(player); var npc = new Thing { Id = ThingId.New(), Name = "cave rat" }; @@ -39,6 +40,7 @@ public async Task ExecuteAsync_SendsNotHereMessage_WhenNoMatchingCombatantInRoom var room = new Thing { Id = ThingId.New(), Name = "Room" }; var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + player.Behaviors.Add(new CombatantBehavior()); room.Add(player); var sut = new AttackCommand(combatManager); @@ -50,6 +52,30 @@ public async Task ExecuteAsync_SendsNotHereMessage_WhenNoMatchingCombatantInRoom await session.Received(1).WriteLineAsync("You don't see that here.", Arg.Any()); } + [Fact] + public async Task ExecuteAsync_SendsCannotFightMessage_WhenActorHasNoCombatantBehavior() + { + var combatManager = Substitute.For(); + var session = Substitute.For(); + + var room = new Thing { Id = ThingId.New(), Name = "Room" }; + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + room.Add(player); + + var npc = new Thing { Id = ThingId.New(), Name = "cave rat" }; + npc.Behaviors.Add(new NpcBehavior()); + npc.Behaviors.Add(new CombatantBehavior()); + room.Add(npc); + + var sut = new AttackCommand(combatManager); + var ctx = new CommandContext(player, room, ["cave", "rat"], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + combatManager.DidNotReceiveWithAnyArgs().StartEncounter(default!, default!); + await session.Received(1).WriteLineAsync("You have no way to fight.", Arg.Any()); + } + [Fact] public async Task ExecuteAsync_SendsAlreadyFightingMessage_WhenActorAlreadyInCombat() { From 9c3f0c740751a70b3728c71ab8d24d63f7dd94d0 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 11:04:09 -0400 Subject: [PATCH 3/6] fix: address second round of PR #18 review feedback Fixes a real, pre-existing bug FleeCommand carried over from before this PR's extraction: it moved the actor directly instead of publishing the same UseExitEvent request MoveCommand does, so a locked exit blocked a normal move but not a flee through the exact same exit. FleeCommand now checks the lock (or any future exit veto) before ending the encounter and moving. Also: adds the test coverage the first review-feedback fix was missing (CombatantBehavior.CurrentHitPoints assertions in Classic's outcome-handler test, plus a new BasicCombatOutcomeHandlerTests covering the same death penalty for Basic - neither existed before, so the HP-halving fix itself had no regression coverage where it actually mattered); narrows CombatantBehaviorConfiguration/BasicStatsBehaviorConfiguration to internal (never referenced by name outside their own assembly, discovered only via EF's ApplyConfigurationsFromAssembly reflection scan - confirmed still works internal via the persistence round-trip tests); and brings docs/combat.md, docs/architecture.md, docs/plans/0008-ruleset-scaffolding-tier.md, and docsite/docs/rulesets.md up to date with the HP-halving fix and the FleeCommand fix from this round. Co-Authored-By: Claude Sonnet 5 --- docs/architecture.md | 4 +- docs/combat.md | 45 +++++++++----- docs/plans/0008-ruleset-scaffolding-tier.md | 12 +++- docsite/docs/rulesets.md | 23 +++++++- .../BasicStatsBehaviorConfiguration.cs | 5 +- .../CombatantBehaviorConfiguration.cs | 5 +- src/SharpMud.Ruleset.Rpg/FleeCommand.cs | 19 +++++- .../BasicCombatOutcomeHandlerTests.cs | 59 +++++++++++++++++++ .../Commands/FleeCommandTests.cs | 34 +++++++++++ .../ClassicCombatOutcomeHandlerTests.cs | 7 +++ 10 files changed, 185 insertions(+), 28 deletions(-) create mode 100644 tests/SharpMud.Ruleset.Basic.Tests/BasicCombatOutcomeHandlerTests.cs diff --git a/docs/architecture.md b/docs/architecture.md index ebd150b..9a1d284 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -107,8 +107,8 @@ provider (see [persistence.md](persistence.md)), the chosen transport - **Unit tests** (xUnit v3 + AutoFixture + NSubstitute + AwesomeAssertions, per the `dotnet-unit-testing-patterns` skill conventions) required for: - `ICommandParser`, each `ICommand` implementation, `ICombatResolver` (now in - `SharpMud.Samples.Classic.Tests`), `Thing`/`BehaviorManager`/`ThingEvents` + `ICommandParser`, each `ICommand` implementation, `ICombatResolver` (in + `SharpMud.Ruleset.Rpg.Tests` as of ADR-0008), `Thing`/`BehaviorManager`/`ThingEvents` propagation, stat-derivation formulas. These are pure/deterministic enough to test without a live session or tick loop — `ISession` and repositories are mocked via NSubstitute. diff --git a/docs/combat.md b/docs/combat.md index c5af4ae..9ca6f08 100644 --- a/docs/combat.md +++ b/docs/combat.md @@ -173,16 +173,24 @@ Classic-stakes model, split between `CombatManager` (generic) and `CombatantBehavior.ExperienceReward`; Basic's does the same against `BasicStatsBehavior`. Loot drops are **not implemented** — the item system itself is a later build-order phase, so there's nothing to drop yet. -- **Player death**: `CombatManager` unconditionally resets the loser's own - `CombatantBehavior.CurrentHitPoints` to `MaxHitPoints` (see the bug note - below), then calls `OnDefeatAsync`, which returns the respawn `Thing` and - applies whatever ruleset-specific penalty it wants — Classic's handler - reduces `StatsBehavior.Experience` by a flat **10%** (placeholder — exact - percentage is still an open item) and resets `StatsBehavior.CurrentHitPoints` - to `MaxHitPoints / 2` (placeholder, minimum 1), returning `WorldContext - .StartingRoom` (Classic's hub). `CombatManager` moves the attacker there and - sends the room description via `LookCommand.SendRoomDescriptionAsync`. No - item loss and no corpse-run. +- **Player death**: HP ownership is split between a generic safe baseline + and a ruleset-specific override. `CombatManager` unconditionally resets + the loser's `CombatantBehavior.CurrentHitPoints` to `MaxHitPoints` first + (see the bug note below) — a "no penalty" ruleset can rely on that and do + nothing further. `CombatManager` then calls `OnDefeatAsync`, which returns + the respawn `Thing` and may apply a real death penalty: Classic's and + Basic's handlers both reduce their own stats behavior's `Experience` by a + flat **10%** (placeholder — exact percentage is still an open item), *and* + halve `CombatantBehavior.CurrentHitPoints` again themselves (minimum 1) - + since that's the value `CombatResolver` actually reads/writes, the earlier + full-HP reset would otherwise make the documented "respawn at half HP" + penalty a no-op. Classic additionally mirrors the halved value into + `StatsBehavior.CurrentHitPoints` (its own character-sheet display field, + never read by combat) so the two don't visibly drift. Respawn destination + is `WorldContext.StartingRoom` for both (Classic's hub, Basic's clearing). + `CombatManager` moves the attacker there and sends the room description + via `LookCommand.SendRoomDescriptionAsync`. No item loss and no + corpse-run. **Bug fixed during the ADR-0008 extraction**: `CombatResolver` reads/writes damage against `CombatantBehavior.CurrentHitPoints`, not any ruleset stats @@ -190,7 +198,9 @@ Classic-stakes model, split between `CombatManager` (generic) and `CombatantBehavior.CurrentHitPoints` at/below 0 - the very next hit instantly re-triggered "defeated" regardless of the roll. `CombatManager` now resets `CombatantBehavior.CurrentHitPoints` itself, unconditionally, - before the outcome handler runs. + before the outcome handler runs — and a follow-up review round caught that + the outcome handlers also needed to override that reset for their own + penalty to have any actual combat effect (see above). ## Flee @@ -198,11 +208,14 @@ Implemented in `FleeCommand` (`SharpMud.Ruleset.Rpg`). Requires an active encounter (`ICombatManager.TryGetEncounter`) and at least one exit in the current room. Success chance is currently a **flat 60%** via `IDiceRoller.Roll(1, 100)` — the real DEX-differential formula from the -original design is still an open item, and `Npc`/`ICombatant` doesn't carry -Dexterity, so there's nothing to differential against yet. On success, a -random exit is chosen (`IRandomSource.Next` over the room's exits — index -selection isn't dice notation, so it stays a direct `IRandomSource` call, not -`IDiceRoller`), the encounter ends, and the actor moves exactly as a normal +original design is still an open item, and neither `CombatantBehavior` nor +any built-in ruleset's stats behavior carries a Dexterity-equivalent value +yet, so there's nothing to differential against. On success, a random exit +is chosen (`IRandomSource.Next` over the room's exits — index selection +isn't dice notation, so it stays a direct `IRandomSource` call, not +`IDiceRoller`) and checked through the same `UseExitEvent` request path +`MoveCommand` uses (so a locked exit can still block a flee), the encounter +ends, and the actor moves exactly as a normal move (see [commands.md](commands.md)). ## Dice-Rolling Abstraction diff --git a/docs/plans/0008-ruleset-scaffolding-tier.md b/docs/plans/0008-ruleset-scaffolding-tier.md index b2c4712..7e0498f 100644 --- a/docs/plans/0008-ruleset-scaffolding-tier.md +++ b/docs/plans/0008-ruleset-scaffolding-tier.md @@ -270,9 +270,15 @@ passing unit tests, to confirm. `OnDefeatAsync` returns the respawn `Thing`, so `CombatManager` never needs a `hubRoom`/`IWorld` reference of its own. The pre-existing `CombatantBehavior.CurrentHitPoints` respawn-reset bug is fixed as part of - this: `CombatManager` resets it unconditionally, before the outcome - handler runs (the outcome handler only owns its own ruleset's - stats-behavior-specific reset). + this: `CombatManager` resets it unconditionally to full, before the + outcome handler runs, as a safe baseline — a follow-up PR review round + then caught that this baseline alone made the documented HP-penalty a + no-op, since neither outcome handler touched `CombatantBehavior` at all + (only their own ruleset's stats-behavior HP field, never read by combat). + Both `ClassicCombatOutcomeHandler` and `BasicCombatOutcomeHandler` now + also halve `CombatantBehavior.CurrentHitPoints` themselves inside + `OnDefeatAsync`, overriding `CombatManager`'s baseline for rulesets that + want a real penalty. - **Package naming**: kept as `SharpMud.Ruleset.Rpg`/`SharpMud.Ruleset.Basic` — no objection raised during implementation. - **`AttackCommand`/`FleeCommand` composition**: the forwarding-callback diff --git a/docsite/docs/rulesets.md b/docsite/docs/rulesets.md index 5ef0c48..8e37986 100644 --- a/docsite/docs/rulesets.md +++ b/docsite/docs/rulesets.md @@ -140,9 +140,26 @@ public sealed class MyCombatOutcomeHandler(WorldContext worldContext) : ICombatO } ``` -You don't need to reset `CombatantBehavior.CurrentHitPoints` yourself — -`ICombatManager` already does that unconditionally before calling -`OnDefeatAsync`, regardless of what your handler does. +`ICombatManager` already resets `CombatantBehavior.CurrentHitPoints` to full +before calling `OnDefeatAsync` — so if you want a "no penalty, full-HP +respawn" ruleset (as `MyCombatOutcomeHandler` above does), you don't need to +touch it at all. But if your death penalty should actually cost HP (a +half-HP respawn, say), you have to set `CombatantBehavior.CurrentHitPoints` +yourself inside `OnDefeatAsync` — that field is what `ICombatResolver` +actually reads and writes during combat, so setting anything else (your own +stats behavior's own HP field, if it has one) has no effect on the fight +itself: + +```csharp +public Task OnDefeatAsync(Thing defeated, Thing victor, CancellationToken ct) +{ + var combatant = defeated.FindBehavior(); + if (combatant is not null) + combatant.CurrentHitPoints = Math.Max(1, combatant.MaxHitPoints / 2); + + return Task.FromResult(worldContext.StartingRoom); +} +``` ### Putting it together diff --git a/src/SharpMud.Ruleset.Basic/Configurations/BasicStatsBehaviorConfiguration.cs b/src/SharpMud.Ruleset.Basic/Configurations/BasicStatsBehaviorConfiguration.cs index 8eab19b..b52ace9 100644 --- a/src/SharpMud.Ruleset.Basic/Configurations/BasicStatsBehaviorConfiguration.cs +++ b/src/SharpMud.Ruleset.Basic/Configurations/BasicStatsBehaviorConfiguration.cs @@ -3,7 +3,10 @@ namespace SharpMud.Ruleset.Basic.Configurations; -public sealed class BasicStatsBehaviorConfiguration : IEntityTypeConfiguration +// internal: discovered via ApplyConfigurationsFromAssembly's reflection scan +// (BasicBehaviorMappingContributor), never referenced by name - not part of +// this package's public contract. +internal sealed class BasicStatsBehaviorConfiguration : IEntityTypeConfiguration { public void Configure(EntityTypeBuilder builder) { diff --git a/src/SharpMud.Ruleset.Rpg/Configurations/CombatantBehaviorConfiguration.cs b/src/SharpMud.Ruleset.Rpg/Configurations/CombatantBehaviorConfiguration.cs index 7a57024..c392f43 100644 --- a/src/SharpMud.Ruleset.Rpg/Configurations/CombatantBehaviorConfiguration.cs +++ b/src/SharpMud.Ruleset.Rpg/Configurations/CombatantBehaviorConfiguration.cs @@ -3,7 +3,10 @@ namespace SharpMud.Ruleset.Rpg.Configurations; -public sealed class CombatantBehaviorConfiguration : IEntityTypeConfiguration +// internal: discovered via ApplyConfigurationsFromAssembly's reflection scan +// (RpgBehaviorMappingContributor), never referenced by name - not part of +// this package's public contract. +internal sealed class CombatantBehaviorConfiguration : IEntityTypeConfiguration { public void Configure(EntityTypeBuilder builder) { diff --git a/src/SharpMud.Ruleset.Rpg/FleeCommand.cs b/src/SharpMud.Ruleset.Rpg/FleeCommand.cs index a8c936d..2fc4e7e 100644 --- a/src/SharpMud.Ruleset.Rpg/FleeCommand.cs +++ b/src/SharpMud.Ruleset.Rpg/FleeCommand.cs @@ -32,8 +32,10 @@ public FleeCommand(ICombatManager combatManager, IDiceRoller dice, IRandomSource /// /// Guards (an active encounter exists, the current room has an exit), - /// rolls a flat success chance, and on success ends the encounter and - /// moves the actor through a random exit. + /// rolls a flat success chance, and on success publishes the same request MoveCommand does (so a locked + /// exit can still veto) before ending the encounter and moving the + /// actor through the chosen exit. /// public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) { @@ -60,6 +62,19 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) } var exit = exits[_random.Next(0, exits.Count - 1)]; + + // Same request/cancellation path MoveCommand uses - without this, + // a locked exit blocks a normal move but not a flee through the + // exact same exit. + var exitThing = exit.Parent!; + var request = new UseExitEvent { ActiveThing = ctx.Actor, Exit = exitThing }; + exitThing.Events.PublishRequest(request, EventScope.SelfOnly); + if (request.IsCanceled) + { + await ctx.Session.WriteLineAsync(request.CancelReason ?? "You can't escape that way!", ct); + return; + } + var destination = exit.Destination; _combatManager.EndEncounter(ctx.Actor.Id); diff --git a/tests/SharpMud.Ruleset.Basic.Tests/BasicCombatOutcomeHandlerTests.cs b/tests/SharpMud.Ruleset.Basic.Tests/BasicCombatOutcomeHandlerTests.cs new file mode 100644 index 0000000..4d9d5cf --- /dev/null +++ b/tests/SharpMud.Ruleset.Basic.Tests/BasicCombatOutcomeHandlerTests.cs @@ -0,0 +1,59 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; +using SharpMud.Hosting; +using SharpMud.Ruleset.Rpg; + +namespace SharpMud.Ruleset.Basic.Tests; + +public sealed class BasicCombatOutcomeHandlerTests +{ + [Fact] + public async Task OnVictoryAsync_AwardsExperienceFromCombatantReward() + { + var session = Substitute.For(); + var victor = new Thing { Id = ThingId.New(), Name = "Hero" }; + victor.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash", Session = session }); + victor.Behaviors.Add(new BasicStatsBehavior { Experience = 0 }); + + var defeated = new Thing { Id = ThingId.New(), Name = "wild boar" }; + defeated.Behaviors.Add(new CombatantBehavior { ExperienceReward = 10 }); + + var worldContext = new WorldContext(); + var sut = new BasicCombatOutcomeHandler(worldContext); + + await sut.OnVictoryAsync(victor, defeated, TestContext.Current.CancellationToken); + + victor.FindBehavior()!.Experience.Should().Be(10); + } + + [Fact] + public async Task OnDefeatAsync_AppliesXpLossAndHalvesCombatHitPoints_AndReturnsStartingRoom() + { + var session = Substitute.For(); + var defeated = new Thing { Id = ThingId.New(), Name = "Hero" }; + defeated.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash", Session = session }); + defeated.Behaviors.Add(new BasicStatsBehavior { Experience = 100 }); + defeated.Behaviors.Add(new CombatantBehavior { MaxHitPoints = 20, CurrentHitPoints = 20 }); + + var victor = new Thing { Id = ThingId.New(), Name = "wild boar" }; + + var startingRoom = new Thing { Id = ThingId.New(), Name = "Clearing" }; + startingRoom.Behaviors.Add(new RoomBehavior()); + var worldContext = new WorldContext(); + worldContext.Initialize(new World(), startingRoom, startingRoom); + + var sut = new BasicCombatOutcomeHandler(worldContext); + + var destination = await sut.OnDefeatAsync(defeated, victor, TestContext.Current.CancellationToken); + + destination.Should().Be(startingRoom); + defeated.FindBehavior()!.Experience.Should().Be(90); + + // The value CombatResolver actually reads/writes during combat - + // BasicStatsBehavior carries no HP field of its own, so this is the + // only assertion that would catch the death penalty regressing to + // a no-op (full-HP respawn). + defeated.FindBehavior()!.CurrentHitPoints.Should().Be(10); + } +} diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs index eb6ddc5..e99afa6 100644 --- a/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs +++ b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs @@ -40,6 +40,40 @@ public async Task ExecuteAsync_MovesActorAndEndsEncounter_WhenRollSucceeds() origin.Children.Should().NotContain(player); } + [Fact] + public async Task ExecuteAsync_DoesNotMoveOrEndEncounter_WhenChosenExitIsLocked() + { + var combatManager = Substitute.For(); + var dice = Substitute.For(); + var random = Substitute.For(); + var session = Substitute.For(); + var encounter = new CombatEncounter { Attacker = new Thing { Id = ThingId.New(), Name = "x" }, Defender = new Thing { Id = ThingId.New(), Name = "y" } }; + combatManager.TryGetEncounter(Arg.Any(), out Arg.Any()) + .Returns(x => { x[1] = encounter; return true; }); + dice.Roll(1, 100).Returns(1); + random.Next(0, 0).Returns(0); + + var origin = new Thing { Id = ThingId.New(), Name = "Origin" }; + var destination = new Thing { Id = ThingId.New(), Name = "Destination" }; + var exit = new Thing { Id = ThingId.New(), Name = "north" }; + exit.Behaviors.Add(new ExitBehavior { Direction = Direction.North, Destination = destination }); + exit.Behaviors.Add(new LockableBehavior { IsLocked = true }); + origin.Add(exit); + + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + player.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash", Session = session }); + origin.Add(player); + + var sut = new FleeCommand(combatManager, dice, random); + var ctx = new CommandContext(player, origin, [], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + combatManager.DidNotReceiveWithAnyArgs().EndEncounter(default!); + player.Parent.Should().Be(origin); + await session.Received(1).WriteLineAsync("The door is locked.", Arg.Any()); + } + [Fact] public async Task ExecuteAsync_SendsFailureMessage_WhenRollFails() { diff --git a/tests/SharpMud.Samples.Classic.Tests/ClassicCombatOutcomeHandlerTests.cs b/tests/SharpMud.Samples.Classic.Tests/ClassicCombatOutcomeHandlerTests.cs index 37f4262..a03e7c5 100644 --- a/tests/SharpMud.Samples.Classic.Tests/ClassicCombatOutcomeHandlerTests.cs +++ b/tests/SharpMud.Samples.Classic.Tests/ClassicCombatOutcomeHandlerTests.cs @@ -34,6 +34,7 @@ public async Task OnDefeatAsync_AppliesXpLossAndHitPointHalving_AndReturnsHubRoo var defeated = new Thing { Id = ThingId.New(), Name = "Hero" }; defeated.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash", Session = session }); defeated.Behaviors.Add(new StatsBehavior { Experience = 100, MaxHitPoints = 20 }); + defeated.Behaviors.Add(new CombatantBehavior { MaxHitPoints = 20, CurrentHitPoints = 20 }); var victor = new Thing { Id = ThingId.New(), Name = "cave rat" }; @@ -49,5 +50,11 @@ public async Task OnDefeatAsync_AppliesXpLossAndHitPointHalving_AndReturnsHubRoo destination.Should().Be(hubRoom); defeated.FindBehavior()!.Experience.Should().Be(90); defeated.FindBehavior()!.CurrentHitPoints.Should().Be(10); + + // The value CombatResolver actually reads/writes during combat - + // the whole point of the ADR-0008-extraction bug fix. Asserting + // only StatsBehavior's copy (above) would still pass even if the + // CombatantBehavior halving were deleted. + defeated.FindBehavior()!.CurrentHitPoints.Should().Be(10); } } From fa056f766d4cfb872a886ab9e88415d3be43962c Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 12:12:39 -0400 Subject: [PATCH 4/6] fix: address third round of PR #18 review feedback Two more real bugs, both in shared combat scaffolding: - CombatManager tracked encounters keyed by attacker only, so a second player could start an independent encounter against a defender someone else was already fighting - both encounters would then resolve, remove, and award victory independently for the same kill (duplicate XP). Added ICombatManager.IsDefenderEngaged, checked by AttackCommand before starting a new encounter. - FleeCommand ignored Remove's return value and unconditionally added the actor to the destination room afterward. Remove can itself be vetoed (publishes a cancellable RemoveChildEvent) - proceeding anyway could add the actor to the new room without ever having left the old one, and ended the encounter regardless of whether the move actually happened. Now mirrors MoveCommand's check-then-proceed order. Also: Dockerfile was missing a COPY for the new SharpMud.Ruleset.Rpg.csproj in the early project-file layer, so `docker build` failed restoring Classic's csproj before src/ was copied wholesale - verified fixed with a real `docker build`, not just by inspection. BasicRulesetOptions' class summary is now a real XML doc comment (was a plain // comment, so it never made it into generated IntelliSense/NuGet docs). The docsite's ICombatOutcomeHandler sample now uses an explicit constructor instead of a primary constructor, matching the coding standard consumers are meant to copy. A couple of remaining stale docs mentions (ICombatant) cleaned up. Co-Authored-By: Claude Sonnet 5 --- Dockerfile | 1 + docs/combat.md | 4 +- docsite/docs/rulesets.md | 13 ++++-- .../BasicRulesetOptions.cs | 12 +++--- src/SharpMud.Ruleset.Rpg/AttackCommand.cs | 16 +++++-- src/SharpMud.Ruleset.Rpg/CombatManager.cs | 3 ++ src/SharpMud.Ruleset.Rpg/FleeCommand.cs | 13 +++++- src/SharpMud.Ruleset.Rpg/ICombatManager.cs | 10 +++++ .../Combat/CombatManagerTests.cs | 17 ++++++++ .../Commands/AttackCommandTests.cs | 26 ++++++++++++ .../Commands/FleeCommandTests.cs | 42 +++++++++++++++++++ 11 files changed, 143 insertions(+), 14 deletions(-) diff --git a/Dockerfile b/Dockerfile index 29c0c82..6ba9426 100644 --- a/Dockerfile +++ b/Dockerfile @@ -11,6 +11,7 @@ COPY src/SharpMud.Persistence/SharpMud.Persistence.csproj src/SharpMud.Persisten COPY src/SharpMud.Persistence.Sqlite/SharpMud.Persistence.Sqlite.csproj src/SharpMud.Persistence.Sqlite/ COPY src/SharpMud.Adapters.Cli/SharpMud.Adapters.Cli.csproj src/SharpMud.Adapters.Cli/ COPY src/SharpMud.Adapters.Telnet/SharpMud.Adapters.Telnet.csproj src/SharpMud.Adapters.Telnet/ +COPY src/SharpMud.Ruleset.Rpg/SharpMud.Ruleset.Rpg.csproj src/SharpMud.Ruleset.Rpg/ COPY samples/SharpMud.Samples.Classic/SharpMud.Samples.Classic.csproj samples/SharpMud.Samples.Classic/ RUN dotnet restore samples/SharpMud.Samples.Classic/SharpMud.Samples.Classic.csproj diff --git a/docs/combat.md b/docs/combat.md index 9ca6f08..c9151f7 100644 --- a/docs/combat.md +++ b/docs/combat.md @@ -236,8 +236,8 @@ damage range (e.g. 2-6) isn't 1-based dice notation and forcing it through - ~~Real linkdead/reconnect handling~~ — resolved by ADR-0004, see Disconnect Mid-Fight above. - Flee success-chance formula (exact DEX-differential-to-probability curve) - — currently a flat 60%; would also need Dexterity added to `ICombatant` or - a separate lookup. + — currently a flat 60%; would also need Dexterity added to + `CombatantBehavior` or a separate ruleset-specific stats behavior lookup. - XP-loss percentage on player death — currently a flat 10% placeholder. - Respawn HP fraction — currently `MaxHitPoints / 2` placeholder. - Loot drops on NPC death — not implemented; blocked on the item system. diff --git a/docsite/docs/rulesets.md b/docsite/docs/rulesets.md index 8e37986..425944d 100644 --- a/docsite/docs/rulesets.md +++ b/docsite/docs/rulesets.md @@ -123,8 +123,15 @@ A minimal implementation, tracking XP on your own stats behavior and always respawning at the world's starting room: ```csharp -public sealed class MyCombatOutcomeHandler(WorldContext worldContext) : ICombatOutcomeHandler +public sealed class MyCombatOutcomeHandler : ICombatOutcomeHandler { + private readonly WorldContext _worldContext; + + public MyCombatOutcomeHandler(WorldContext worldContext) + { + _worldContext = worldContext; + } + public Task OnVictoryAsync(Thing victor, Thing defeated, CancellationToken ct) { var stats = victor.FindBehavior(); @@ -136,7 +143,7 @@ public sealed class MyCombatOutcomeHandler(WorldContext worldContext) : ICombatO } public Task OnDefeatAsync(Thing defeated, Thing victor, CancellationToken ct) => - Task.FromResult(worldContext.StartingRoom); + Task.FromResult(_worldContext.StartingRoom); } ``` @@ -157,7 +164,7 @@ public Task OnDefeatAsync(Thing defeated, Thing victor, CancellationToken if (combatant is not null) combatant.CurrentHitPoints = Math.Max(1, combatant.MaxHitPoints / 2); - return Task.FromResult(worldContext.StartingRoom); + return Task.FromResult(_worldContext.StartingRoom); } ``` diff --git a/src/SharpMud.Ruleset.Basic/BasicRulesetOptions.cs b/src/SharpMud.Ruleset.Basic/BasicRulesetOptions.cs index 4321028..8595f28 100644 --- a/src/SharpMud.Ruleset.Basic/BasicRulesetOptions.cs +++ b/src/SharpMud.Ruleset.Basic/BasicRulesetOptions.cs @@ -1,10 +1,12 @@ namespace SharpMud.Ruleset.Basic; -// Plain mutable options class configured via AddSharpMudBasicRuleset(...)'s -// callback, not IOptions/appsettings.json-bound - same shape as Engine's -// GameLoopOptions. These are the tunable starting numbers for a fresh -// player character; the default default world's NPC keeps its own fixed -// stats regardless (small, deliberately simple content, not a tunable). +/// +/// Tunable starting numbers for a fresh player character, configured via +/// AddSharpMudBasicRuleset(...)'s callback - not IOptions<T>/ +/// appsettings.json-bound, same shape as Engine's +/// GameLoopOptions. The default world's NPC keeps its own fixed +/// stats regardless (small, deliberately simple content, not a tunable). +/// public sealed class BasicRulesetOptions { /// A fresh character's starting (and max) hit points. Must be at least 1 - see . diff --git a/src/SharpMud.Ruleset.Rpg/AttackCommand.cs b/src/SharpMud.Ruleset.Rpg/AttackCommand.cs index 7533b5b..c4e7558 100644 --- a/src/SharpMud.Ruleset.Rpg/AttackCommand.cs +++ b/src/SharpMud.Ruleset.Rpg/AttackCommand.cs @@ -27,9 +27,9 @@ public AttackCommand(ICombatManager combatManager) /// /// Guards (not already fighting, actor can fight, a matching NPC target - /// exists in the room), then starts the encounter via . Resolution happens on the - /// next game tick, not synchronously here. + /// exists in the room and isn't already engaged by someone else), then + /// starts the encounter via . + /// Resolution happens on the next game tick, not synchronously here. /// public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) { @@ -65,6 +65,16 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) return; } + // Without this, a second attacker could start a second, independent + // encounter against a target someone else is already fighting - + // both encounters would then resolve/remove/award victory for the + // same kill (see ICombatManager.IsDefenderEngaged). + if (_combatManager.IsDefenderEngaged(target.Id)) + { + await ctx.Session.WriteLineAsync($"Someone else is already fighting {target.Name}!", ct); + return; + } + _combatManager.StartEncounter(ctx.Actor, target); await ctx.Session.WriteLineAsync($"You attack {target.Name}!", ct); } diff --git a/src/SharpMud.Ruleset.Rpg/CombatManager.cs b/src/SharpMud.Ruleset.Rpg/CombatManager.cs index b7497f6..79d166b 100644 --- a/src/SharpMud.Ruleset.Rpg/CombatManager.cs +++ b/src/SharpMud.Ruleset.Rpg/CombatManager.cs @@ -42,6 +42,9 @@ public CombatManager(ICombatResolver resolver, ICombatOutcomeHandler outcomeHand /// public bool IsInCombat(ThingId thingId) => _encounters.ContainsKey(thingId); + /// + public bool IsDefenderEngaged(ThingId defenderId) => _encounters.Values.Any(e => e.Defender.Id == defenderId); + /// public void StartEncounter(Thing attacker, Thing defender) => _encounters[attacker.Id] = new CombatEncounter { Attacker = attacker, Defender = defender }; diff --git a/src/SharpMud.Ruleset.Rpg/FleeCommand.cs b/src/SharpMud.Ruleset.Rpg/FleeCommand.cs index 2fc4e7e..f73257b 100644 --- a/src/SharpMud.Ruleset.Rpg/FleeCommand.cs +++ b/src/SharpMud.Ruleset.Rpg/FleeCommand.cs @@ -77,13 +77,24 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) var destination = exit.Destination; + // Remove can itself publish a cancellable RemoveChildEvent and + // return false (e.g. a future "can't leave this room" behavior) - + // same check MoveCommand does, and for the same reason: proceeding + // to Add into destination after a failed Remove would leave the + // actor added to the new room without ever having left the old + // one. EndEncounter/messaging only happen once the move is real. + if (!ctx.CurrentRoom.Remove(ctx.Actor)) + { + await ctx.Session.WriteLineAsync("You can't escape that way!", ct); + return; + } + _combatManager.EndEncounter(ctx.Actor.Id); await ctx.Session.WriteLineAsync($"You flee {exit.Direction.ToDisplayString()}!", ct); await RoomBroadcast.ToOccupantsAsync( ctx.CurrentRoom, ctx.Actor, $"{ctx.Actor.Name} flees {exit.Direction.ToDisplayString()}.", ct); - ctx.CurrentRoom.Remove(ctx.Actor); destination.Add(ctx.Actor); await LookCommand.SendRoomDescriptionAsync(ctx.Actor, destination, ct); diff --git a/src/SharpMud.Ruleset.Rpg/ICombatManager.cs b/src/SharpMud.Ruleset.Rpg/ICombatManager.cs index e9caf38..9cd3652 100644 --- a/src/SharpMud.Ruleset.Rpg/ICombatManager.cs +++ b/src/SharpMud.Ruleset.Rpg/ICombatManager.cs @@ -26,6 +26,16 @@ public interface ICombatManager /// Whether the given Thing is currently the attacker in an active encounter. bool IsInCombat(ThingId thingId); + /// + /// Whether the given Thing is currently the defender in any active + /// encounter - since encounters are keyed by attacker only, this is the + /// check that stops a second attacker from starting a second, redundant + /// encounter against a target someone else is already fighting (which + /// would otherwise let both encounters independently resolve, remove, + /// and award victory for the same kill). + /// + bool IsDefenderEngaged(ThingId defenderId); + /// Starts (or replaces) the encounter keyed by . void StartEncounter(Thing attacker, Thing defender); diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs index 6951ab7..05c7750 100644 --- a/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs +++ b/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs @@ -7,6 +7,23 @@ namespace SharpMud.Ruleset.Rpg.Tests.Combat; public sealed class CombatManagerTests { + [Fact] + public void IsDefenderEngaged_ReturnsTrue_WhenAnotherAttackerAlreadyTargetsTheSameDefender() + { + var resolver = Substitute.For(); + var outcomeHandler = Substitute.For(); + + var attackerOne = new Thing { Id = ThingId.New(), Name = "Hero One" }; + var attackerTwo = new Thing { Id = ThingId.New(), Name = "Hero Two" }; + var npc = new Thing { Id = ThingId.New(), Name = "cave rat" }; + + var sut = new CombatManager(resolver, outcomeHandler); + sut.StartEncounter(attackerOne, npc); + + sut.IsDefenderEngaged(npc.Id).Should().BeTrue(); + sut.IsDefenderEngaged(attackerTwo.Id).Should().BeFalse(); + } + [Fact] public async Task OnTickAsync_NotifiesOutcomeHandlerAndEndsEncounter_WhenPlayerDefeatsNpc() { diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs index ebaf69a..2178c3e 100644 --- a/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs +++ b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs @@ -76,6 +76,32 @@ public async Task ExecuteAsync_SendsCannotFightMessage_WhenActorHasNoCombatantBe await session.Received(1).WriteLineAsync("You have no way to fight.", Arg.Any()); } + [Fact] + public async Task ExecuteAsync_SendsAlreadyEngagedMessage_WhenTargetIsAlreadyBeingFoughtByAnotherAttacker() + { + var combatManager = Substitute.For(); + var session = Substitute.For(); + combatManager.IsDefenderEngaged(Arg.Any()).Returns(true); + + var room = new Thing { Id = ThingId.New(), Name = "Room" }; + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + player.Behaviors.Add(new CombatantBehavior()); + room.Add(player); + + var npc = new Thing { Id = ThingId.New(), Name = "cave rat" }; + npc.Behaviors.Add(new NpcBehavior()); + npc.Behaviors.Add(new CombatantBehavior()); + room.Add(npc); + + var sut = new AttackCommand(combatManager); + var ctx = new CommandContext(player, room, ["cave", "rat"], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + combatManager.DidNotReceiveWithAnyArgs().StartEncounter(default!, default!); + await session.Received(1).WriteLineAsync("Someone else is already fighting cave rat!", Arg.Any()); + } + [Fact] public async Task ExecuteAsync_SendsAlreadyFightingMessage_WhenActorAlreadyInCombat() { diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs index e99afa6..ef76af0 100644 --- a/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs +++ b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs @@ -74,6 +74,48 @@ public async Task ExecuteAsync_DoesNotMoveOrEndEncounter_WhenChosenExitIsLocked( await session.Received(1).WriteLineAsync("The door is locked.", Arg.Any()); } + [Fact] + public async Task ExecuteAsync_DoesNotMoveOrEndEncounter_WhenLeavingTheRoomIsVetoed() + { + var combatManager = Substitute.For(); + var dice = Substitute.For(); + var random = Substitute.For(); + var session = Substitute.For(); + var encounter = new CombatEncounter { Attacker = new Thing { Id = ThingId.New(), Name = "x" }, Defender = new Thing { Id = ThingId.New(), Name = "y" } }; + combatManager.TryGetEncounter(Arg.Any(), out Arg.Any()) + .Returns(x => { x[1] = encounter; return true; }); + dice.Roll(1, 100).Returns(1); + random.Next(0, 0).Returns(0); + + var origin = new Thing { Id = ThingId.New(), Name = "Origin" }; + var destination = new Thing { Id = ThingId.New(), Name = "Destination" }; + var exit = new Thing { Id = ThingId.New(), Name = "north" }; + exit.Behaviors.Add(new ExitBehavior { Direction = Direction.North, Destination = destination }); + origin.Add(exit); + + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + player.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash", Session = session }); + origin.Add(player); + + // Registered after Add so it only vetoes the flee's own Remove, not + // Thing.Add's own AddChildEvent publish on the same origin.Events. + origin.Events.SubscribeRequest((_, evt) => + { + if (evt is RemoveChildEvent) + evt.Cancel("You are rooted to the spot."); + }); + + var sut = new FleeCommand(combatManager, dice, random); + var ctx = new CommandContext(player, origin, [], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + combatManager.DidNotReceiveWithAnyArgs().EndEncounter(default!); + player.Parent.Should().Be(origin); + destination.Children.Should().NotContain(player); + await session.Received(1).WriteLineAsync("You can't escape that way!", Arg.Any()); + } + [Fact] public async Task ExecuteAsync_SendsFailureMessage_WhenRollFails() { From ddc9a6845d5c012d3805bb7417ea75579add2e7f Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 12:36:26 -0400 Subject: [PATCH 5/6] fix: address fourth round of PR #18 review feedback (concurrency + flee rollback) CombatManager's encounter tracking assumed single-threaded access ("world mutation happens on the single game-loop thread"), but that's only true for tick-vs-tick ordering - AttackCommand/FleeCommand run inside whichever session's independently-scheduled task happens to be executing a command (TelnetTransportBackgroundService runs one task per connection), so _encounters is genuinely accessed from multiple threads. The previous round's IsDefenderEngaged-then-StartEncounter guard was a real TOCTOU race: two players targeting the same NPC at nearly the same time could both observe "not engaged" before either inserted, recreating the duplicate-XP bug. Replaced StartEncounter with an atomic TryStartEncounter (check + insert under one System.Threading.Lock critical section) and made every other _encounters access go through the same lock. Added a real concurrent test (32 attackers racing the same defender via Task.WhenAll/Task.Run, asserting exactly one wins) alongside the sequential case, since the sequential case alone wouldn't have caught the original race. Also: FleeCommand checked Remove's cancelable result but not destination.Add's - a vetoed Add (e.g. a future "room is full" behavior) would leave the actor already removed from the encounter and their old room, with success already announced, but never actually added anywhere. Now rolls back into the original room and reports failure instead. docs/combat.md's ICombatManager snippet/sequence updated for IsDefenderEngaged, TryStartEncounter, the "someone else is already fighting" message, and switched off the stale primary-constructor sketch. Co-Authored-By: Claude Sonnet 5 --- docs/combat.md | 35 +++++--- src/SharpMud.Ruleset.Rpg/AttackCommand.cs | 22 +++--- src/SharpMud.Ruleset.Rpg/CombatManager.cs | 79 ++++++++++++++----- src/SharpMud.Ruleset.Rpg/FleeCommand.cs | 16 +++- src/SharpMud.Ruleset.Rpg/ICombatManager.cs | 26 ++++-- .../Combat/CombatManagerTests.cs | 51 ++++++++++-- .../Commands/AttackCommandTests.cs | 18 +++-- .../Commands/FleeCommandTests.cs | 40 ++++++++++ 8 files changed, 228 insertions(+), 59 deletions(-) diff --git a/docs/combat.md b/docs/combat.md index c9151f7..2117cf9 100644 --- a/docs/combat.md +++ b/docs/combat.md @@ -50,18 +50,29 @@ public sealed class CombatEncounter public interface ICombatManager { bool IsInCombat(ThingId thingId); - void StartEncounter(Thing attacker, Thing defender); + bool IsDefenderEngaged(ThingId defenderId); + bool TryStartEncounter(Thing attacker, Thing defender); void EndEncounter(ThingId thingId); bool TryGetEncounter(ThingId thingId, [MaybeNullWhen(false)] out CombatEncounter encounter); } -public sealed class CombatManager(ICombatResolver resolver, ICombatOutcomeHandler outcomeHandler) - : ICombatManager, ITickable +public sealed class CombatManager : ICombatManager, ITickable { + private readonly ICombatResolver _resolver; + private readonly ICombatOutcomeHandler _outcomeHandler; + + public CombatManager(ICombatResolver resolver, ICombatOutcomeHandler outcomeHandler) + { + _resolver = resolver; + _outcomeHandler = outcomeHandler; + } + public Task OnTickAsync(TickContext ctx, CancellationToken ct) { /* see below */ } } ``` +`_encounters` isn't only touched from the tick loop - `TryStartEncounter`/`EndEncounter`/etc. are also called from whichever session's command-execution task happens to be running `AttackCommand`/`FleeCommand` at that moment, and each connection runs independently (see `TelnetTransportBackgroundService`). `TryStartEncounter` checks "is this attacker already fighting" and "is this defender already engaged by someone else" and inserts the new encounter as one atomic operation (a `System.Threading.Lock` critical section) - not two separate steps - so two players targeting the same NPC at nearly the same time can't both succeed. `IsDefenderEngaged` alone is a point-in-time status query only, useful for a custom command that wants to display "who's fighting what," but not by itself race-free the way `TryStartEncounter` is. + No `hubRoomId`/`IWorld` constructor parameter — `CombatManager` has no respawn-destination or world-lookup concept of its own. `ICombatOutcomeHandler` (implemented per-ruleset) owns both the XP-award/death-penalty side effects @@ -117,13 +128,17 @@ public sealed record CombatRoundResult(bool Hit, int Damage, bool DefenderDefeat 1. Player types `"kill cave rat"` → `AttackCommand` matches the target among the current room's children carrying both `NpcBehavior` and `CombatantBehavior` (`ObjectMatcher.FindMatch`, case-insensitive), calls - `ICombatManager.StartEncounter`, sends `"You attack cave rat!"` - immediately (engagement is instant; resolution is tick-gated). If the - player is already in combat, the command instead sends `"You are already - fighting!"`; if the actor itself has no `CombatantBehavior` (a consumer's - own `IPlayerFactory` forgot to attach it), it sends `"You have no way to - fight."` instead of starting an encounter that would crash on the next - tick. + `ICombatManager.TryStartEncounter`, sends `"You attack cave rat!"` + immediately if it returns `true` (engagement is instant; resolution is + tick-gated). If the player is already in combat, the command sends `"You + are already fighting!"` without calling `TryStartEncounter` at all; if the + actor itself has no `CombatantBehavior` (a consumer's own `IPlayerFactory` + forgot to attach it), it sends `"You have no way to fight."` instead of + starting an encounter that would crash on the next tick. If + `TryStartEncounter` itself returns `false` - the target is already + engaged by a different attacker, checked and inserted atomically so two + players targeting the same NPC at once can't both succeed - it sends + `"Someone else is already fighting cave rat!"`. 2. On the next global tick, `IGameLoop` calls `CombatManager.OnTickAsync`, which iterates every active encounter. 3. `ICombatResolver.ResolveRound(attacker, defender)` computes the player's diff --git a/src/SharpMud.Ruleset.Rpg/AttackCommand.cs b/src/SharpMud.Ruleset.Rpg/AttackCommand.cs index c4e7558..fda109d 100644 --- a/src/SharpMud.Ruleset.Rpg/AttackCommand.cs +++ b/src/SharpMud.Ruleset.Rpg/AttackCommand.cs @@ -27,9 +27,12 @@ public AttackCommand(ICombatManager combatManager) /// /// Guards (not already fighting, actor can fight, a matching NPC target - /// exists in the room and isn't already engaged by someone else), then - /// starts the encounter via . - /// Resolution happens on the next game tick, not synchronously here. + /// exists in the room), then atomically starts the encounter via - which is also the guard + /// against a target someone else is already fighting, checked and + /// inserted as one operation so two concurrent attackers targeting the + /// same NPC can't both succeed. Resolution happens on the next game + /// tick, not synchronously here. /// public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) { @@ -65,17 +68,18 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) return; } - // Without this, a second attacker could start a second, independent - // encounter against a target someone else is already fighting - - // both encounters would then resolve/remove/award victory for the - // same kill (see ICombatManager.IsDefenderEngaged). - if (_combatManager.IsDefenderEngaged(target.Id)) + // TryStartEncounter is the actual guard against a target someone + // else is already fighting - checked and inserted atomically, so + // two concurrent attackers targeting the same NPC (two different + // players' session-loop tasks racing each other) can't both + // succeed. The IsInCombat check above already ruled out "this same + // actor is already fighting" as the reason for a false return here. + if (!_combatManager.TryStartEncounter(ctx.Actor, target)) { await ctx.Session.WriteLineAsync($"Someone else is already fighting {target.Name}!", ct); return; } - _combatManager.StartEncounter(ctx.Actor, target); await ctx.Session.WriteLineAsync($"You attack {target.Name}!", ct); } } diff --git a/src/SharpMud.Ruleset.Rpg/CombatManager.cs b/src/SharpMud.Ruleset.Rpg/CombatManager.cs index 79d166b..773d7d5 100644 --- a/src/SharpMud.Ruleset.Rpg/CombatManager.cs +++ b/src/SharpMud.Ruleset.Rpg/CombatManager.cs @@ -8,9 +8,15 @@ namespace SharpMud.Ruleset.Rpg; // Registered once with IGameLoop and resolves every active encounter each -// tick - simpler Host wiring than one ITickable per encounter, and all -// world-state mutation happens on the single game-loop "thread" (sequential -// awaits in GameLoop.RunAsync), so there's no concurrent-mutation risk. +// tick, but "each tick" is not the only time _encounters is touched - +// AttackCommand/FleeCommand call into this manager from whichever session's +// SessionLoop.RunAsync happens to be running a command at that moment, and +// each connection runs its own independently-scheduled task (see +// TelnetTransportBackgroundService.HandleConnectionAsync). So _encounters is +// genuinely accessed from multiple threads, not just serialized through the +// tick loop - every access goes through _lock (System.Threading.Lock, per +// this repo's own concurrency guidance for a real, unavoidable critical +// section) rather than assuming single-threaded access. // // Combat-outcome side effects (XP awards, death penalties, respawn // destination) are delegated to ICombatOutcomeHandler rather than touching a @@ -30,6 +36,7 @@ public sealed class CombatManager : ICombatManager, ITickable { private readonly ICombatResolver _resolver; private readonly ICombatOutcomeHandler _outcomeHandler; + private readonly Lock _lock = new(); private readonly Dictionary _encounters = []; /// Creates the manager against a resolver and a ruleset's outcome handler. @@ -40,32 +47,68 @@ public CombatManager(ICombatResolver resolver, ICombatOutcomeHandler outcomeHand } /// - public bool IsInCombat(ThingId thingId) => _encounters.ContainsKey(thingId); + public bool IsInCombat(ThingId thingId) + { + lock (_lock) + return _encounters.ContainsKey(thingId); + } /// - public bool IsDefenderEngaged(ThingId defenderId) => _encounters.Values.Any(e => e.Defender.Id == defenderId); + public bool IsDefenderEngaged(ThingId defenderId) + { + lock (_lock) + return _encounters.Values.Any(e => e.Defender.Id == defenderId); + } /// - public void StartEncounter(Thing attacker, Thing defender) => - _encounters[attacker.Id] = new CombatEncounter { Attacker = attacker, Defender = defender }; + public bool TryStartEncounter(Thing attacker, Thing defender) + { + lock (_lock) + { + if (_encounters.ContainsKey(attacker.Id)) + return false; + + if (_encounters.Values.Any(e => e.Defender.Id == defender.Id)) + return false; + + _encounters[attacker.Id] = new CombatEncounter { Attacker = attacker, Defender = defender }; + return true; + } + } /// - public void EndEncounter(ThingId thingId) => _encounters.Remove(thingId); + public void EndEncounter(ThingId thingId) + { + lock (_lock) + _encounters.Remove(thingId); + } /// - public bool TryGetEncounter(ThingId thingId, [MaybeNullWhen(false)] out CombatEncounter encounter) => - _encounters.TryGetValue(thingId, out encounter); + public bool TryGetEncounter(ThingId thingId, [MaybeNullWhen(false)] out CombatEncounter encounter) + { + lock (_lock) + return _encounters.TryGetValue(thingId, out encounter); + } /// Resolves one round for every active encounter - see the class remarks above for the full sequence. public async Task OnTickAsync(TickContext ctx, CancellationToken ct) { - foreach (var thingId in _encounters.Keys.ToArray()) + ThingId[] activeEncounterIds; + lock (_lock) + activeEncounterIds = [.. _encounters.Keys]; + + foreach (var thingId in activeEncounterIds) { - if (!_encounters.TryGetValue(thingId, out var encounter)) - continue; + CombatEncounter? encounter; + lock (_lock) + { + if (!_encounters.TryGetValue(thingId, out encounter)) + continue; + } - // Only AttackCommand calls StartEncounter, and only with a player - // Thing as the attacker - encounter.Attacker always has a PlayerBehavior. + // Only AttackCommand calls TryStartEncounter, and only with a + // player Thing as the attacker - encounter.Attacker always has a + // PlayerBehavior. var attackerBehavior = encounter.Attacker.FindBehavior()!; if (attackerBehavior.ConnectionState == ConnectionState.Linkdead) { @@ -76,7 +119,7 @@ public async Task OnTickAsync(TickContext ctx, CancellationToken ct) // same grace window LoginFlow/LinkdeadSweeper use has elapsed. // Linkdead always sets LinkdeadSinceUtc (PlayerBehavior.EnterLinkdead). if (ctx.Timestamp - attackerBehavior.LinkdeadSinceUtc!.Value >= ReconnectPolicy.GraceWindow) - _encounters.Remove(thingId); + EndEncounter(thingId); continue; } @@ -117,7 +160,7 @@ private async Task HandleDefenderDefeatedAsync(CombatEncounter encounter, ISessi await _outcomeHandler.OnVictoryAsync(encounter.Attacker, encounter.Defender, ct); encounter.Defender.Parent?.Remove(encounter.Defender); - _encounters.Remove(encounter.Attacker.Id); + EndEncounter(encounter.Attacker.Id); } private async Task HandleAttackerDefeatedAsync(CombatEncounter encounter, ISession session, CancellationToken ct) @@ -149,7 +192,7 @@ private async Task HandleAttackerDefeatedAsync(CombatEncounter encounter, ISessi var destination = await _outcomeHandler.OnDefeatAsync(attacker, encounter.Defender, ct); - _encounters.Remove(attacker.Id); + EndEncounter(attacker.Id); attacker.Parent?.Remove(attacker); destination.Add(attacker); diff --git a/src/SharpMud.Ruleset.Rpg/FleeCommand.cs b/src/SharpMud.Ruleset.Rpg/FleeCommand.cs index f73257b..9c33d02 100644 --- a/src/SharpMud.Ruleset.Rpg/FleeCommand.cs +++ b/src/SharpMud.Ruleset.Rpg/FleeCommand.cs @@ -89,14 +89,26 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) return; } + // Add can itself publish a cancellable AddChildEvent and return + // false (e.g. a future "room is full" behavior). Unlike Remove + // above, there's no early-return option here - the actor is already + // detached from ctx.CurrentRoom - so a failed Add is rolled back by + // re-adding to the original room, keeping the encounter (and the + // actor) exactly where they were rather than leaving them parentless + // after an announced-but-not-actually-completed flee. + if (!destination.Add(ctx.Actor)) + { + ctx.CurrentRoom.Add(ctx.Actor); + await ctx.Session.WriteLineAsync("You can't escape that way!", ct); + return; + } + _combatManager.EndEncounter(ctx.Actor.Id); await ctx.Session.WriteLineAsync($"You flee {exit.Direction.ToDisplayString()}!", ct); await RoomBroadcast.ToOccupantsAsync( ctx.CurrentRoom, ctx.Actor, $"{ctx.Actor.Name} flees {exit.Direction.ToDisplayString()}.", ct); - destination.Add(ctx.Actor); - await LookCommand.SendRoomDescriptionAsync(ctx.Actor, destination, ct); } } diff --git a/src/SharpMud.Ruleset.Rpg/ICombatManager.cs b/src/SharpMud.Ruleset.Rpg/ICombatManager.cs index 9cd3652..4f88be1 100644 --- a/src/SharpMud.Ruleset.Rpg/ICombatManager.cs +++ b/src/SharpMud.Ruleset.Rpg/ICombatManager.cs @@ -28,16 +28,28 @@ public interface ICombatManager /// /// Whether the given Thing is currently the defender in any active - /// encounter - since encounters are keyed by attacker only, this is the - /// check that stops a second attacker from starting a second, redundant - /// encounter against a target someone else is already fighting (which - /// would otherwise let both encounters independently resolve, remove, - /// and award victory for the same kill). + /// encounter - since encounters are keyed by attacker only, a second + /// attacker targeting an already-engaged defender would otherwise let + /// both encounters independently resolve, remove, and award victory for + /// the same kill. This is a point-in-time status query only - the actual + /// enforcement against that race is , + /// which checks and inserts atomically; don't call this separately and + /// then call expecting the combination + /// to be race-free, since another attacker can start an encounter + /// between the two calls. /// bool IsDefenderEngaged(ThingId defenderId); - /// Starts (or replaces) the encounter keyed by . - void StartEncounter(Thing attacker, Thing defender); + /// + /// Atomically starts the encounter keyed by , + /// unless is already fighting or is already engaged by a different attacker - the + /// check and the insert happen under the same lock, so two concurrent + /// callers (e.g. two players' independent session-loop tasks targeting + /// the same NPC at nearly the same time) can't both succeed against the + /// same defender. Returns whether the encounter was actually started. + /// + bool TryStartEncounter(Thing attacker, Thing defender); /// Ends the encounter keyed by the given Thing, if one exists. void EndEncounter(ThingId thingId); diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs index 05c7750..1f1308c 100644 --- a/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs +++ b/tests/SharpMud.Ruleset.Rpg.Tests/Combat/CombatManagerTests.cs @@ -18,12 +18,51 @@ public void IsDefenderEngaged_ReturnsTrue_WhenAnotherAttackerAlreadyTargetsTheSa var npc = new Thing { Id = ThingId.New(), Name = "cave rat" }; var sut = new CombatManager(resolver, outcomeHandler); - sut.StartEncounter(attackerOne, npc); + sut.TryStartEncounter(attackerOne, npc); sut.IsDefenderEngaged(npc.Id).Should().BeTrue(); sut.IsDefenderEngaged(attackerTwo.Id).Should().BeFalse(); } + [Fact] + public void TryStartEncounter_ReturnsFalse_WhenDefenderIsAlreadyEngagedByAnotherAttacker() + { + var resolver = Substitute.For(); + var outcomeHandler = Substitute.For(); + + var attackerOne = new Thing { Id = ThingId.New(), Name = "Hero One" }; + var attackerTwo = new Thing { Id = ThingId.New(), Name = "Hero Two" }; + var npc = new Thing { Id = ThingId.New(), Name = "cave rat" }; + + var sut = new CombatManager(resolver, outcomeHandler); + + sut.TryStartEncounter(attackerOne, npc).Should().BeTrue(); + sut.TryStartEncounter(attackerTwo, npc).Should().BeFalse(); + } + + // Regression coverage for the actual concurrency bug (two players' + // independent session-loop tasks both targeting the same NPC at nearly + // the same time) - not just the sequential "second call fails" case + // above, which wouldn't have caught the original TOCTOU race between a + // separate IsDefenderEngaged check and StartEncounter. + [Fact] + public async Task TryStartEncounter_AllowsExactlyOneCaller_WhenManyAttackersRaceTheSameDefender() + { + var resolver = Substitute.For(); + var outcomeHandler = Substitute.For(); + var npc = new Thing { Id = ThingId.New(), Name = "cave rat" }; + var sut = new CombatManager(resolver, outcomeHandler); + + var attackers = Enumerable.Range(0, 32) + .Select(i => new Thing { Id = ThingId.New(), Name = $"Hero {i}" }) + .ToArray(); + + var results = await Task.WhenAll(attackers.Select(attacker => + Task.Run(() => sut.TryStartEncounter(attacker, npc)))); + + results.Count(succeeded => succeeded).Should().Be(1); + } + [Fact] public async Task OnTickAsync_NotifiesOutcomeHandlerAndEndsEncounter_WhenPlayerDefeatsNpc() { @@ -44,7 +83,7 @@ public async Task OnTickAsync_NotifiesOutcomeHandlerAndEndsEncounter_WhenPlayerD resolver.ResolveRound(player, npc).Returns(new CombatRoundResult(true, 6, true)); var sut = new CombatManager(resolver, outcomeHandler); - sut.StartEncounter(player, npc); + sut.TryStartEncounter(player, npc); await sut.OnTickAsync(new TickContext(DateTimeOffset.UtcNow), TestContext.Current.CancellationToken); @@ -78,7 +117,7 @@ public async Task OnTickAsync_ResetsCombatantHitPointsAndRespawnsAtHandlerDestin outcomeHandler.OnDefeatAsync(player, npc, TestContext.Current.CancellationToken).Returns(hubRoom); var sut = new CombatManager(resolver, outcomeHandler); - sut.StartEncounter(player, npc); + sut.TryStartEncounter(player, npc); await sut.OnTickAsync(new TickContext(DateTimeOffset.UtcNow), TestContext.Current.CancellationToken); @@ -120,7 +159,7 @@ public async Task OnTickAsync_SendsDefeatMessageBeforeInvokingOutcomeHandler_Whe .AndDoes(_ => callOrder.Add("outcome-handler")); var sut = new CombatManager(resolver, outcomeHandler); - sut.StartEncounter(player, npc); + sut.TryStartEncounter(player, npc); await sut.OnTickAsync(new TickContext(DateTimeOffset.UtcNow), TestContext.Current.CancellationToken); @@ -150,7 +189,7 @@ public async Task OnTickAsync_FreezesEncounter_WhenAttackerLinkdeadWithinGraceWi room.Add(npc); var sut = new CombatManager(resolver, outcomeHandler); - sut.StartEncounter(player, npc); + sut.TryStartEncounter(player, npc); await sut.OnTickAsync(new TickContext(DateTimeOffset.UtcNow), TestContext.Current.CancellationToken); @@ -179,7 +218,7 @@ public async Task OnTickAsync_AbandonsEncounter_WhenAttackerLinkdeadPastGraceWin room.Add(npc); var sut = new CombatManager(resolver, outcomeHandler); - sut.StartEncounter(player, npc); + sut.TryStartEncounter(player, npc); await sut.OnTickAsync(new TickContext(DateTimeOffset.UtcNow), TestContext.Current.CancellationToken); diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs index 2178c3e..49a46c3 100644 --- a/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs +++ b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/AttackCommandTests.cs @@ -12,6 +12,7 @@ public async Task ExecuteAsync_StartsEncounter_WhenTargetExists() { var combatManager = Substitute.For(); var session = Substitute.For(); + combatManager.TryStartEncounter(Arg.Any(), Arg.Any()).Returns(true); var room = new Thing { Id = ThingId.New(), Name = "Room" }; var player = new Thing { Id = ThingId.New(), Name = "Hero" }; @@ -28,7 +29,7 @@ public async Task ExecuteAsync_StartsEncounter_WhenTargetExists() await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); - combatManager.Received(1).StartEncounter(player, npc); + combatManager.Received(1).TryStartEncounter(player, npc); await session.Received(1).WriteLineAsync("You attack cave rat!", Arg.Any()); } @@ -48,7 +49,7 @@ public async Task ExecuteAsync_SendsNotHereMessage_WhenNoMatchingCombatantInRoom await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); - combatManager.DidNotReceiveWithAnyArgs().StartEncounter(default!, default!); + combatManager.DidNotReceiveWithAnyArgs().TryStartEncounter(default!, default!); await session.Received(1).WriteLineAsync("You don't see that here.", Arg.Any()); } @@ -72,16 +73,16 @@ public async Task ExecuteAsync_SendsCannotFightMessage_WhenActorHasNoCombatantBe await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); - combatManager.DidNotReceiveWithAnyArgs().StartEncounter(default!, default!); + combatManager.DidNotReceiveWithAnyArgs().TryStartEncounter(default!, default!); await session.Received(1).WriteLineAsync("You have no way to fight.", Arg.Any()); } [Fact] - public async Task ExecuteAsync_SendsAlreadyEngagedMessage_WhenTargetIsAlreadyBeingFoughtByAnotherAttacker() + public async Task ExecuteAsync_SendsAlreadyEngagedMessage_WhenTryStartEncounterFails() { var combatManager = Substitute.For(); var session = Substitute.For(); - combatManager.IsDefenderEngaged(Arg.Any()).Returns(true); + combatManager.TryStartEncounter(Arg.Any(), Arg.Any()).Returns(false); var room = new Thing { Id = ThingId.New(), Name = "Room" }; var player = new Thing { Id = ThingId.New(), Name = "Hero" }; @@ -98,7 +99,10 @@ public async Task ExecuteAsync_SendsAlreadyEngagedMessage_WhenTargetIsAlreadyBei await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); - combatManager.DidNotReceiveWithAnyArgs().StartEncounter(default!, default!); + // TryStartEncounter is called (and is the actual source of truth for + // this failure) rather than skipped - distinguishes this from the + // guard-clause tests above, which never even attempt to start. + combatManager.Received(1).TryStartEncounter(player, npc); await session.Received(1).WriteLineAsync("Someone else is already fighting cave rat!", Arg.Any()); } @@ -118,7 +122,7 @@ public async Task ExecuteAsync_SendsAlreadyFightingMessage_WhenActorAlreadyInCom await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); - combatManager.DidNotReceiveWithAnyArgs().StartEncounter(default!, default!); + combatManager.DidNotReceiveWithAnyArgs().TryStartEncounter(default!, default!); await session.Received(1).WriteLineAsync("You are already fighting!", Arg.Any()); } } diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs index ef76af0..b28ea24 100644 --- a/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs +++ b/tests/SharpMud.Ruleset.Rpg.Tests/Commands/FleeCommandTests.cs @@ -116,6 +116,46 @@ public async Task ExecuteAsync_DoesNotMoveOrEndEncounter_WhenLeavingTheRoomIsVet await session.Received(1).WriteLineAsync("You can't escape that way!", Arg.Any()); } + [Fact] + public async Task ExecuteAsync_RollsBackAndDoesNotEndEncounter_WhenEnteringDestinationIsVetoed() + { + var combatManager = Substitute.For(); + var dice = Substitute.For(); + var random = Substitute.For(); + var session = Substitute.For(); + var encounter = new CombatEncounter { Attacker = new Thing { Id = ThingId.New(), Name = "x" }, Defender = new Thing { Id = ThingId.New(), Name = "y" } }; + combatManager.TryGetEncounter(Arg.Any(), out Arg.Any()) + .Returns(x => { x[1] = encounter; return true; }); + dice.Roll(1, 100).Returns(1); + random.Next(0, 0).Returns(0); + + var origin = new Thing { Id = ThingId.New(), Name = "Origin" }; + var destination = new Thing { Id = ThingId.New(), Name = "Destination" }; + destination.Events.SubscribeRequest((_, evt) => + { + if (evt is AddChildEvent) + evt.Cancel("The room is full."); + }); + var exit = new Thing { Id = ThingId.New(), Name = "north" }; + exit.Behaviors.Add(new ExitBehavior { Direction = Direction.North, Destination = destination }); + origin.Add(exit); + + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + player.Behaviors.Add(new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash", Session = session }); + origin.Add(player); + + var sut = new FleeCommand(combatManager, dice, random); + var ctx = new CommandContext(player, origin, [], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + combatManager.DidNotReceiveWithAnyArgs().EndEncounter(default!); + player.Parent.Should().Be(origin, "a vetoed Add must roll back the earlier Remove, not leave the actor parentless"); + origin.Children.Should().Contain(player); + destination.Children.Should().NotContain(player); + await session.Received(1).WriteLineAsync("You can't escape that way!", Arg.Any()); + } + [Fact] public async Task ExecuteAsync_SendsFailureMessage_WhenRollFails() { From 50289a0a127984e817b70ed58ac5f4f90ef605ab Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 12:46:35 -0400 Subject: [PATCH 6/6] fix: close narrow stale-encounter race in CombatManager.OnTickAsync The per-tick loop fetched an encounter under _lock, then resolved a round against that captured object outside the lock (necessarily, since holding a lock across an await isn't possible). A concurrent FleeCommand (ending this attacker's encounter) immediately followed by AttackCommand (starting a new one against a different defender) could land in that window - the tick would then resolve a round against the stale, already-replaced encounter, and could wrongly EndEncounter the new one afterward based on that stale round's outcome. Added a second lock-guarded re-check immediately before resolving each round: skip this tick for that id unless _encounters[thingId] is still reference-equal to what was fetched. Narrows the actual race window to nothing meaningful (no yield point between the check and the synchronous resolve call that follows it). Co-Authored-By: Claude Sonnet 5 --- src/SharpMud.Ruleset.Rpg/CombatManager.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/SharpMud.Ruleset.Rpg/CombatManager.cs b/src/SharpMud.Ruleset.Rpg/CombatManager.cs index 773d7d5..e728b31 100644 --- a/src/SharpMud.Ruleset.Rpg/CombatManager.cs +++ b/src/SharpMud.Ruleset.Rpg/CombatManager.cs @@ -124,6 +124,22 @@ public async Task OnTickAsync(TickContext ctx, CancellationToken ct) continue; } + // Re-verify this is still the live encounter for thingId, + // immediately before resolving a round against it. Nothing + // above this point holds _lock across an await, so a concurrent + // FleeCommand (EndEncounter) + AttackCommand (TryStartEncounter, + // against a *different* defender) pair could otherwise replace + // _encounters[thingId] between the fetch above and here - this + // tick would then resolve a round against the stale, already- + // replaced encounter and could wrongly EndEncounter the new one + // afterward. Skip and pick up the real current encounter next + // tick instead. + lock (_lock) + { + if (!_encounters.TryGetValue(thingId, out var currentEncounter) || !ReferenceEquals(currentEncounter, encounter)) + continue; + } + // Not Linkdead (checked above), so Session is the live, connected session. var session = attackerBehavior.Session!;