From d665f880671fa46a1bb39cd09325b71a734aba8a Mon Sep 17 00:00:00 2001 From: gamer147 Date: Mon, 1 Jun 2026 20:00:52 -0400 Subject: [PATCH] refactor(battle-node): unify IMatchingBridge.RegisterBattle signature Single RegisterBattle(p1, p2?, type) with contract validation throws on invalid combinations (Pvp requires both; Bot requires p2==null; Scripted accepts either). PendingBattle carries Type + P1 + nullable P2. Handler + controller adapt; v1.2 behaviour preserved because Scripted is the only type used today (Phase 2 adds Pvp, Phase 3 adds Bot). --- SVSim.BattleNode/Bridge/IMatchingBridge.cs | 19 +++++-- SVSim.BattleNode/Bridge/MatchingBridge.cs | 38 ++++++++++---- .../Hosting/BattleNodeWebSocketHandler.cs | 8 +-- SVSim.BattleNode/Sessions/PendingBattle.cs | 8 +-- .../ArenaTwoPickBattleController.cs | 5 +- .../BattleNode/Bridge/MatchingBridgeTests.cs | 50 +++++++++++++++---- .../Integration/BattleNodeFlowTests.cs | 10 +++- .../InMemoryBattleSessionStoreTests.cs | 10 ++-- 8 files changed, 111 insertions(+), 37 deletions(-) diff --git a/SVSim.BattleNode/Bridge/IMatchingBridge.cs b/SVSim.BattleNode/Bridge/IMatchingBridge.cs index 3a3f577..0092326 100644 --- a/SVSim.BattleNode/Bridge/IMatchingBridge.cs +++ b/SVSim.BattleNode/Bridge/IMatchingBridge.cs @@ -1,12 +1,25 @@ +using SVSim.BattleNode.Sessions; + namespace SVSim.BattleNode.Bridge; public interface IMatchingBridge { /// - /// Mint a battle id, register a pending session for the given viewer with their per-battle - /// MatchContext snapshot, and return the URL the client should open a socket to. + /// Mint a battle id, register a pending session, return the URL the client should + /// open a socket to. /// - PendingMatch RegisterPendingBattle(long viewerId, MatchContext context); + /// + /// Contract rules (enforced; throws ): + /// + /// Pvp: required. Both viewers expected to + /// connect WS within 60s. + /// Bot: must be null. One viewer expected; + /// opponent runs in client. + /// Scripted: currently null; future + /// server-driven bot config rides on . + /// + /// + PendingMatch RegisterBattle(BattlePlayer p1, BattlePlayer? p2, BattleType type); } public sealed record PendingMatch(string BattleId, string NodeServerUrl); diff --git a/SVSim.BattleNode/Bridge/MatchingBridge.cs b/SVSim.BattleNode/Bridge/MatchingBridge.cs index 706c2d4..565250d 100644 --- a/SVSim.BattleNode/Bridge/MatchingBridge.cs +++ b/SVSim.BattleNode/Bridge/MatchingBridge.cs @@ -5,10 +5,8 @@ namespace SVSim.BattleNode.Bridge; /// /// In-process implementation of . The HTTP-side -/// do_matching controller calls , which mints a -/// 12-digit decimal battle id, stashes a entry in the -/// , and returns the node URL the client should connect to. -/// The WebSocket handler resolves the same battle id back to the viewer on connect. +/// matching queue calls once it has decided "these two +/// play each other" or "this viewer is solo (bot/scripted)." /// public sealed class MatchingBridge : IMatchingBridge { @@ -21,17 +19,39 @@ public sealed class MatchingBridge : IMatchingBridge _options = options; } - public PendingMatch RegisterPendingBattle(long viewerId, MatchContext context) + public PendingMatch RegisterBattle(BattlePlayer p1, BattlePlayer? p2, BattleType type) { + ValidateContract(p1, p2, type); + // 12-digit decimal battle id mirrors the captures (e.g. "975695075012"). // Two unbiased 6-digit draws concatenated — RandomNumberGenerator.GetInt32 uses - // rejection sampling so the result is uniform on [0, 10^6). The previous - // implementation (Guid.GetHashCode + mod) collapsed 128 bits into ~32 of entropy - // and tripped Math.Abs(int.MinValue) UB on the unlucky hash. + // rejection sampling so the result is uniform on [0, 10^6). var hi = RandomNumberGenerator.GetInt32(0, 1_000_000); var lo = RandomNumberGenerator.GetInt32(0, 1_000_000); var battleId = $"{hi:D6}{lo:D6}"; - _store.RegisterPending(new PendingBattle(battleId, viewerId, context)); + + _store.RegisterPending(new PendingBattle(battleId, type, p1, p2)); return new PendingMatch(battleId, _options.NodeServerUrl); } + + private static void ValidateContract(BattlePlayer p1, BattlePlayer? p2, BattleType type) + { + if (p1 is null) throw new ArgumentNullException(nameof(p1)); + switch (type) + { + case BattleType.Pvp: + if (p2 is null) throw new ArgumentException("Pvp requires both p1 and p2.", nameof(p2)); + if (p1.ViewerId == p2.ViewerId) + throw new ArgumentException("Pvp requires distinct viewer ids.", nameof(p2)); + break; + case BattleType.Bot: + if (p2 is not null) throw new ArgumentException("Bot must have p2==null.", nameof(p2)); + break; + case BattleType.Scripted: + // p2 currently null; future server-driven bot will populate it. + break; + default: + throw new ArgumentOutOfRangeException(nameof(type), type, "Unknown BattleType."); + } + } } diff --git a/SVSim.BattleNode/Hosting/BattleNodeWebSocketHandler.cs b/SVSim.BattleNode/Hosting/BattleNodeWebSocketHandler.cs index 79e81f1..008e5eb 100644 --- a/SVSim.BattleNode/Hosting/BattleNodeWebSocketHandler.cs +++ b/SVSim.BattleNode/Hosting/BattleNodeWebSocketHandler.cs @@ -93,18 +93,20 @@ public sealed class BattleNodeWebSocketHandler ctx.Response.StatusCode = StatusCodes.Status404NotFound; return; } - if (pending.ViewerId != viewerId) + if (pending.P1.ViewerId != viewerId) { _log.LogWarning( "WS upgrade viewer-id mismatch on BattleId={Bid}: bridge expected={Expected}, decrypted={Got}.", - battleId, pending.ViewerId, viewerId); + battleId, pending.P1.ViewerId, viewerId); ctx.Response.StatusCode = StatusCodes.Status401Unauthorized; return; } var ws = await ctx.WebSockets.AcceptWebSocketAsync(); _store.RemovePending(battleId); - var session = new BattleSession(ws, battleId, viewerId, pending.Context, _loggerFactory.CreateLogger()); + // Phase 1: handler still constructs the old single-WS BattleSession. + // Task 9 switches to BattleSessionV2 + RealParticipant + ScriptedBotParticipant. + var session = new BattleSession(ws, battleId, viewerId, pending.P1.Context, _loggerFactory.CreateLogger()); await session.RunAsync(ctx.RequestAborted); } diff --git a/SVSim.BattleNode/Sessions/PendingBattle.cs b/SVSim.BattleNode/Sessions/PendingBattle.cs index 5a6bc0a..65f0990 100644 --- a/SVSim.BattleNode/Sessions/PendingBattle.cs +++ b/SVSim.BattleNode/Sessions/PendingBattle.cs @@ -3,8 +3,8 @@ using SVSim.BattleNode.Bridge; namespace SVSim.BattleNode.Sessions; /// -/// Sparse pre-connect record: viewer id + the per-battle MatchContext snapshot. Enough to -/// validate the incoming WS connect, resolve the viewer, and seed the BattleSession with the -/// player-half lifecycle data. Full BattleSession is created on connect. +/// Sparse pre-connect record. Carries the battle type + one or two players. The +/// WebSocket handler reads this to validate the incoming WS connect and to +/// construct the right participants. /// -public sealed record PendingBattle(string BattleId, long ViewerId, MatchContext Context); +public sealed record PendingBattle(string BattleId, BattleType Type, BattlePlayer P1, BattlePlayer? P2); diff --git a/SVSim.EmulatedEntrypoint/Controllers/ArenaTwoPickBattleController.cs b/SVSim.EmulatedEntrypoint/Controllers/ArenaTwoPickBattleController.cs index 06d14d4..d87a438 100644 --- a/SVSim.EmulatedEntrypoint/Controllers/ArenaTwoPickBattleController.cs +++ b/SVSim.EmulatedEntrypoint/Controllers/ArenaTwoPickBattleController.cs @@ -30,7 +30,10 @@ public class ArenaTwoPickBattleController : SVSimController try { var ctx = await _matchContextBuilder.BuildForTwoPickAsync(vid); - var match = _matching.RegisterPendingBattle(vid, ctx); + var match = _matching.RegisterBattle( + new SVSim.BattleNode.Bridge.BattlePlayer(vid, ctx), + p2: null, + SVSim.BattleNode.Sessions.BattleType.Scripted); return Ok(new DoMatchingResponseDto { MatchingState = 3004, diff --git a/SVSim.UnitTests/BattleNode/Bridge/MatchingBridgeTests.cs b/SVSim.UnitTests/BattleNode/Bridge/MatchingBridgeTests.cs index b37e5ce..25ddec2 100644 --- a/SVSim.UnitTests/BattleNode/Bridge/MatchingBridgeTests.cs +++ b/SVSim.UnitTests/BattleNode/Bridge/MatchingBridgeTests.cs @@ -8,44 +8,74 @@ namespace SVSim.UnitTests.BattleNode.Bridge; public class MatchingBridgeTests { [Test] - public void RegisterPendingBattle_RegistersInStoreAndReturnsNodeUrl() + public void RegisterBattle_Scripted_stores_pending_and_returns_node_url() { var store = new InMemoryBattleSessionStore(); var bridge = new MatchingBridge(store, new BattleNodeOptions { NodeServerUrl = "localhost:5148/socket.io/" }); - var ctx = FixtureCtx(); + var p1 = new BattlePlayer(906243102, FixtureCtx()); - var match = bridge.RegisterPendingBattle(viewerId: 906243102, context: ctx); + var match = bridge.RegisterBattle(p1, p2: null, BattleType.Scripted); Assert.That(match.NodeServerUrl, Is.EqualTo("localhost:5148/socket.io/")); Assert.That(match.BattleId, Is.Not.Empty); var pending = store.TryGetPending(match.BattleId); Assert.That(pending, Is.Not.Null); - Assert.That(pending!.ViewerId, Is.EqualTo(906243102)); - Assert.That(pending.Context, Is.SameAs(ctx)); + Assert.That(pending!.Type, Is.EqualTo(BattleType.Scripted)); + Assert.That(pending.P1.ViewerId, Is.EqualTo(906243102)); + Assert.That(pending.P2, Is.Null); } [Test] - public void RegisterPendingBattle_MintsUniqueBattleIds() + public void RegisterBattle_mints_unique_BattleIds_per_call() { var bridge = new MatchingBridge(new InMemoryBattleSessionStore(), new BattleNodeOptions()); - var a = bridge.RegisterPendingBattle(1, FixtureCtx()); - var b = bridge.RegisterPendingBattle(2, FixtureCtx()); + var a = bridge.RegisterBattle(new BattlePlayer(1, FixtureCtx()), null, BattleType.Scripted); + var b = bridge.RegisterBattle(new BattlePlayer(2, FixtureCtx()), null, BattleType.Scripted); Assert.That(a.BattleId, Is.Not.EqualTo(b.BattleId)); } [Test] - public void RegisterPendingBattle_ProducesTwelveDigitDecimalBattleId() + public void RegisterBattle_produces_twelve_digit_decimal_BattleId() { var bridge = new MatchingBridge(new InMemoryBattleSessionStore(), new BattleNodeOptions()); - var match = bridge.RegisterPendingBattle(viewerId: 1, context: FixtureCtx()); + var match = bridge.RegisterBattle(new BattlePlayer(1, FixtureCtx()), null, BattleType.Scripted); Assert.That(match.BattleId, Has.Length.EqualTo(12)); Assert.That(match.BattleId, Does.Match("^[0-9]{12}$")); } + [Test] + public void RegisterBattle_Pvp_requires_both_players() + { + var bridge = new MatchingBridge(new InMemoryBattleSessionStore(), new BattleNodeOptions()); + + Assert.Throws(() => + bridge.RegisterBattle(new BattlePlayer(1, FixtureCtx()), p2: null, BattleType.Pvp)); + } + + [Test] + public void RegisterBattle_Pvp_rejects_same_viewer_twice() + { + var bridge = new MatchingBridge(new InMemoryBattleSessionStore(), new BattleNodeOptions()); + + Assert.Throws(() => + bridge.RegisterBattle( + new BattlePlayer(1, FixtureCtx()), new BattlePlayer(1, FixtureCtx()), BattleType.Pvp)); + } + + [Test] + public void RegisterBattle_Bot_rejects_non_null_p2() + { + var bridge = new MatchingBridge(new InMemoryBattleSessionStore(), new BattleNodeOptions()); + + Assert.Throws(() => + bridge.RegisterBattle( + new BattlePlayer(1, FixtureCtx()), new BattlePlayer(2, FixtureCtx()), BattleType.Bot)); + } + private static MatchContext FixtureCtx() => new( SelfDeckCardIds: Enumerable.Range(1, 30).Select(i => 100_011_010L).ToList(), ClassId: "1", CharaId: "1", CardMasterName: "card_master_node_10015", diff --git a/SVSim.UnitTests/BattleNode/Integration/BattleNodeFlowTests.cs b/SVSim.UnitTests/BattleNode/Integration/BattleNodeFlowTests.cs index a582000..64ad07d 100644 --- a/SVSim.UnitTests/BattleNode/Integration/BattleNodeFlowTests.cs +++ b/SVSim.UnitTests/BattleNode/Integration/BattleNodeFlowTests.cs @@ -32,7 +32,10 @@ public class BattleNodeFlowTests using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); var ct = cts.Token; - var pending = bridge.RegisterPendingBattle(viewerId: 906243102, context: FixtureCtx()); + var pending = bridge.RegisterBattle( + new SVSim.BattleNode.Bridge.BattlePlayer(906243102, FixtureCtx()), + p2: null, + SVSim.BattleNode.Sessions.BattleType.Scripted); var key = MakeKey(); var encryptedVid = NodeCrypto.EncryptForNode("906243102", key); @@ -132,7 +135,10 @@ public class BattleNodeFlowTests using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); var ct = cts.Token; - var pending = bridge.RegisterPendingBattle(viewerId: vid, context: ctx); + var pending = bridge.RegisterBattle( + new SVSim.BattleNode.Bridge.BattlePlayer(vid, ctx), + p2: null, + SVSim.BattleNode.Sessions.BattleType.Scripted); var key = MakeKey(); var encryptedVid = NodeCrypto.EncryptForNode(vid.ToString(), key); diff --git a/SVSim.UnitTests/BattleNode/Sessions/InMemoryBattleSessionStoreTests.cs b/SVSim.UnitTests/BattleNode/Sessions/InMemoryBattleSessionStoreTests.cs index 02d863f..4980678 100644 --- a/SVSim.UnitTests/BattleNode/Sessions/InMemoryBattleSessionStoreTests.cs +++ b/SVSim.UnitTests/BattleNode/Sessions/InMemoryBattleSessionStoreTests.cs @@ -14,7 +14,7 @@ public class InMemoryBattleSessionStoreTests [Test] public void RegisterThenGet_ReturnsRegisteredBattle() { - var battle = new PendingBattle("bid-1", 906243102, FixtureCtx()); + var battle = new PendingBattle("bid-1", BattleType.Scripted, new BattlePlayer(906243102, FixtureCtx()), null); _store.RegisterPending(battle); Assert.That(_store.TryGetPending("bid-1"), Is.EqualTo(battle)); @@ -29,7 +29,7 @@ public class InMemoryBattleSessionStoreTests [Test] public void Remove_ReturnsTrueWhenPresent_FalseWhenAbsent() { - _store.RegisterPending(new PendingBattle("bid", 1, FixtureCtx())); + _store.RegisterPending(new PendingBattle("bid", BattleType.Scripted, new BattlePlayer(1, FixtureCtx()), null)); Assert.That(_store.RemovePending("bid"), Is.True); Assert.That(_store.RemovePending("bid"), Is.False); } @@ -37,9 +37,9 @@ public class InMemoryBattleSessionStoreTests [Test] public void Register_DuplicateBattleId_OverwritesPrior() { - _store.RegisterPending(new PendingBattle("bid", 1, FixtureCtx())); - _store.RegisterPending(new PendingBattle("bid", 2, FixtureCtx())); - Assert.That(_store.TryGetPending("bid")!.ViewerId, Is.EqualTo(2)); + _store.RegisterPending(new PendingBattle("bid", BattleType.Scripted, new BattlePlayer(1, FixtureCtx()), null)); + _store.RegisterPending(new PendingBattle("bid", BattleType.Scripted, new BattlePlayer(2, FixtureCtx()), null)); + Assert.That(_store.TryGetPending("bid")!.P1.ViewerId, Is.EqualTo(2)); } private static MatchContext FixtureCtx() => new(