fix(chat): display retail's text for WeenieError 0x0504 and three PK siblings
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 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
2f0c26c8a4
commit
b633b10967
3 changed files with 211 additions and 0 deletions
136
docs/ISSUES.md
136
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, <colour>, 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<unsigned short>::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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
};
|
||||
|
||||
/// <summary>
|
||||
|
|
|
|||
|
|
@ -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]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue