From b633b10967a4637ece39f25055b57d53eae4615d Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 3 Aug 2026 22:20:31 +0200 Subject: [PATCH] fix(chat): display retail's text for WeenieError 0x0504 and three PK siblings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User saw "[System] WeenieError 0x0504" on login after a PK Lite reversion. 0x0504 is YouAreNonPKAgain; only ~56 of 378 codes had strings, so the raw hex fallback fired. Retail's source is ClientCommunicationSystem::HandleFailureEvent @0x00571990 — a switch with per-case literal UTF-16 strings, not a DAT string-table lookup, so hardcoding them is retail-faithful. The decomp could not be trusted for the text. Its dump of data_7d32c0 declares [0x5f] and shows 95 characters ending mid-word at "...protection of the Lig". The real string is 139 characters. The 0x5f is Binary Ninja's PREVIEW TRUNCATION LENGTH, not the array size — worth remembering for the rest of the switch, since a copy-paste from the dump would have shipped a truncated sentence. Recovered by PE byte read (VA 0x007D32C0 -> RVA -> .rdata file offset), cross-confirmed against the raw hex the pseudo-C carries immediately after the preview. Mapped 0x0504, 0x0505, 0x04EC, 0x04ED, each byte-verified and cited with its case address. Retail's trailing newline is dropped deliberately (documented in-comment): acdream renders one ChatEntry per system message where retail has a single scrolling buffer. Adjacent codes are deliberately left unmapped with a test pinning that 0x04EE still falls back to hex — a wrong message is worse than a raw code. Files #306 for the full port, with three findings that make it more than a string table: the switch is SIX compiler-lowered blocks spanning 339 distinct case values from 0x17 to 0x593, not one contiguous band; retail passes a colour argument with three values in use (0 x162, 0x1a x113, 7 x59) and acdream's chat has no colour concept; and HandleFailureEvent aborts an in-progress automatic attack on 0x43/0x3f7/0x3e/0x23/0x36 — verified against the decomp, with the nuance that 0x43 has no display case at all and is abort-only, so that one is a pure gameplay gap. Gates: complete Release solution 10,909 passed / 4 skipped / 0 failed (baseline 10,904; +5 = the five new tests). Co-Authored-By: Claude Opus 5 --- docs/ISSUES.md | 136 ++++++++++++++++++ src/AcDream.Core/Chat/WeenieErrorMessages.cs | 20 +++ .../Chat/WeenieErrorMessagesTests.cs | 55 +++++++ 3 files changed, 211 insertions(+) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index c9bd4ae7..e6aa2833 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -627,6 +627,142 @@ it. Do #297 FIRST — #298 depends on it. `FindClosest`/auto-acquisition on the narrow policy. Filed from the #298 review. +## WeenieError message-mapping coverage — 2026-08-03 + +- **#306 — OPEN — `WeenieErrorMessages` should map every code retail's + `HandleFailureEvent` switch displays, not accumulate a bigger partial + table. MEDIUM–LARGE (string-table port + two structural gaps).** + Filed after fixing #504 (`YouAreNonPKAgain`, "WeenieError 0x0504" shown + on a PK Lite status reversion) by hand-recovering four strings; that + session also mapped the retail switch's true shape, which changes the + scope of "finish this" considerably from what a quick read suggests. + + **The goal is complete coverage, not a bigger partial map.** Today + `src/AcDream.Core/Chat/WeenieErrorMessages.cs` maps 60 of 378 + `WeenieError` enum values (`src/AcDream.Core/Physics/WeenieError.cs`); + the raw `WeenieError 0xNNNN[: param]` fallback is the norm for any code + a player actually triggers outside chat/channel/tell/allegiance + chatter. Target state: `WeenieErrorMessages` handles every code + retail's switch handles, and the raw fallback is a genuine last resort + for codes retail itself never displays — not a "we haven't gotten to + it yet" placeholder. + + **Authoritative source + extraction method.** The complete display + switch is `ClientCommunicationSystem::HandleFailureEvent` @0x00571990, + decompiled at + `docs/research/named-retail/acclient_2013_pseudo_c.txt:382616`+. Each + case constructs a literal UTF-16 string and calls + `ClientSystem::AddTextToScroll`. **The pseudo-C dump truncates long + string literals at their declared array bound mid-sentence** (e.g. + `data_7d32c0`'s declared `wchar16 const [0x5f]` stops at "...protection + of the Lig", but the real null-terminated string in the binary is 139 + chars, not 95). The exact text must be recovered from the PDB-paired + binary at `C:\Users\erikn\Downloads\acclient.exe` (imagebase + 0x400000, verify pairing with `tools/pdb-extract/check_exe_pdb.py` + first) via VA → RVA → file-offset mapping — the technique is written + up in `claude-memory/reference_pe_byte_decode.md`. **Guessing a string + ending is not acceptable; an unrecoverable string must stay unmapped + rather than be approximated** — a wrong message is worse than a raw + hex code, because it reads as authoritative when it isn't. (Some + strings are NOT truncated — the dump appends `, 0` right after the + closing quote when the declared array bound exactly equals string + length + 1; those can be transcribed directly without touching the + binary. Truncation is only a risk when there's no trailing `, 0`.) + + **Scope the switch precisely — it is not one contiguous band.** + Binary Ninja renders the compiled switch as **(at least) six separate + `switch (arg2) { ... }` blocks**, all reported at the same decompiler + block address `0x00571dc5` — a decompiler/codegen artifact from the + compiler lowering one sparse switch into several contiguous + range-checked jump tables chained together (each guarded by its own + `if (arg2 > X) { if ((arg2 - Y) <= Z) switch(arg2) {...} }`). One + example, confirmed by dumping its jump table + (`acclient_2013_pseudo_c.txt:385851-385971`): `arg2 > 0x4e8` then + `(arg2 - 0x4e9) <= 0xaa` selects a 171-entry table covering codes + `0x4e9`–`0x593`. That is only the *highest* of the six sub-switches — + do not mistake it for the whole thing. A mechanical scan of the full + function body (`grep -oE "case 0x[0-9a-f]+"` over + lines 382616–385971) found **339 distinct case values**, spanning + `0x17`–`0x593` in 41 contiguous runs with gaps between them. Codes + that appear in none of the six sub-switches (and are not one of the + five `AbortAutomaticAttack`-only codes below) are the legitimate + fallback set — retail truly does not display them via this path. This + 339-code figure was obtained by grep, not exhaustive verification + against our enum's numeric values (see verification note below) — + treat it as a strong estimate, not a final count. + + **Three structural gaps mean this is not a pure string table:** + + 1. **Per-case colour.** Every case calls + `ClientSystem::AddTextToScroll(this, &str, , 1, 0)` with a + colour argument — a scan of the whole function found exactly three + distinct values in use: `0` (162 call sites), `0x1a` (113 call + sites), `7` (59 call sites). `WeenieErrorMessages.Format` has no + colour concept today; `ChatLog.OnWeenieError` emits a plain + `ChatEntry` with `Kind: ChatKind.System` and no colour field. + Before porting the full table, check whether `ChatEntry`/`ChatVM` + can express a per-entry colour at all — if not, that's a + prerequisite sub-task, not a detail to skip. + 2. **`AbortAutomaticAttack` side effect — verified true.** + `HandleFailureEvent` opens with: + ``` + if (arg2 == 0x43 || arg2 == 0x3f7) goto label_5719de; + if (arg2 == 0x3e || arg2 == 0x23 || arg2 == 0x36) goto label_5719de; + label_5719de: + if (ClientCombatSystem::GetCombatSystem() != 0 && + ClientCombatSystem::RepeatAttackInProgress(...) != 0) + ClientCombatSystem::AbortAutomaticAttack(...); + ``` + (`acclient_2013_pseudo_c.txt:382625-382630`, block + `0x005719b1`-`0x005719fe`). Confirmed against our enum: + `0x0043=NoMtableData`, `0x03F7=YouAreTooFatiguedToAttack`, + `0x003E=YouAreTooTiredToDoThat`, `0x0023=MotionFailure`, + `0x0036=ActionCancelled` — all combat-failure codes, so the + behaviour is coherent (stop swinging on a swing-failure error). + Also confirmed: this check runs *before*, and independently of, + the display switches — `0x43` has no case label anywhere in the + 339-value scan (abort-only, no scroll text), while the other four + also have their own display cases further down (so they both show + text *and* aborted an in-progress auto-attack). acdream has no + equivalent hook — an auto-attack that should stop on any of these + five errors currently keeps swinging. This is a real gameplay gap, + not cosmetics, and should land as its own sub-task (find + acdream's repeat-attack/auto-attack state — likely + `RuntimeActionState`'s combat-attack owner per J5.3 — and the + equivalent abort call) rather than be bundled silently into the + string-table commit. + 3. **`%s`-parameterised (WithString) cases.** A meaningful fraction of + the 339 cases build their string via + `PStringBase::sprintf(..., u"...%s...")` using + `arg3->m_charbuffer` (the event's string parameter) rather than a + bare literal — e.g. case `0x4e9`/`0x4ea` (`acclient_2013_pseudo_c.txt:382636-382653`). + These map to `WeenieErrorWithString` on our side + (`WeenieErrorMessages.WithStringTemplates`, `_`-placeholder + convention). When porting, classify each case as plain-literal + (→ `NoParamTemplates`) or `%s`-templated (→ `WithStringTemplates`) + — do not assume the split matches `WeenieError` vs + `WeenieErrorWithString` enum membership 1:1 without checking, since + ACE's wire dispatch and retail's switch are independently + authored. + + **Verification shape (378 hand-transcribed strings is exactly where + silent typos live).** Recommended before merging the full port: + - A test that iterates every code `WeenieErrorMessages` claims to + map and asserts `Format(code, ...)` never returns the + `WeenieError 0x...` fallback form (catches accidental + no-ops/typo'd dictionary keys). + - A mechanical cross-check against the extracted data rather than + eyeballing: e.g. a one-off script that walks the PE bytes for every + `data_7dXXXX` address referenced by a `case` in the six sub-switches + and asserts the transcribed C# literal matches the recovered bytes + exactly (reusing the VA→offset routine from the #504 fix). Treat any + mismatch as a hard stop, not a judgment call. + + Prior art from this session: `src/AcDream.Core/Chat/WeenieErrorMessages.cs` + (0x0504/0x0505/0x04EC/0x04ED, each cited with its case address and + `data_7dXXXX` symbol) and + `tests/AcDream.Core.Tests/Chat/WeenieErrorMessagesTests.cs`. + ## C3c placement cutover — 2026-08-02 - **#276 — OPEN — SpawnPlacementSettler discards the settle's resolved diff --git a/src/AcDream.Core/Chat/WeenieErrorMessages.cs b/src/AcDream.Core/Chat/WeenieErrorMessages.cs index a238922e..daaa85b5 100644 --- a/src/AcDream.Core/Chat/WeenieErrorMessages.cs +++ b/src/AcDream.Core/Chat/WeenieErrorMessages.cs @@ -121,6 +121,26 @@ public static class WeenieErrorMessages // Player flavour [0x0526] = "You chicken out.", // YouChickenOut + + // PK status (retail: ClientCommunicationSystem::HandleFailureEvent + // @0x00571990, decompiled at + // docs/research/named-retail/acclient_2013_pseudo_c.txt:382900+. + // Strings recovered from the PDB-paired + // C:\Users\erikn\Downloads\acclient.exe (imagebase 0x400000) via + // VA -> RVA -> file-offset mapping per + // claude-memory/reference_pe_byte_decode.md, because the pseudo-C + // dump for data_7d32c0 truncates at its declared array bound + // (0x5f wchar16) mid-sentence. Retail's literal ends with a + // trailing "\n" that we drop here — acdream's ChatEntry already + // renders one line per system message, unlike retail's single + // scrolling text buffer. + [0x0504] = "You are enveloped in a feeling of warmth as you are brought back into the protection of the Light. You are once again a Non-Player Killer.", + // YouAreNonPKAgain, case 0x504 @0x005745cd, data_7d32c0 + [0x0505] = "You're too close to your sanctuary!", // YoureTooCloseToYourSanctuary, case 0x505 @0x00574c65, data_7d2640 + [0x04EC] = "You cannot modify your player killer status while you are recovering from a PK death.", + // CannotChangePKStatusWhileRecovering, case 0x4ec @0x0057446f, data_7d3820 + [0x04ED] = "Advocates may not change their player killer status!", + // AdvocatesCannotChangePKStatus, case 0x4ed @0x005744a1, data_7d37b0 }; /// diff --git a/tests/AcDream.Core.Tests/Chat/WeenieErrorMessagesTests.cs b/tests/AcDream.Core.Tests/Chat/WeenieErrorMessagesTests.cs index 3e929a20..be9fd72a 100644 --- a/tests/AcDream.Core.Tests/Chat/WeenieErrorMessagesTests.cs +++ b/tests/AcDream.Core.Tests/Chat/WeenieErrorMessagesTests.cs @@ -104,6 +104,61 @@ public sealed class WeenieErrorMessagesTests public void Format_CombatMovementErrors(uint code, string expected) => Assert.Equal(expected, WeenieErrorMessages.Format(code, null)); + // ── PK status codes ─────────────────────────────────────────────── + + [Fact] + public void Format_YouAreNonPKAgain_ExactRetailText() + { + // 0x0504 = WeenieError.YouAreNonPKAgain. Filed after the user saw + // "WeenieError 0x0504" on login following a PK Lite status + // reversion. Retail: ClientCommunicationSystem::HandleFailureEvent + // @0x00571990 case 0x504 @0x005745cd; string recovered byte-exact + // from data_7d32c0 in the PDB-paired binary (the pseudo-C dump + // truncates at the declared 0x5f-wchar16 array bound, mid-sentence + // at "...protection of the Lig"). + Assert.Equal( + "You are enveloped in a feeling of warmth as you are brought back into the protection of the Light. You are once again a Non-Player Killer.", + WeenieErrorMessages.Format(0x0504, null)); + } + + [Fact] + public void Format_YoureTooCloseToYourSanctuary() + { + // 0x0505, case 0x505 @0x00574c65, data_7d2640. + Assert.Equal( + "You're too close to your sanctuary!", + WeenieErrorMessages.Format(0x0505, null)); + } + + [Fact] + public void Format_CannotChangePKStatusWhileRecovering() + { + // 0x04EC, case 0x4ec @0x0057446f, data_7d3820. + Assert.Equal( + "You cannot modify your player killer status while you are recovering from a PK death.", + WeenieErrorMessages.Format(0x04EC, null)); + } + + [Fact] + public void Format_AdvocatesCannotChangePKStatus() + { + // 0x04ED, case 0x4ed @0x005744a1, data_7d37b0. + Assert.Equal( + "Advocates may not change their player killer status!", + WeenieErrorMessages.Format(0x04ED, null)); + } + + [Fact] + public void Format_AdjacentUnmappedPKCode_StillFallsBackToHex() + { + // 0x04EE (LevelTooLowToChangePKStatusWithObject) sits right next + // to the codes above but its retail literal + // ("Your level is too low to change…") was NOT independently + // byte-recovered in this pass — confirms the fallback still + // covers the untouched neighbours rather than silently guessing. + Assert.Equal("WeenieError 0x04EE", WeenieErrorMessages.Format(0x04EE, null)); + } + // ── unknown codes — graceful fallback preserves debug info ─────── [Fact]