fix #436: combat no-target refusal reaches the SpewBox with retail's exact text

Attacking with no valid target has told the player nothing since Campaign V
slice V11 orphaned the DebugVM toast the message was wired to (#434 found
the drop; this closes it retail-faithfully).

Ground truth from the Ghidra decompile of
ClientCombatSystem::ExecuteAttack (0x0056bb70): retail writes
"You must select a valid combat target before attacking" via
ClientSystem::AddTextToScroll(..., 0x1A, true, 0) — the ClientLocal
SpewBox channel this codebase already routes every other client-local
refusal through. And retail has ONE message, not the two we carried:
attacking outside melee/missile modes is silent (ExecuteAttack is
unreachable there), so the invented "Enter melee or missile combat first"
text is deleted rather than rerouted, and the invented "No monster
target" is replaced by the retail string, which joins ClientTextRefusals
with its decomp citation.

Wiring: CombatFeedbackSlot gains the sibling BindOwned session-lifetime
shape, and SessionPlayerComposition.CompleteSessionPlayer binds it to
RuntimeCommunicationState.AddText(ClientLocal) with session-owned
teardown — a torn-down session's slot returns to its silent unbound
state. A binding-seam test
(CompleteSessionPlayerBindsCombatFeedbackToTheClientLocalSpewBoxRoute)
inspects the compiled composition for the BindOwned call and its
AddText-routing lambda, so the slot can never again pass its unit tests
while production leaves it unbound — the exact failure mode that hid
this defect. The two tests that pinned the invented strings now pin the
retail contract (exact string; silence for the unsupported-mode case).

Full hermetic suite 15,325 passed / 0 failed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-24 12:53:11 +02:00
parent 7969a20c8c
commit 35454a9f58
7 changed files with 157 additions and 19 deletions

View file

@ -34,13 +34,42 @@ public sealed class CombatFeedbackSlotTests
Assert.Throws<InvalidOperationException>(() => slot.Bind(_ => { }));
}
[Fact]
public void SessionOwnedBindingForwardsAndItsDisposalRestoresSilence()
{
// #436: the session composition owns the production binding
// (SpewBox/ClientLocal) through BindOwned, mirroring
// CombatAttackOperationsSlot's lifetime shape — disposal at session
// teardown returns the slot to its silent unbound state instead of
// leaving a dead session's chat sink reachable.
var slot = new CombatFeedbackSlot();
List<string> sink = [];
IDisposable binding = slot.BindOwned(sink.Add);
slot.Show("routed");
Assert.Equal(["routed"], sink);
binding.Dispose();
slot.Show("after-teardown");
Assert.Equal(["routed"], sink);
}
[Fact]
public void SessionOwnedBindingRefusesASecondOwner()
{
var slot = new CombatFeedbackSlot();
using IDisposable binding = slot.BindOwned(_ => { });
Assert.Throws<InvalidOperationException>(() => slot.BindOwned(_ => { }));
}
[Fact]
public void AnUnboundSlotDropsItsMessages()
{
// #434/#436: this is the shipped behavior, not an aspiration —
// nothing binds the slot in production, so combat refusal text
// ("No monster target") is discarded. Pinned so the day it gets a
// real binder, this test is the one that has to change.
// Deliberate: before a session composes (and after one tears down)
// there is no chat surface, so Show is a no-op rather than a queue —
// matching retail, where the refusal text only exists inside a live
// session's ExecuteAttack path (0x0056bb70).
var slot = new CombatFeedbackSlot();
slot.Show("dropped");

View file

@ -54,13 +54,18 @@ public sealed class LiveCombatAttackOperationsTests
}
[Fact]
public void UnsupportedCombatModeUsesTypedFeedbackSink()
public void UnsupportedCombatModeRefusesSilently()
{
// #436: retail emits NO text for this case —
// ClientCombatSystem::ExecuteAttack (0x0056bb70) is unreachable
// outside melee/missile modes, so there is no retail string to show.
// The pre-#436 "Enter melee or missile combat first" message was
// invented dev text and is deleted, not rerouted.
Harness harness = CreateHarness(inWorld: true);
Assert.False(harness.Owner.CanStartAttack());
Assert.Equal(["Enter melee or missile combat first"], harness.Feedback.Messages);
Assert.Empty(harness.Feedback.Messages);
Assert.Equal(0, harness.Targets.ResolveCount);
}
@ -88,7 +93,11 @@ public sealed class LiveCombatAttackOperationsTests
Assert.False(harness.Owner.CanStartAttack());
Assert.Equal(["No monster target"], harness.Feedback.Messages);
// Retail's exact string — ClientCombatSystem::ExecuteAttack's
// no-valid-target branch (0x0056bc05), AddTextToScroll(0x1A).
Assert.Equal(
[AcDream.Core.Chat.ClientTextRefusals.MustSelectCombatTarget],
harness.Feedback.Messages);
}
private static Harness CreateHarness(bool inWorld = false)

View file

@ -106,6 +106,38 @@ public sealed class SessionPlayerCompositionTests
field => field.FieldType == typeof(GameWindow));
}
[Fact]
public void CompleteSessionPlayerBindsCombatFeedbackToTheClientLocalSpewBoxRoute()
{
// #436 binding-seam check (a slot can pass its own unit tests while
// nothing in production binds it — that is exactly how the combat
// refusals were silently dropped for months): CompleteSessionPlayer
// must session-own a CombatFeedbackSlot.BindOwned whose target lambda
// routes to RuntimeCommunicationState.AddText (the ClientLocal
// SpewBox chokepoint; retail ClientCombatSystem::ExecuteAttack
// 0x0056bb70 -> AddTextToScroll(0x1A)).
MethodInfo complete = RequiredMethod(
typeof(SessionPlayerCompositionPhase),
"CompleteSessionPlayer");
Assert.Contains(
CompiledCallGraph.Read(complete),
call => call.Target.DeclaringType
== typeof(AcDream.App.Combat.CombatFeedbackSlot)
&& call.Target.Name
== nameof(AcDream.App.Combat.CombatFeedbackSlot.BindOwned));
IEnumerable<MethodBase> lambdas =
CompiledCallGraph.ReadMethodReferences(complete)
.Select(call => call.Target)
.Where(method => method.GetMethodBody() is not null);
Assert.Contains(
lambdas,
method => CompiledCallGraph.Read(method).Any(call =>
call.Target.DeclaringType
== typeof(AcDream.Runtime.Gameplay.RuntimeCommunicationState)
&& call.Target.Name == "AddText"));
}
[Fact]
public void ProductionPhaseStartsStreamerBeforeSessionAndTransfersPortalLast()
{