fix(chat): CH1 review fixes — sbb-idiom channel catch-all, command-output typing

Applies the Opus review findings on CH1 (172c6f9a), the exact retail chat
color table. Two blockers plus should-fixes/nits, one commit:

BLOCKER 1 — LegacyChannelChatType.Resolve's channel-bit table was wrong.
Binary Ninja renders retail's `neg esi; sbb esi, esi` idiom (a branchless
select between Channel 0x08 and Channel_Send 0x09) as the trivial pseudo-C
`esi - esi` (always 0), hiding the real values. Corrected by decoding the
raw bytes at the PDB-paired binary: HEAR sbb site VA 0x00570F0A (mask -6 ->
0x08), SEND sbb site VA 0x00570D4F (mask -5 -> 0x09). The generic
admin/audit/sentinel catch-all is Channel/Channel_Send, NOT Abuse (0x0E) —
Abuse is retail's ONLY 0x0E producer (bit 0x0001). The unnamed
FellowBroadcast bit (0x4000000) is hear=Channel(0x08)/send=Fellowship(0x13),
not a flat 0x13. ACE's PDB-sourced Channel enum corroborates. Introduces
`RetailLogTextType`, the 34-value named enum for the wire LogTextType space
(values only, no color — Core stays presentation-free).

BLOCKER 2 — three ChatLog.OnSystemMessage sinks (ChatVM.ShowSystemMessage,
LiveSessionRuntimeFactory's ShowSystemMessage delegate,
HeadlessGameplayOperations.DisplayMessage) were typing ALL
ClientCommandController output 0x1A (bright red), including informational
command output (@version, /loc, friends list, usage lines). Retail types
the great majority of that output 0x00 Default (green) and reserves 0x1A
for genuine refusals/errors. Reverted to 0x00 with a comment noting the
refusal-vs-info split lands with CH2's SpewBox producer rewiring. The five
App composition sites that pass 0x1A for actual refusal text
(InteractionRetainedUiComposition, SessionPlayerComposition) were already
correct and are untouched (aside from converting the literal to the new
enum).

Also: AP-176 divergence-register row for OnWeenieError/OnCombatLine's
single-type approximation of retail's per-code/per-message dispatch; a
carry-forward test for the out-of-range LogTextType color fallback in
ChatWindowController; decomp-confirmed anchors replacing ACE-inferred
citations in CombatChatTranslator and ChatLog.OnPlayerKilled; required
(non-optional) logTextType parameters on OnLocalSpeech/OnTellReceived/
OnCombatLine/OnSelfSent since no production caller relied on a default;
LegacyChannelChatType.Resolve's parameter renamed channelBit -> channelId
with a doc note on multi-bit ids; corrections to the color-table research
doc's §3.3 wire tables; and issue #359 for the pre-existing (not
CH1-introduced) 0x019E PlayerKilled participant-suppression gap retail has
and acdream lacks.

dotnet build clean; full Release suite 11,835 passed / 4 skipped / 0 failed
(11,839 total), up from the CH1 baseline of 11,833/4/0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-09 16:02:59 +02:00
parent e306c979ae
commit 34d8a3c0e7
31 changed files with 481 additions and 213 deletions

View file

@ -75,6 +75,27 @@ CPU, so it silences already-playing voices instantly without touching the
retail mixing math or persisted volumes. Once the trigger works, no further
audio-side work is needed.
## #359 — 0x019E PlayerKilled line prints to participants — retail suppresses it
**Status:** OPEN — filed 2026-08-09 at the CH1 Opus review. Pre-existing (not
introduced by CH1); candidate for CH4/CH5.
**Symptom:** `ChatLog.OnPlayerKilled` (`src/AcDream.Core/Chat/ChatLog.cs`)
always appends the death message for every recipient of the `0x019E`
PlayerKilled GameEvent. Retail's `ClientCombatSystem::HandlePlayerDeathEvent
@0x0056C320` skips the `AddTextToScroll` call when the receiving player IS a
participant — `player_id == victim || player_id == killer` — so the victim
and killer see the notification through their own dedicated
Victim/KillerNotification lines (0x01AC/0x01AD) instead, and would see it
twice if the bystander-facing PlayerKilled line were not suppressed for
them. acdream has no such guard: `OnPlayerKilled` prints unconditionally
regardless of whether the local player is the victim, the killer, or an
uninvolved bystander.
**Fix shape:** thread the local player's guid into `OnPlayerKilled` (or its
caller) and skip the append when it matches `victimGuid` or `killerGuid`,
matching retail's participant check.
## #357 — Login stalls: reveal reaches ready=True but the player is never placed; UI + sky render, world never opens
**Status:** CLOSED 2026-08-08 — root-caused and fixed same session (see the

File diff suppressed because one or more lines are too long

View file

@ -99,7 +99,7 @@ implementer per slice against a pinned contract (per
| Slice | Commit | Suite | Review | User gate |
|---|---|---|---|---|
| R1R4 research | `see docs/research/2026-08-09-chat-retail-*` | — | — | — |
| CH1 colors | `172c6f9a` | 11,833 passed / 4 skipped / 0 failed | in review | pending |
| CH1 colors | `172c6f9a` | 11,835 passed / 4 skipped / 0 failed | APPROVE-WITH-FIXES; fixed `5f85ec9c` | pending |
| CH2 interface text | — | — | — | — |
| CH3 side channels | — | — | — | — |
| CH4 commands | — | — | — | — |

View file

@ -267,15 +267,26 @@ Anchors are the `ClientCommunicationSystem::Handle_Communication__*` handlers.
`Handle_Communication__ChannelBroadcast` channel-bit → type (the `m_buffer_5`
variable feeding `AddTextToScroll` at `0x00571169`):
**Corrected 2026-08-09 (review):** the row 8 rows below marked "corrected"
were wrong in the original CH1 drop. Binary Ninja renders retail's `neg esi;
sbb esi, esi` idiom — a branchless select between `Channel` (0x08) and
`Channel_Send` (0x09) — as the trivial pseudo-C expression `esi - esi`
(always 0), which hid the real values. The correction comes from decoding
the raw bytes at the PDB-paired binary: the HEAR branch's `sbb` site is at
VA `0x00570F0A` (mask `-6``0x08` Channel) and the SEND branch's is at VA
`0x00570D4F` (mask `-5``0x09` Channel_Send).
| Channel bit | Prefix retail prints | Type |
|---|---|---|
| `0x0001` Abuse | `[<name>]` | `0x0E` — retail's ONLY 0x0E producer (corrected 2026-08-09) |
| `0x0400` Help | `[<name>]` | `0x0F` |
| `0x0800` Fellowship | `[Fellowship]` | `0x13` |
| `0x1000` Patron / `0x2000` Vassal | `Your patron …` / `Your vassal …` | `0x0A` |
| `0x1000` Patron / `0x2000` Vassal | `Your patron …` / `Your vassal …` | `0x0A` (hear) / `0x0B` (own send) — own-send precision added 2026-08-09 (corrected) |
| `0x4000` Follower/Monarch | `Your follower …` | `0x0A` (hear) / `0x0B` (own send) |
| `0x1000000` Co-Vassals | `[Co-Vassals]` | `0x0A` |
| `0x2000000` Allegiance Broadcast | `[Allegiance Broadcast]` | `0x0A` |
| `0x4000000` | — | `0x13` |
| admin/audit/sentinel channels | `[<name>]` | `0x0E` / `0x0F` |
| `0x4000000` FellowBroadcast | — | `0x08` (hear) / `0x13` (own send) — corrected 2026-08-09, was wrongly `0x13` for both |
| admin/audit/advocate/QA/sentinel/town catch-all | `[<name>]` | `0x08` (hear) / `0x09` (own send) — corrected 2026-08-09, was wrongly `0x0E` / `0x0F` for all of them |
Note the split that surprises people: **legacy allegiance-family chat arrives
as `Social` (`0x0A`, yellow) / `Social_Send` (`0x0B`, dark yellow)**, while