fix(chat): CH3 review fixes — phantom UN-9, allegiance-broadcast echo, /a legacy fallback
Applies the Opus review of Campaign CH slice CH3 (614a1e05):
- B1: UN-9 was a phantom divergence — ACE's CharacterOptions1.cs:47
OR-sum is 0x50C4A54A (its own comment confirms 1355064650), identical
to acdream's literal. The wrong 0x50C48D4A existed only in the research
doc. Row deleted, register §5 reverted to 4 rows, research doc corrected
with dated notes.
- S1/S4: AllegianceBroadcast (0x02000000) is a server-echoing channel —
ACE's GameActionChatChannel handler includes the sender in its real-name
Allegiance.Members broadcast (retail's DoAllegianceBroadcast has no
AddTextToScroll), so the client must skip its local optimistic echo, not
keep it. ChatChannelInfo.Legacy.IsSelfEchoChannel() now returns true for
it; RouteLegacyChannel's comment corrected; Turbine.IsSelfEchoChannel()'s
backwards comment rewritten truthfully.
- S3: retail's /a stays on the legacy AllegianceBroadcast bitflag until
StartupTurbineChatSystem successfully starts Turbine chat — "never
started" (TurbineChatState.Enabled == false) now falls back to legacy in
both LiveSessionCommandRouter.RouteChat and
DirectGameRuntimeCommandAdapter.TrySendChannel, while "enabled but no
allegiance room" still correctly refuses locally.
- S5: added a LiveSessionEventRouter test proving the Options.Replace ->
OnCharacterOptionsChanged seeding order, and RuntimeSettingsTargets /
GameWindowLiveSessionOwnershipTests tests proving the concrete
ICommandBus.Publish wiring and the single LiveSessionCommandSurface
construction site.
- S6: AP-181 rewritten to name both of retail's omitted pre-send checks
(IsMessageSafe silent-drop, then IsMessageSpam) and stop misattributing
either to RouteLegacyChannel, which has no such gates.
- N1-N7: CharacterOptionId moved below SocialActions so its doc comment
re-attaches; TurbineChatMembershipGate reuses TurbineChatDisplayNames
instead of a duplicate table; the gate-to-refusal-text mapping is now
shared via TurbineChatMembershipGate.ResolveRefusalText instead of
duplicated in both hosts; ChatSettings.Default now matches ACE's real
CharacterOptions2.Default (Roleplay/Society start off); a doc-comment
clarifies only the five Hear toggles are server-backed; the register's
§3 header recounted 129 -> 128.
Suite: 11,964 passed / 4 skipped / 0 failed (baseline 11,957/4/0 + 7 new
tests). Campaign ledger CH3 review column updated to APPROVE-WITH-FIXES.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
d3f1c21835
commit
e07fba5731
21 changed files with 582 additions and 153 deletions
File diff suppressed because one or more lines are too long
|
|
@ -945,7 +945,7 @@ the way retail + holtburger expect.
|
|||
- **✓ SHIPPED — I.3 — `LiveCommandBus` + `WorldSession.Send{Talk,Tell,Channel}`.** Replaces `NullCommandBus.Instance` with a real handler-registry `ICommandBus`. New `SendChatCmd` record + `ChatChannelKind` enum + `ChannelResolver` legacy-id mapping (per holtburger). `WorldSession.SendTalk` / `SendTell` / `SendChannel` are 3-line wrappers around existing `ChatRequests.BuildTalk/Tell/ChatChannel`. Commit `8e6e5a0`.
|
||||
- **✓ SHIPPED — I.4 — `ChatPanel` input field + slash commands.** Enter-to-submit input field on `ChatPanel`; `ChatInputParser` recognises `/say` `/t` `/tell` `/r` `/g` `/f` `/a` `/m` `/p` `/v` `/cv` `/lfg` `/trade` `/role` `/society` `/olthoi`; `ChatVM.LastIncomingTellSender` tracks for `/r` reply. `ImGui.WantCaptureKeyboard` already suppresses WASD on input focus. Commit `f14296c`.
|
||||
- **✓ SHIPPED — I.5 — Holtburger inbound chat parity + Windows-1252.** `EmoteText (0x01E0)`, `SoulEmote (0x01E2)`, `ServerMessage (0xF7E0)`, `PlayerKilled (0x019E)` parsers + `WeenieError` routing through `GameEventWiring`. Global string codec switch from `Encoding.ASCII` to `Encoding.GetEncoding(1252)` so accented names round-trip per retail + holtburger. Commit `ff5ed9e`.
|
||||
- **✓ SHIPPED — I.6 — TurbineChat codec + `ChatChannelInfo`.** Full `0xF7DE` codec with three payload variants (`EventSendToRoom`, `RequestSendToRoomById`, `Response`), UTF-16LE strings with variable-length prefix, `SetTurbineChatChannels (0x0295)` parser, unified `ChatChannelInfo` (Legacy + Turbine variants), `TurbineChatState`. **ACE doesn't host a TurbineChat server — codec is ready when retail-emulating servers exist.** Commit `ca968fc`.
|
||||
- **✓ SHIPPED — I.6 — TurbineChat codec + `ChatChannelInfo`.** Full `0xF7DE` codec with three payload variants (`EventSendToRoom`, `RequestSendToRoomById`, `Response`), UTF-16LE strings with variable-length prefix, `SetTurbineChatChannels (0x0295)` parser, unified `ChatChannelInfo` (Legacy + Turbine variants), `TurbineChatState`. **Correction (Campaign CH slice CH3, 2026-08-09): the "ACE doesn't host a TurbineChat server" note above was FALSE — ACE has a complete, on-by-default TurbineChat implementation; see `docs/research/2026-08-09-chat-side-channels-vs-ace.md` §1.** Commit `ca968fc`.
|
||||
- **✓ SHIPPED — I.7 — `CombatChatTranslator`.** Retail-faithful combat-text formatters into `ChatLog` ("You hit drudge for 50 slashing damage (87%)"). Subscribes to visible damage/evasion/miss/kill events; `AttackDone` was removed from chat after named retail + ACE proved its nonzero final status is control-only. Commit `3d26c8e`, corrected 2026-07-11.
|
||||
- **✓ SHIPPED — I.8 — Docs alignment.** Roadmap (this file) + `docs/ISSUES.md` issues #14-#20 closed + `memory/project_chat_pipeline.md` crib + `MEMORY.md` index entry + `CLAUDE.md` UI strategy paragraph all updated to reflect Phase I shipped state. Commit `(this commit)`.
|
||||
|
||||
|
|
|
|||
|
|
@ -108,7 +108,7 @@ implementer per slice against a pinned contract (per
|
|||
| R1–R4 research | `see docs/research/2026-08-09-chat-retail-*` | — | — | — |
|
||||
| CH1 colors | `172c6f9a` | 11,835 passed / 4 skipped / 0 failed | APPROVE-WITH-FIXES; fixed `34d8a3c0` | pending |
|
||||
| CH2 interface text | `77c8296e`, reworked `e0e78883` | 11,916 passed / 4 skipped / 0 failed | REJECT → reworked `e0e78883` → re-review APPROVE-WITH-FIXES → nits `233c30d1` | pending |
|
||||
| CH3 side channels | `614a1e05` | 11,957 passed / 4 skipped / 0 failed | not yet reviewed | pending (connected gate — see handoff below) |
|
||||
| CH3 side channels | `614a1e05` | 11,957 passed / 4 skipped / 0 failed | APPROVE-WITH-FIXES; fixed `(this commit)` | pending (connected gate — see handoff below) |
|
||||
| CH4 commands | — | — | — | — |
|
||||
| CH5 closeout | — | — | — | — |
|
||||
|
||||
|
|
@ -175,6 +175,50 @@ notes above (no 6th Allegiance toggle, no `/allegiancebroadcast` verb) are
|
|||
scope-narrowing decisions made against the retail command registry and the
|
||||
existing `ChatSettings` shape, not skipped work.
|
||||
|
||||
### CH3 Opus review fixes (2026-08-09)
|
||||
|
||||
The review found the CH3 closeout above had three items that no longer
|
||||
match the fixed code — corrections, not a rewrite of the historical
|
||||
record:
|
||||
|
||||
- **Item 5 is now WRONG for AllegianceBroadcast.** ACE's
|
||||
`GameActionChatChannel` handler iterates `player.Allegiance.Members` —
|
||||
the sender IS a member, so they get their own real-name line back
|
||||
through the same broadcast, same as Fellow/Vassals/Patron/Monarch/
|
||||
CoVassals (a different mechanism, same self-echo consequence).
|
||||
`ChatChannelInfo.Legacy.IsSelfEchoChannel()` now returns `true` for
|
||||
`0x02000000` too; `RouteLegacyChannel` skips the local echo for it.
|
||||
- **Item 7 is now WRONG.** `/a` is NOT unconditionally Turbine. Retail's
|
||||
base binding keeps it on the legacy `AllegianceBroadcast` bitflag until
|
||||
`StartupTurbineChatSystem` successfully starts Turbine chat and rebinds
|
||||
it (research doc §4.3). `LiveSessionCommandRouter.RouteChat` and
|
||||
`DirectGameRuntimeCommandAdapter.TrySendChannel` now special-case
|
||||
`TurbineChatState.Enabled == false` to fall back to the legacy send;
|
||||
`Enabled == true` with `AllegianceRoom == 0` still correctly refuses
|
||||
locally ("Turbine chat is not available.").
|
||||
- **Item 9's UN-9 filing was a phantom.** ACE's own
|
||||
`CharacterOptions1.cs:47` OR-sum is `0x50C4A54A` (confirmed by its own
|
||||
inline comment, `// 1355064650`), identical to acdream's
|
||||
`PlayerDescriptionParser.cs:217` — there was never a divergence. The
|
||||
wrong literal `0x50C48D4A` existed only in
|
||||
`docs/research/2026-08-09-chat-side-channels-vs-ace.md`. UN-9 is deleted
|
||||
from the register; AP-181 is rewritten to name BOTH of retail's omitted
|
||||
pre-send checks (`IsMessageSafe` silent-drop, THEN `IsMessageSpam`) and
|
||||
no longer misattributes either to `RouteLegacyChannel`.
|
||||
|
||||
Also fixed this review: `ChatSettings.Default` now matches ACE's real
|
||||
`CharacterOptions2.Default` (Roleplay/Society start OFF, not the
|
||||
previously-claimed "all on"); `TurbineChatMembershipGate` reuses
|
||||
`TurbineChatDisplayNames.Resolve` instead of a second name table; the
|
||||
gate-result-to-refusal-text mapping is shared via
|
||||
`TurbineChatMembershipGate.ResolveRefusalText` instead of being
|
||||
duplicated in both hosts; the `CharacterOptionId` enum in
|
||||
`SocialActions.cs` moved below the class so its doc comment re-attaches
|
||||
correctly; `docs/plans/2026-04-11-roadmap.md` line ~948 got the same
|
||||
TurbineChat-server retraction already applied at line ~429; and the
|
||||
register's §3 header count was corrected from a pre-existing off-by-one
|
||||
(129 claimed vs 128 actual `| AP-` rows).
|
||||
|
||||
**What the connected gate must verify (not run this session — build+test
|
||||
only per the CH3 task's hard constraint):** General/Trade/LFG round-trip
|
||||
send+receive; Roleplay is now silent-but-correctly-refused until the user
|
||||
|
|
|
|||
|
|
@ -300,7 +300,13 @@ and from allegiance changes.
|
|||
| `0x00080000` | HearSocietyChat | **NO** |
|
||||
|
||||
`Default = 0x00948700`. `CharacterOptions1.Default` includes
|
||||
`HearAllegianceChat (0x40000000)`; `CharacterOptions1.Default = 0x50C48D4A`.
|
||||
`HearAllegianceChat (0x40000000)`; `CharacterOptions1.Default = 0x50C4A54A`
|
||||
(corrected 2026-08-09 at the CH3 Opus review — the original filing here
|
||||
had `0x50C48D4A`, a copy error with no basis in ACE's own source; ACE's
|
||||
`CharacterOptions1.cs:47` OR-sum is `0x50C4A54A`, confirmed by its own
|
||||
inline comment `// 1355064650`, and is identical to acdream's
|
||||
`PlayerDescriptionParser.cs:217`. This wrong literal is what filed the
|
||||
now-retracted UN-9 register row).
|
||||
`PlayerFactory.CharacterCreateSetDefaultCharacterOptions` sets exactly these
|
||||
two defaults on every new character.
|
||||
|
||||
|
|
@ -620,7 +626,9 @@ unknown channel ids are dropped with no client-visible response.
|
|||
as `flags`, then reads `characterOptions1` past the end of the payload.
|
||||
Worse, `CharacterOptionDataFlag.CharacterOptions2 = 0x40` collides with
|
||||
`CharacterOptions1.AllowGive = 0x40`, which **is** set in
|
||||
`CharacterOptions1.Default (0x50C48D4A)` — so ACE would also try to read an
|
||||
`CharacterOptions1.Default (0x50C4A54A)` (corrected 2026-08-09 at the CH3
|
||||
Opus review; see the §3.5 correction note above — same copy error, same
|
||||
fix) — so ACE would also try to read an
|
||||
options2 value. Reachable only via `IGameRuntimeCommands.SetOptions1`, which has
|
||||
no production call site (grep: only tests). **Latent, but this is the exact
|
||||
message a Settings-sync feature would reach for.**
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue