refactor(matching): IMatchingResolver shared by every do_matching family
SoloDefaultsToScripted was only consulted by ArenaTwoPickBattleController; RankBattleController did its own inline pair-up + state-code mapping and ignored the flag entirely. Result: turning on the flag globally only short-circuited TK2 polls, while rank-battle polls still parked for the PvpFirstThenAiFallback threshold (15s) before resolving — surfaced today when the user set the flag and saw rank-battle still queue, then bot- battle via the client-side AI (not the server-side Scripted lifecycle we need to test WS traffic against). New IMatchingResolver owns the cross-cutting decisions: - honor scriptedOptIn (per-request) OR options.SoloDefaultsToScripted (process-wide) — bypass pair-up, register Scripted, return 3004 - otherwise call IMatchingPairUpService.TryPairAsync and translate the PairUpResult to the 3002/3004/3007/3011 vocabulary Family controllers shed the duplicated logic: - ArenaTwoPickBattleController: ~50 LOC → ~25; preserves ?scripted=1 query opt-in (parsed permissively for "1"/"true") and the ArenaTwoPickException catch - RankBattleController: ~30 LOC → ~12; preserves the 3001 mapping for InvalidOperationException (no deck for format) and card_master_id emission DoMatchingContractTests is the durable enforcement: parametrized over TK2 + rotation + unlimited rank, asserts SoloDefaultsToScripted=true makes every family's first poll skip 3002 and return SUCCEEDED with a battle_id + node_server_url. Adding a fourth family that forgets to route through IMatchingResolver fails this test — that's the point. MatchingResolverTests covers the six resolver paths in isolation with mocks; per-test Harness locals (not fixture-level fields) because the assembly is [Parallelizable(ParallelScope.All)] and shared mocks race. 957 tests passing (was 948; +9: 6 resolver + 3 contract parametrizations). No regressions in the existing TK2 / rank-battle controller suites. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -11,23 +11,17 @@ namespace SVSim.EmulatedEntrypoint.Controllers;
|
||||
public class ArenaTwoPickBattleController : SVSimController
|
||||
{
|
||||
private readonly IArenaTwoPickService _svc;
|
||||
private readonly IMatchingBridge _matching;
|
||||
private readonly IMatchContextBuilder _matchContextBuilder;
|
||||
private readonly IMatchingPairUpService _pairUp;
|
||||
private readonly BattleNodeOptions _battleNodeOptions;
|
||||
private readonly IMatchingResolver _resolver;
|
||||
|
||||
public ArenaTwoPickBattleController(
|
||||
IArenaTwoPickService svc,
|
||||
IMatchingBridge matching,
|
||||
IMatchContextBuilder matchContextBuilder,
|
||||
IMatchingPairUpService pairUp,
|
||||
BattleNodeOptions battleNodeOptions)
|
||||
IMatchingResolver resolver)
|
||||
{
|
||||
_svc = svc;
|
||||
_matching = matching;
|
||||
_matchContextBuilder = matchContextBuilder;
|
||||
_pairUp = pairUp;
|
||||
_battleNodeOptions = battleNodeOptions;
|
||||
_resolver = resolver;
|
||||
}
|
||||
|
||||
[HttpPost("do_matching")]
|
||||
@@ -37,59 +31,21 @@ public class ArenaTwoPickBattleController : SVSimController
|
||||
CancellationToken ct = default)
|
||||
{
|
||||
if (!TryGetViewerId(out var vid)) return Unauthorized();
|
||||
// Accept "1" or "true" (case-insensitive) as opt-in for the legacy Scripted path.
|
||||
// ASP.NET's default bool binder rejects "1", so do a permissive parse here.
|
||||
// The server-side BattleNodeOptions.SoloDefaultsToScripted flag is the other
|
||||
// route — it bypasses pair-up for every solo poll, useful when the live client
|
||||
// (which can't append query params) needs a Scripted match.
|
||||
var useScripted = (scripted is not null
|
||||
&& (scripted == "1" || string.Equals(scripted, "true", StringComparison.OrdinalIgnoreCase)))
|
||||
|| _battleNodeOptions.SoloDefaultsToScripted;
|
||||
// Accept "1" or "true" (case-insensitive) as per-request opt-in for the Scripted
|
||||
// path. ASP.NET's default bool binder rejects "1", so parse permissively here.
|
||||
// BattleNodeOptions.SoloDefaultsToScripted is the process-wide equivalent and is
|
||||
// applied inside the resolver.
|
||||
var scriptedOptIn = scripted is not null
|
||||
&& (scripted == "1" || string.Equals(scripted, "true", StringComparison.OrdinalIgnoreCase));
|
||||
try
|
||||
{
|
||||
var ctx = await _matchContextBuilder.BuildForTwoPickAsync(vid);
|
||||
|
||||
if (useScripted)
|
||||
{
|
||||
var scriptedMatch = _matching.RegisterBattle(
|
||||
new SVSim.BattleNode.Bridge.BattlePlayer(vid, ctx),
|
||||
p2: null,
|
||||
SVSim.BattleNode.Sessions.BattleType.Scripted);
|
||||
return Ok(new DoMatchingResponseDto
|
||||
{
|
||||
MatchingState = 3004,
|
||||
BattleId = scriptedMatch.BattleId,
|
||||
NodeServerUrl = scriptedMatch.NodeServerUrl,
|
||||
});
|
||||
}
|
||||
|
||||
var paired = await _pairUp.TryPairAsync(
|
||||
"arena_two_pick_battle",
|
||||
new SVSim.BattleNode.Bridge.BattlePlayer(vid, ctx),
|
||||
ct);
|
||||
if (paired is null)
|
||||
{
|
||||
// 3002 = RC_BATTLE_MATCHING_RETRY: client polls again. 3001 is ILLEGAL
|
||||
// and shows an error dialog on the client side. node_server_url must be
|
||||
// present (the client's DoMatchingBase.SettingDoMatchingData calls
|
||||
// .ToString() on it without a Keys.Contains guard); prod sends "" while
|
||||
// waiting and the real URL only on SUCCEEDED. battle_id stays absent
|
||||
// (its accessor IS guarded).
|
||||
return Ok(new DoMatchingResponseDto
|
||||
{
|
||||
MatchingState = 3002,
|
||||
NodeServerUrl = "",
|
||||
});
|
||||
}
|
||||
|
||||
// Owner (first arriver, cache hit) gets 3007 = RC_BATTLE_MATCHING_SUCCEEDED_OWNER;
|
||||
// joiner (second arriver who triggered the pair) gets 3004 = RC_BATTLE_MATCHING_SUCCEEDED.
|
||||
// See PairUpResult docs for why this split is observationally inert in TK2 today.
|
||||
var r = await _resolver.ResolveAsync("arena_two_pick_battle", new BattlePlayer(vid, ctx), scriptedOptIn, ct);
|
||||
return Ok(new DoMatchingResponseDto
|
||||
{
|
||||
MatchingState = paired.IsOwner ? 3007 : 3004,
|
||||
BattleId = paired.Match.BattleId,
|
||||
NodeServerUrl = paired.Match.NodeServerUrl,
|
||||
MatchingState = r.MatchingState,
|
||||
BattleId = r.BattleId,
|
||||
NodeServerUrl = r.NodeServerUrl,
|
||||
});
|
||||
}
|
||||
catch (ArenaTwoPickException ex)
|
||||
|
||||
@@ -23,23 +23,20 @@ namespace SVSim.EmulatedEntrypoint.Controllers;
|
||||
[Authorize(AuthenticationSchemes = SteamAuthenticationConstants.SchemeName)]
|
||||
public sealed class RankBattleController : ControllerBase
|
||||
{
|
||||
private readonly IMatchingPairUpService _pairUp;
|
||||
private readonly IMatchingBridge _bridge;
|
||||
private readonly IMatchingResolver _resolver;
|
||||
private readonly IBattleSessionStore _sessionStore;
|
||||
private readonly IMatchContextBuilder _ctxBuilder;
|
||||
private readonly IBotRoster _botRoster;
|
||||
private readonly ILogger<RankBattleController> _log;
|
||||
|
||||
public RankBattleController(
|
||||
IMatchingPairUpService pairUp,
|
||||
IMatchingBridge bridge,
|
||||
IMatchingResolver resolver,
|
||||
IBattleSessionStore sessionStore,
|
||||
IMatchContextBuilder ctxBuilder,
|
||||
IBotRoster botRoster,
|
||||
ILogger<RankBattleController> log)
|
||||
{
|
||||
_pairUp = pairUp;
|
||||
_bridge = bridge;
|
||||
_resolver = resolver;
|
||||
_sessionStore = sessionStore;
|
||||
_ctxBuilder = ctxBuilder;
|
||||
_botRoster = botRoster;
|
||||
@@ -135,33 +132,16 @@ public sealed class RankBattleController : ControllerBase
|
||||
return Ok(new DoMatchingResponseDto { MatchingState = 3001, NodeServerUrl = "" });
|
||||
}
|
||||
|
||||
var paired = await _pairUp.TryPairAsync(mode, new BattlePlayer(vid, ctx), ct);
|
||||
|
||||
if (paired is null)
|
||||
{
|
||||
// Parked. 3002 RETRY. node_server_url must be present as empty string —
|
||||
// client's DoMatchingBase parser calls .ToString() without a guard.
|
||||
return Ok(new DoMatchingResponseDto
|
||||
{
|
||||
MatchingState = 3002,
|
||||
NodeServerUrl = "",
|
||||
});
|
||||
}
|
||||
|
||||
// Owner cache-pickup → 3007 (PvP) or 3011 (AI fallback).
|
||||
// Joiner (only PvP) → 3004.
|
||||
var state = paired switch
|
||||
{
|
||||
{ IsAiFallback: true } => 3011,
|
||||
{ IsOwner: true } => 3007,
|
||||
_ => 3004,
|
||||
};
|
||||
// Rank battle has no ?scripted=1 query opt-in (no live capture has shown such a
|
||||
// param on the rank URLs). The process-wide BattleNodeOptions.SoloDefaultsToScripted
|
||||
// toggle is the only scripted entry point and is honored inside the resolver.
|
||||
var r = await _resolver.ResolveAsync(mode, new BattlePlayer(vid, ctx), scriptedOptIn: false, ct);
|
||||
|
||||
return Ok(new DoMatchingResponseDto
|
||||
{
|
||||
MatchingState = state,
|
||||
BattleId = paired.Match.BattleId,
|
||||
NodeServerUrl = paired.Match.NodeServerUrl,
|
||||
MatchingState = r.MatchingState,
|
||||
BattleId = r.BattleId,
|
||||
NodeServerUrl = r.NodeServerUrl,
|
||||
// Placeholder per spec § Out of scope — per-battle card-master split is deferred.
|
||||
CardMasterId = 0,
|
||||
});
|
||||
|
||||
54
SVSim.EmulatedEntrypoint/Matching/IMatchingResolver.cs
Normal file
54
SVSim.EmulatedEntrypoint/Matching/IMatchingResolver.cs
Normal file
@@ -0,0 +1,54 @@
|
||||
using SVSim.BattleNode.Bridge;
|
||||
|
||||
namespace SVSim.EmulatedEntrypoint.Matching;
|
||||
|
||||
/// <summary>
|
||||
/// Single source of truth for how a <c>/do_matching</c> request is resolved into a wire
|
||||
/// matching_state + battle_id + node_server_url across every battle family.
|
||||
/// <para>
|
||||
/// Lives here (and not on each controller) because the resolution rules are the same
|
||||
/// regardless of which URL family carried the request:
|
||||
/// </para>
|
||||
/// <list type="number">
|
||||
/// <item>Honor the dev-affordance scripted opt-in (route flag and/or
|
||||
/// <see cref="BattleNodeOptions.SoloDefaultsToScripted"/>) — bypass pair-up,
|
||||
/// register a Scripted match, return immediately.</item>
|
||||
/// <item>Otherwise consult <see cref="IMatchingPairUpService"/> and translate the
|
||||
/// resulting <see cref="PairUpResult"/> into a wire matching_state per the
|
||||
/// 3002 / 3004 / 3007 / 3011 vocabulary.</item>
|
||||
/// </list>
|
||||
/// <para>
|
||||
/// Family-specific details (DTO shapes, family-specific request fields like
|
||||
/// <c>card_master_id</c>, error-mapping like rank-battle's 3001 on a missing deck) stay
|
||||
/// on the controllers. The resolver only owns the cross-cutting "did the flag win, did
|
||||
/// pair-up resolve, what's the state code" decision.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public interface IMatchingResolver
|
||||
{
|
||||
/// <param name="mode">
|
||||
/// The matching-mode key the resolver passes through to
|
||||
/// <see cref="IMatchingPairUpService.TryPairAsync"/> — one of the
|
||||
/// <see cref="ModePolicy"/> registry's mode names (e.g. <c>"arena_two_pick_battle"</c>,
|
||||
/// <c>"rotation_rank_battle"</c>, <c>"unlimited_rank_battle"</c>).
|
||||
/// </param>
|
||||
/// <param name="player">Caller's <see cref="BattlePlayer"/> (viewer-id + built MatchContext).</param>
|
||||
/// <param name="scriptedOptIn">
|
||||
/// Per-request opt-in from a controller-specific signal (e.g. TK2's <c>?scripted=1</c>
|
||||
/// query param). OR'd with <see cref="BattleNodeOptions.SoloDefaultsToScripted"/>;
|
||||
/// either being true short-circuits to a Scripted match.
|
||||
/// </param>
|
||||
Task<MatchingResolution> ResolveAsync(
|
||||
string mode,
|
||||
BattlePlayer player,
|
||||
bool scriptedOptIn,
|
||||
CancellationToken ct);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Wire-level outcome of a <c>/do_matching</c> resolution. Always carries a non-null
|
||||
/// <see cref="NodeServerUrl"/> — empty string while parked (3002), real URL on resolution —
|
||||
/// because the client's <c>DoMatchingBase.SettingDoMatchingData()</c> calls
|
||||
/// <c>.ToString()</c> on the wire field without a <c>Keys.Contains</c> guard.
|
||||
/// </summary>
|
||||
public sealed record MatchingResolution(int MatchingState, string? BattleId, string NodeServerUrl);
|
||||
63
SVSim.EmulatedEntrypoint/Matching/MatchingResolver.cs
Normal file
63
SVSim.EmulatedEntrypoint/Matching/MatchingResolver.cs
Normal file
@@ -0,0 +1,63 @@
|
||||
using SVSim.BattleNode.Bridge;
|
||||
using SVSim.BattleNode.Sessions;
|
||||
|
||||
namespace SVSim.EmulatedEntrypoint.Matching;
|
||||
|
||||
/// <inheritdoc cref="IMatchingResolver"/>
|
||||
public sealed class MatchingResolver : IMatchingResolver
|
||||
{
|
||||
private readonly IMatchingBridge _bridge;
|
||||
private readonly IMatchingPairUpService _pairUp;
|
||||
private readonly BattleNodeOptions _options;
|
||||
|
||||
public MatchingResolver(
|
||||
IMatchingBridge bridge,
|
||||
IMatchingPairUpService pairUp,
|
||||
BattleNodeOptions options)
|
||||
{
|
||||
_bridge = bridge;
|
||||
_pairUp = pairUp;
|
||||
_options = options;
|
||||
}
|
||||
|
||||
public Task<MatchingResolution> ResolveAsync(
|
||||
string mode,
|
||||
BattlePlayer player,
|
||||
bool scriptedOptIn,
|
||||
CancellationToken ct)
|
||||
{
|
||||
// Dev-affordance short-circuit. Either a per-request flag (e.g. ?scripted=1) or the
|
||||
// process-wide BattleNodeOptions.SoloDefaultsToScripted toggle puts us here.
|
||||
// Registers a Scripted match (server-side scripted opponent in BattleSession) and
|
||||
// returns matching_state=3004 SUCCEEDED so the client opens the WS and proceeds.
|
||||
if (scriptedOptIn || _options.SoloDefaultsToScripted)
|
||||
{
|
||||
var m = _bridge.RegisterBattle(player, p2: null, BattleType.Scripted);
|
||||
return Task.FromResult(new MatchingResolution(3004, m.BattleId, m.NodeServerUrl));
|
||||
}
|
||||
|
||||
return ResolveViaPairUpAsync(mode, player, ct);
|
||||
}
|
||||
|
||||
private async Task<MatchingResolution> ResolveViaPairUpAsync(string mode, BattlePlayer player, CancellationToken ct)
|
||||
{
|
||||
var paired = await _pairUp.TryPairAsync(mode, player, ct);
|
||||
if (paired is null)
|
||||
{
|
||||
// Parked. matching_state 3002 RETRY. node_server_url MUST be present as empty
|
||||
// string (the client unguarded-.ToString()s it before consulting matching_state).
|
||||
return new MatchingResolution(3002, BattleId: null, "");
|
||||
}
|
||||
|
||||
// 3011 = AI_BATTLE_MATCHING_SUCCEEDED (PvpFirstThenAiFallback policy's threshold fired)
|
||||
// 3007 = RC_BATTLE_MATCHING_SUCCEEDED_OWNER (first arriver, cache pickup)
|
||||
// 3004 = RC_BATTLE_MATCHING_SUCCEEDED (joiner — triggered the pair)
|
||||
var state = paired switch
|
||||
{
|
||||
{ IsAiFallback: true } => 3011,
|
||||
{ IsOwner: true } => 3007,
|
||||
_ => 3004,
|
||||
};
|
||||
return new MatchingResolution(state, paired.Match.BattleId, paired.Match.NodeServerUrl);
|
||||
}
|
||||
}
|
||||
@@ -138,6 +138,10 @@ public class Program
|
||||
new ModePolicy("unlimited_rank_battle", PolicyKind.PvpFirstThenAiFallback),
|
||||
}));
|
||||
builder.Services.AddSingleton<IMatchingPairUpService, InProcessPairUp>();
|
||||
// Single resolver shared by every /do_matching family controller. Owns the scripted-
|
||||
// flag short-circuit + the pair-up → matching_state mapping. Singleton: stateless,
|
||||
// all deps are singletons too.
|
||||
builder.Services.AddSingleton<IMatchingResolver, MatchingResolver>();
|
||||
// Phase 3: bot roster used by RankBattleController.AiStart to compose oppo_info.
|
||||
// Transient because BotRoster depends on the transient IGlobalsRepository.
|
||||
builder.Services.AddTransient<IBotRoster, BotRoster>();
|
||||
@@ -156,7 +160,7 @@ public class Program
|
||||
});
|
||||
|
||||
var app = builder.Build();
|
||||
|
||||
|
||||
// Update database (skipped for non-relational providers, e.g. InMemory in tests, and
|
||||
// skipped under the "Testing" environment where the test fixture has already called
|
||||
// EnsureCreated against a SQLite in-memory DB — the Postgres migrations would fail there).
|
||||
|
||||
Reference in New Issue
Block a user