From 6a32f37589ebda85d8149a0a3465d37c47585d68 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 14 Aug 2026 15:46:48 +0200 Subject: [PATCH 1/3] =?UTF-8?q?feat(net):=20Campaign=20LA=20LA7a=20?= =?UTF-8?q?=E2=80=94=20CharacterDelete/CharacterRestore/CharacterError=20w?= =?UTF-8?q?ire=20messages?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports the three character-management wire messages LA7 (design spec §7, plan §11 item 4) identified as missing before the character-select screen (LA8) can be built: delete, restore, and the server error channel. Message types + tests only — no WorldSession/Runtime/UI wiring, that is LA7b. CharacterDelete (0xF655): outbound account+SLOT-INDEX request per Proto_UI::SendDeleteCharacter@0x00546b30 (retail packs the account as String16L then writes the trailing u32 directly after — NOT the character guid; CPlayerSystem::DeleteCharacter@0x0055f830 resolves that slot via CharacterSet::GetSlot before sending). The server's ack reuses the same opcode with an empty body (ACE GameMessageCharacterDelete.cs); a fresh CharacterList follows separately per CharacterHandler.cs:322 — that refresh flow is explicitly out of scope here (LA7b). CharacterRestore (0xF7D9 request / 0xF643 response): guid-only request, per ACE (CharacterHandler.cs:331-385, ReadUInt32 only) and holtburger (CharacterRestoreRequestData, guid-only) independent consensus. The decompiled call site (Proto_UI::SendAdminRestoreCharacter@0x00546cf0) appears to pack two extra strings, but its only caller (CPlayerSystem::RestoreCharacter@0x0055d760) passes an uninitialized local (`class PStringBase* edx;`, never assigned) as the second argument and `this` (a CPlayerSystem*, not a string) as the third — textbook decompiler register-corruption, not real arguments. No divergence-register row: this follows the correct reading of a corrupted decompile, not a deviation from retail (spec §11 item 4). The response reuses opcode 0xF643, a genuine retail collision with CharacterCreateResponse (ACE's own comment: "This is a duplicate...", GameMessageOpcode.cs:42); GameMessageCharacterRestore.cs always writes a success shape (flag=1 + guid + name + secondsGreyedOut), but retail's CharacterRestore handler can also reply via the CharacterCreateResponse path on failure (e.g. NameInUse) with a flag-only body and no trailing fields — the parser mirrors that conditionality instead of assuming the four fields are always present. CharacterError (0xF659): u32 error code, confirmed directly from retail's inbound dispatcher UIQueueManager::ProcessNetBlobData@0x0055b000 -> CPlayerSystem::Handle_CharacterError@0x0055d5d0, which reads `enum charError` straight off the wire. The Code enum is a verbatim port of retail's own enum charError (docs/research/named-retail/acclient.h: 4038-4067, 26 members incl. CHAR_ERROR_NUM_ERRORS) rather than a subset filtered through ACE — retail's header names four members ACE's C# CharacterError enum omits (LoggedOn, NoPremade, AccountInUse, CharacterIsBooted) because ACE's server never sends them, though a genuine retail server could. The 32-bit storage-width compiler sentinel FORCE_charError_32_BIT is deliberately excluded (not a real value). Unknown codes never throw — RawErrorCode always preserves the wire value. Today acdream cannot surface any character-stage server error; this is the first parser for the family. 46 new tests (byte-exact builder assertions, ACE-serializer-shaped parser fixtures via the existing AceWireWriter test helper, all 26 retail error codes round-tripped, unknown/truncated/wrong-opcode handling). Full Core.Net.Tests suite: 951 passed, 0 failed, 0 skipped. Release build green. Co-Authored-By: Claude Fable 5 --- .../Messages/CharacterDelete.cs | 82 ++++++ .../Messages/CharacterError.cs | 270 ++++++++++++++++++ .../Messages/CharacterRestore.cs | 140 +++++++++ .../Messages/CharacterDeleteTests.cs | 83 ++++++ .../Messages/CharacterErrorTests.cs | 113 ++++++++ .../Messages/CharacterRestoreTests.cs | 123 ++++++++ 6 files changed, 811 insertions(+) create mode 100644 src/AcDream.Core.Net/Messages/CharacterDelete.cs create mode 100644 src/AcDream.Core.Net/Messages/CharacterError.cs create mode 100644 src/AcDream.Core.Net/Messages/CharacterRestore.cs create mode 100644 tests/AcDream.Core.Net.Tests/Messages/CharacterDeleteTests.cs create mode 100644 tests/AcDream.Core.Net.Tests/Messages/CharacterErrorTests.cs create mode 100644 tests/AcDream.Core.Net.Tests/Messages/CharacterRestoreTests.cs diff --git a/src/AcDream.Core.Net/Messages/CharacterDelete.cs b/src/AcDream.Core.Net/Messages/CharacterDelete.cs new file mode 100644 index 00000000..6fa07037 --- /dev/null +++ b/src/AcDream.Core.Net/Messages/CharacterDelete.cs @@ -0,0 +1,82 @@ +using System.Buffers.Binary; +using AcDream.Core.Net.Packets; + +namespace AcDream.Core.Net.Messages; + +/// +/// Retail character-delete request and server acknowledgement, both riding +/// opcode 0xF655. +/// +/// +/// Wire layout ported from retail Proto_UI::SendDeleteCharacter at +/// 0x00546b30: the opcode, then AC1Legacy::PStringBase<char>::Pack +/// of the account id as a String16L, then a trailing u32 written directly +/// after the packed string (*(uint32_t*)var_4 = arg2): +/// +/// +/// +/// u32 opcode (0xF655) +/// String16L accountName +/// u32 characterSlot (NOT the character guid) +/// +/// +/// +/// The caller, CPlayerSystem::DeleteCharacter at 0x0055f830, +/// resolves that trailing u32 from the target character's guid via +/// CharacterSet::GetSlot(persistentData + 4, guid) before sending — +/// retail deletes by **account + SLOT INDEX**, never the character guid. +/// This builder takes the already-resolved slot; resolving a selected +/// character to its slot is Runtime selection-state work (Campaign LA +/// slice LA7b), not this file's job. +/// +/// +/// +/// The server's acknowledgement reuses the same opcode with no trailing +/// payload — ACE's GameMessageCharacterDelete constructs a bare +/// 4-byte body +/// (ACE.Server/Network/GameMessages/Messages/GameMessageCharacterDelete.cs, +/// base constructor called with bodyLength: 4 and no further +/// Writer.Write calls). holtburger's inbound dispatcher +/// (holtburger-protocol/src/messages/game_message/unpack.rs:50-58) +/// disambiguates request vs. ack the identical way we do here — a request +/// has bytes remaining after the opcode, the ack does not. +/// +/// +/// +/// After the ack, ACE immediately follows with a fresh +/// so the roster reflects the character's new pending-delete state +/// (CharacterHandler.CharacterDelete, +/// ACE.Server/Network/Handlers/CharacterHandler.cs:322, inside the +/// SaveCharacter success callback). Requesting and re-rendering that +/// refreshed roster belongs to LA7b's Runtime selection state — this file +/// only builds the request and recognizes the ack. +/// +/// +public static class CharacterDelete +{ + public const uint Opcode = 0xF655u; + + /// + /// Build the body bytes for an outbound CharacterDelete request. + /// Layout: opcode(4) + String16L(accountName) + characterSlot(4). + /// + public static byte[] BuildRequestBody(string accountName, uint characterSlot) + { + ArgumentNullException.ThrowIfNull(accountName); + var w = new PacketWriter(32); + w.WriteUInt32(Opcode); + w.WriteString16L(accountName); + w.WriteUInt32(characterSlot); + return w.ToArray(); + } + + /// + /// Returns whether a complete game-message body is the server's + /// delete acknowledgement — the canonical four-byte opcode-only form + /// ACE emits. A fresh follows separately + /// and is not this method's concern. + /// + public static bool IsAcknowledgement(ReadOnlySpan body) => + body.Length == sizeof(uint) && + BinaryPrimitives.ReadUInt32LittleEndian(body) == Opcode; +} diff --git a/src/AcDream.Core.Net/Messages/CharacterError.cs b/src/AcDream.Core.Net/Messages/CharacterError.cs new file mode 100644 index 00000000..7d2c70ce --- /dev/null +++ b/src/AcDream.Core.Net/Messages/CharacterError.cs @@ -0,0 +1,270 @@ +using System.Buffers.Binary; + +namespace AcDream.Core.Net.Messages; + +/// +/// Inbound CharacterError GameMessage (opcode 0xF659) — the +/// server's catch-all failure notice during the pre-world character-select +/// stage (logon conflicts, delete/restore failures, enter-world rejections, +/// subscription state). Today acdream cannot surface ANY character-stage +/// server error to the user; this is the first parser for the family. +/// +/// +/// Wire layout confirmed directly from retail's inbound dispatcher, +/// UIQueueManager::ProcessNetBlobData at 0x0055b000, which +/// reads a u32 immediately after the opcode and passes it to +/// CPlayerSystem::Handle_CharacterError at 0x0055d5d0 typed +/// as enum charError (enum charError eax_86 = *(uint32_t*)((char*)ecx + 4);): +/// +/// +/// +/// u32 opcode (0xF659) +/// u32 errorCode (enum charError) +/// +/// +/// +/// ACE agrees: GameMessageCharacterError +/// (ACE.Server/Network/GameMessages/Messages/GameMessageCharacterError.cs) +/// writes exactly opcode + (uint)error, and every +/// session.SendCharacterError(...) call site in +/// CharacterHandler.cs (the two this slice's +/// / handlers can raise — +/// CharacterError.Delete, CharacterError.LogonServerFull, +/// CharacterError.EnterGameCouldntPlaceCharacter, +/// CharacterError.EnterGameCharacterNotOwned — plus every other +/// value the wider character-stage flow can raise) goes through this same +/// shape. +/// +/// +/// +/// is a verbatim port of retail's enum charError +/// (docs/research/named-retail/acclient.h:4038-4067) — the header's +/// own numeric ground truth, not a subset filtered through ACE's C# port. +/// It is a strict superset of ACE's ACE.Server.Network.Enum.CharacterError +/// (references/ACE/Source/ACE.Server/Network/Enum/CharacterError.cs): +/// retail additionally names 0x2 (LoggedOn), 0x7 (NoPremade), +/// 0x8 (AccountInUse), and 0x16 (CharacterIsBooted), none of +/// which ACE's server ever sends but all of which retail's client can +/// receive from a genuine retail server — per the project's +/// property-enum-divergence lesson, we port the complete oracle, not just +/// what today's one server implementation emits. ACE's per-value doc +/// comments (themselves sourced from the client's ID_CHAR_ERROR_* +/// string table) are folded in below where they exist. One retail member, +/// FORCE_charError_32_BIT = 0x7FFFFFFF, is a compiler +/// storage-width pragma (MSVC's "force this enum to 32-bit backing store" +/// idiom) and not a real wire value — it is deliberately NOT ported. +/// +/// +/// +/// Unknown values are never rejected: +/// always carries the wire value verbatim, and casting it to +/// (see ) can never throw in +/// C# even for a value retail itself never defined — future server +/// revisions or private servers may add codes we haven't named yet. +/// +/// +public static class CharacterError +{ + public const uint Opcode = 0xF659u; + + /// + /// Verbatim port of retail's enum charError + /// (acclient.h:4038-4067), excluding the 32-bit storage-width + /// sentinel FORCE_charError_32_BIT. + /// + public enum Code : uint + { + /// 0x00 — CHAR_ERROR_UNDEF. + Undefined = 0x00, + + /// + /// 0x01 — CHAR_ERROR_LOGON. ACE: "Cannot have two accounts logged + /// on at the same time." + /// + Logon = 0x01, + + /// 0x02 — CHAR_ERROR_LOGGED_ON. Retail-only; no ACE member. + LoggedOn = 0x02, + + /// + /// 0x03 — CHAR_ERROR_ACCOUNT_LOGON. ACE: "Server could not access + /// your account information. Please try again in a few minutes." + /// + AccountLogon = 0x03, + + /// + /// 0x04 — CHAR_ERROR_SERVER_CRASH. ACE: "The server has + /// disconnected. Please try again in a few minutes." + /// + ServerCrash = 0x04, + + /// 0x05 — CHAR_ERROR_LOGOFF. ACE: "Server could not log off your character." + Logoff = 0x05, + + /// + /// 0x06 — CHAR_ERROR_DELETE. ACE: "Server could not delete your + /// character." Sent by 's + /// server-side handler on every rejection path. + /// + Delete = 0x06, + + /// 0x07 — CHAR_ERROR_NO_PREMADE. Retail-only; no ACE member. + NoPremade = 0x07, + + /// 0x08 — CHAR_ERROR_ACCOUNT_IN_USE. Retail-only; no ACE member. + AccountInUse = 0x08, + + /// + /// 0x09 — CHAR_ERROR_ACCOUNT_INVALID. ACE: "The account name you + /// specified was not valid." + /// + AccountInvalid = 0x09, + + /// + /// 0x0A — CHAR_ERROR_ACCOUNT_DOESNT_EXIST. ACE: "The account you + /// specified doesn't exist." + /// + AccountDoesntExist = 0x0A, + + /// + /// 0x0B — CHAR_ERROR_ENTER_GAME_GENERIC. ACE: forces the player + /// back to character-select if in 3D mode; otherwise a no-op OK + /// popup. + /// + EnterGameGeneric = 0x0B, + + /// + /// 0x0C — CHAR_ERROR_ENTER_GAME_STRESS_ACCOUNT. ACE: "You cannot + /// enter the game with a stress creating character." + /// + EnterGameStressAccount = 0x0C, + + /// + /// 0x0D — CHAR_ERROR_ENTER_GAME_CHARACTER_IN_WORLD. ACE: "One of + /// your characters is still in the world. Please try again in a + /// few minutes." + /// + EnterGameCharacterInWorld = 0x0D, + + /// + /// 0x0E — CHAR_ERROR_ENTER_GAME_PLAYER_ACCOUNT_MISSING. ACE: + /// "Server unable to find player account. Please try again + /// later." + /// + EnterGamePlayerAccountMissing = 0x0E, + + /// + /// 0x0F — CHAR_ERROR_ENTER_GAME_CHARACTER_NOT_OWNED. ACE: "You do + /// not own this character." Sent by + /// 's + /// server-side handler when the delete grace window has expired. + /// + EnterGameCharacterNotOwned = 0x0F, + + /// + /// 0x10 — CHAR_ERROR_ENTER_GAME_CHARACTER_IN_WORLD_SERVER. ACE: + /// "One of your characters is currently in the world. Please try + /// again later. This is likely an internal server error." + /// + EnterGameCharacterInWorldServer = 0x10, + + /// + /// 0x11 — CHAR_ERROR_ENTER_GAME_OLD_CHARACTER. ACE: forces the + /// player back to character-select if in 3D mode; no-op + /// otherwise. + /// + EnterGameOldCharacter = 0x11, + + /// + /// 0x12 — CHAR_ERROR_ENTER_GAME_CORRUPT_CHARACTER. ACE: "This + /// character's data has been corrupted. Please delete it and + /// create a new character." + /// + EnterGameCorruptCharacter = 0x12, + + /// + /// 0x13 — CHAR_ERROR_ENTER_GAME_START_SERVER_DOWN. ACE: "This + /// character's starting server is experiencing difficulties. + /// Please try again in a few minutes." + /// + EnterGameStartServerDown = 0x13, + + /// + /// 0x14 — CHAR_ERROR_ENTER_GAME_COULDNT_PLACE_CHARACTER. ACE: + /// "This character couldn't be placed in the world right now. + /// Please try again in a few minutes." Sent by + /// 's + /// server-side handler during a shutdown-in-progress race. + /// + EnterGameCouldntPlaceCharacter = 0x14, + + /// + /// 0x15 — CHAR_ERROR_LOGON_SERVER_FULL. ACE: "Sorry, but the + /// Asheron's Call server is full currently. Please try again + /// later." Sent by both + /// and + /// 's + /// server-side handlers when the world is closed to non-advocates. + /// + LogonServerFull = 0x15, + + /// 0x16 — CHAR_ERROR_CHARACTER_IS_BOOTED. Retail-only; no ACE member. + CharacterIsBooted = 0x16, + + /// + /// 0x17 — CHAR_ERROR_ENTER_GAME_CHARACTER_LOCKED. ACE: "A save of + /// this character is still in progress. Please try again later." + /// + EnterGameCharacterLocked = 0x17, + + /// + /// 0x18 — CHAR_ERROR_SUBSCRIPTION_EXPIRED. ACE: "Your + /// subscription to this game has expired." + /// + SubscriptionExpired = 0x18, + + /// + /// 0x19 — CHAR_ERROR_NUM_ERRORS. Retail's own count-of-errors + /// sentinel (the array-bound idiom, one past the last real code) — + /// never sent on the wire as an actual error. Kept for verbatim + /// completeness of the enum range; do not treat a received 0x19 + /// as meaningful. + /// + NumErrors = 0x19, + } + + public readonly record struct Parsed(uint RawErrorCode) + { + /// + /// Best-effort named view of . A plain + /// enum cast never throws in C#, so this is safe even for values + /// retail never defined — always trust + /// as the source of truth. + /// + public Code AsCode => (Code)RawErrorCode; + } + + /// + /// Parse a CharacterError body. must start + /// with the 4-byte opcode (0xF659). + /// + public static Parsed Parse(ReadOnlySpan body) + { + int pos = 0; + + uint opcode = ReadU32(body, ref pos); + if (opcode != Opcode) + throw new FormatException($"expected CharacterError opcode 0x{Opcode:X4}, got 0x{opcode:X8}"); + + uint errorCode = ReadU32(body, ref pos); + return new Parsed(errorCode); + } + + private static uint ReadU32(ReadOnlySpan source, ref int pos) + { + if (source.Length - pos < 4) throw new FormatException("truncated u32"); + uint value = BinaryPrimitives.ReadUInt32LittleEndian(source.Slice(pos)); + pos += 4; + return value; + } +} diff --git a/src/AcDream.Core.Net/Messages/CharacterRestore.cs b/src/AcDream.Core.Net/Messages/CharacterRestore.cs new file mode 100644 index 00000000..a40858e2 --- /dev/null +++ b/src/AcDream.Core.Net/Messages/CharacterRestore.cs @@ -0,0 +1,140 @@ +using System.Buffers.Binary; +using AcDream.Core.Net.Packets; + +namespace AcDream.Core.Net.Messages; + +/// +/// Retail character-restore request (opcode 0xF7D9) and its response +/// (opcode 0xF643). +/// +/// +/// Request — guid-only, by reference consensus. The decompiled call +/// site (Proto_UI::SendAdminRestoreCharacter at 0x00546cf0, +/// declared with three parameters — a u32 and two PStringBase<char> +/// pointers — and packing two strings after the u32) LOOKS like it sends +/// guid + two strings. It does not: its only real caller, +/// CPlayerSystem::RestoreCharacter at 0x0055d760, declares +/// class PStringBase<char>* edx; as a local and passes it +/// straight through UNINITIALIZED as the second argument, and passes +/// this (a CPlayerSystem*, not a string) as the third. Both +/// are textbook decompiler register-corruption artifacts (uninitialized +/// register reuse + a mistyped extra parameter from an over-declared +/// callee signature), not real arguments the real call site ever +/// supplied. ACE +/// (CharacterHandler.CharacterRestore, +/// ACE.Server/Network/Handlers/CharacterHandler.cs:331-385, reads +/// only ReadUInt32()) and holtburger +/// (holtburger-protocol/src/messages/character/types.rs::CharacterRestoreRequestData, +/// guid-only) independently agree on guid-only. We follow the two +/// independent, uncorrupted references (design spec §11 item 4 — wire +/// consensus, no divergence-register row needed: this isn't a deviation +/// from retail, it's picking the correct reading of a corrupted decompile). +/// +/// +/// +/// u32 opcode (0xF7D9) +/// u32 characterGuid +/// +/// +/// +/// Response — opcode collision with CharacterCreateResponse. ACE's +/// own GameMessageOpcode.cs declares both +/// CharacterCreateResponse = 0xF643 and +/// CharacterRestoreResponse = 0xF643, // This is a duplicate... — a +/// genuine retail opcode reuse, not an ACE bug. GameMessageCharacterRestore +/// (ACE.Server/Network/GameMessages/Messages/GameMessageCharacterRestore.cs) +/// unconditionally writes a success shape: +/// +/// +/// +/// u32 opcode (0xF643) +/// u32 verificationFlag (1 = Ok, matching CharacterGenerationVerificationResponse.Ok) +/// u32 characterGuid +/// String16L characterName +/// u32 secondsGreyedOut +/// +/// +/// +/// But retail's CharacterRestore handler can ALSO reply on this same +/// opcode via the character-CREATE response path when restore itself fails +/// (e.g. SendCharacterCreateResponse(session, CharacterGenerationVerificationResponse.NameInUse) +/// when the freed name collides) — that shape is flag-only, with NO +/// trailing fields (GameMessageCharacterCreateResponse.cs: the guid / +/// name / trailing u32 are only written if (response == ... .Ok)). +/// mirrors that conditionality: the trailing three +/// fields are read only when verificationFlag == 1. Because the two +/// message families are wire-identical when they collide, a caller cannot +/// tell "restore response" from "create response" by opcode or shape +/// alone — it must track which outbound request (this file's +/// vs. a future CharacterCreate) it is +/// awaiting a reply to. Character creation is out of this campaign's scope +/// (design spec §7 non-goals); this type does not attempt to disambiguate +/// the two families itself. +/// +/// +public static class CharacterRestore +{ + public const uint RequestOpcode = 0xF7D9u; + public const uint ResponseOpcode = 0xF643u; + + /// + /// Restore response body. , , and + /// are only populated when + /// equals 1 (Ok) — retail omits them + /// entirely on the wire otherwise (see the collision note above). + /// + public readonly record struct Parsed( + uint VerificationFlag, + uint? Guid, + string? Name, + uint? SecondsGreyedOut) + { + /// True when the trailing character fields are present. + public bool IsOk => VerificationFlag == 1u; + } + + /// + /// Build the body bytes for an outbound CharacterRestore request. + /// Layout: opcode(4) + characterGuid(4). Guid-only — see the class doc + /// comment for why the decompiled call site's apparent extra strings + /// are not real. + /// + public static byte[] BuildRequestBody(uint characterGuid) + { + var w = new PacketWriter(8); + w.WriteUInt32(RequestOpcode); + w.WriteUInt32(characterGuid); + return w.ToArray(); + } + + /// + /// Parse a CharacterRestore response body (opcode 0xF643). + /// must start with the 4-byte opcode. + /// + public static Parsed Parse(ReadOnlySpan body) + { + int pos = 0; + + uint opcode = ReadU32(body, ref pos); + if (opcode != ResponseOpcode) + throw new FormatException($"expected CharacterRestore response opcode 0x{ResponseOpcode:X4}, got 0x{opcode:X8}"); + + uint verificationFlag = ReadU32(body, ref pos); + if (verificationFlag != 1u) + return new Parsed(verificationFlag, null, null, null); + + uint guid = ReadU32(body, ref pos); + string name = StringReader.ReadString16L(body, ref pos); + uint secondsGreyedOut = ReadU32(body, ref pos); + + return new Parsed(verificationFlag, guid, name, secondsGreyedOut); + } + + private static uint ReadU32(ReadOnlySpan source, ref int pos) + { + if (source.Length - pos < 4) throw new FormatException("truncated u32"); + uint value = BinaryPrimitives.ReadUInt32LittleEndian(source.Slice(pos)); + pos += 4; + return value; + } +} diff --git a/tests/AcDream.Core.Net.Tests/Messages/CharacterDeleteTests.cs b/tests/AcDream.Core.Net.Tests/Messages/CharacterDeleteTests.cs new file mode 100644 index 00000000..d1fed597 --- /dev/null +++ b/tests/AcDream.Core.Net.Tests/Messages/CharacterDeleteTests.cs @@ -0,0 +1,83 @@ +using System.Buffers.Binary; +using AcDream.Core.Net.Messages; + +namespace AcDream.Core.Net.Tests.Messages; + +public sealed class CharacterDeleteTests +{ + [Fact] + public void BuildRequestBody_Layout_OpcodeThenAccountThenSlot() + { + byte[] body = CharacterDelete.BuildRequestBody("testaccount", characterSlot: 3); + + int pos = 0; + Assert.Equal(CharacterDelete.Opcode, + BinaryPrimitives.ReadUInt32LittleEndian(body.AsSpan(pos))); pos += 4; + + // String16L("testaccount") = u16(11) + 11 ASCII bytes, padded to a + // 4-byte boundary counted from the length prefix: 2 + 11 = 13 -> 16 + // (3 pad bytes). + ushort len = BinaryPrimitives.ReadUInt16LittleEndian(body.AsSpan(pos)); + Assert.Equal(11, len); pos += 2; + string name = System.Text.Encoding.ASCII.GetString(body.AsSpan(pos, 11)); + Assert.Equal("testaccount", name); pos += 11; + Assert.Equal(0, body[pos++]); + Assert.Equal(0, body[pos++]); + Assert.Equal(0, body[pos++]); + + uint slot = BinaryPrimitives.ReadUInt32LittleEndian(body.AsSpan(pos)); pos += 4; + Assert.Equal(3u, slot); + + Assert.Equal(4 + 16 + 4, body.Length); // opcode + padded string + slot + Assert.Equal(pos, body.Length); + } + + [Fact] + public void BuildRequestBody_ExactByteSequence_ShortAccount() + { + // "ab" -> String16L = u16(2) + 2 bytes = 4, already 4-byte aligned, + // no padding. + byte[] body = CharacterDelete.BuildRequestBody("ab", characterSlot: 0x11u); + + byte[] expected = + [ + 0x55, 0xF6, 0x00, 0x00, // opcode 0xF655 LE + 0x02, 0x00, // String16L length = 2 + (byte)'a', (byte)'b', // string bytes + 0x11, 0x00, 0x00, 0x00, // characterSlot = 0x11 LE + ]; + + Assert.Equal(expected, body); + } + + [Fact] + public void BuildRequestBody_NullAccountName_Throws() + { + Assert.Throws( + () => CharacterDelete.BuildRequestBody(null!, characterSlot: 0)); + } + + [Fact] + public void IsAcknowledgement_AcceptsOpcodeOnlyBody() + { + byte[] body = BitConverter.GetBytes(CharacterDelete.Opcode); + + Assert.True(CharacterDelete.IsAcknowledgement(body)); + } + + [Fact] + public void IsAcknowledgement_RejectsRequestShapedBody() + { + byte[] request = CharacterDelete.BuildRequestBody("acct", characterSlot: 1); + + Assert.False(CharacterDelete.IsAcknowledgement(request)); + } + + [Fact] + public void IsAcknowledgement_RejectsTruncatedOrDifferentOpcode() + { + Assert.False(CharacterDelete.IsAcknowledgement([0x55, 0xF6, 0x00])); + Assert.False(CharacterDelete.IsAcknowledgement(BitConverter.GetBytes(0xF656u))); + Assert.False(CharacterDelete.IsAcknowledgement([])); + } +} diff --git a/tests/AcDream.Core.Net.Tests/Messages/CharacterErrorTests.cs b/tests/AcDream.Core.Net.Tests/Messages/CharacterErrorTests.cs new file mode 100644 index 00000000..b738378f --- /dev/null +++ b/tests/AcDream.Core.Net.Tests/Messages/CharacterErrorTests.cs @@ -0,0 +1,113 @@ +using System.Buffers.Binary; +using AcDream.Core.Net.Messages; + +namespace AcDream.Core.Net.Tests.Messages; + +public sealed class CharacterErrorTests +{ + [Theory] + [InlineData(0x00u, CharacterError.Code.Undefined)] + [InlineData(0x01u, CharacterError.Code.Logon)] + [InlineData(0x02u, CharacterError.Code.LoggedOn)] + [InlineData(0x03u, CharacterError.Code.AccountLogon)] + [InlineData(0x04u, CharacterError.Code.ServerCrash)] + [InlineData(0x05u, CharacterError.Code.Logoff)] + [InlineData(0x06u, CharacterError.Code.Delete)] + [InlineData(0x07u, CharacterError.Code.NoPremade)] + [InlineData(0x08u, CharacterError.Code.AccountInUse)] + [InlineData(0x09u, CharacterError.Code.AccountInvalid)] + [InlineData(0x0Au, CharacterError.Code.AccountDoesntExist)] + [InlineData(0x0Bu, CharacterError.Code.EnterGameGeneric)] + [InlineData(0x0Cu, CharacterError.Code.EnterGameStressAccount)] + [InlineData(0x0Du, CharacterError.Code.EnterGameCharacterInWorld)] + [InlineData(0x0Eu, CharacterError.Code.EnterGamePlayerAccountMissing)] + [InlineData(0x0Fu, CharacterError.Code.EnterGameCharacterNotOwned)] + [InlineData(0x10u, CharacterError.Code.EnterGameCharacterInWorldServer)] + [InlineData(0x11u, CharacterError.Code.EnterGameOldCharacter)] + [InlineData(0x12u, CharacterError.Code.EnterGameCorruptCharacter)] + [InlineData(0x13u, CharacterError.Code.EnterGameStartServerDown)] + [InlineData(0x14u, CharacterError.Code.EnterGameCouldntPlaceCharacter)] + [InlineData(0x15u, CharacterError.Code.LogonServerFull)] + [InlineData(0x16u, CharacterError.Code.CharacterIsBooted)] + [InlineData(0x17u, CharacterError.Code.EnterGameCharacterLocked)] + [InlineData(0x18u, CharacterError.Code.SubscriptionExpired)] + [InlineData(0x19u, CharacterError.Code.NumErrors)] + public void Parse_EveryRetailCode_RoundTripsRawAndNamedValue(uint raw, CharacterError.Code expected) + { + var w = AceWireWriter.GameMessage(CharacterError.Opcode).Write(raw); + + CharacterError.Parsed parsed = CharacterError.Parse(w.ToArray()); + + Assert.Equal(raw, parsed.RawErrorCode); + Assert.Equal(expected, parsed.AsCode); + Assert.Equal((uint)expected, raw); + } + + [Fact] + public void Parse_UnknownErrorCode_DoesNotThrow_PreservesRawValue() + { + // A value retail never defined (and well past CHAR_ERROR_NUM_ERRORS) + // — a future server revision or a private server could still send + // it. Must not throw; the raw wire value is the source of truth. + var w = AceWireWriter.GameMessage(CharacterError.Opcode).Write(0xDEADBEEFu); + + CharacterError.Parsed parsed = CharacterError.Parse(w.ToArray()); + + Assert.Equal(0xDEADBEEFu, parsed.RawErrorCode); + Assert.Equal((CharacterError.Code)0xDEADBEEFu, parsed.AsCode); + } + + [Fact] + public void Parse_MaxUintErrorCode_DoesNotThrow() + { + var w = AceWireWriter.GameMessage(CharacterError.Opcode).Write(uint.MaxValue); + + CharacterError.Parsed parsed = CharacterError.Parse(w.ToArray()); + + Assert.Equal(uint.MaxValue, parsed.RawErrorCode); + } + + [Fact] + public void Parse_ExactByteSequence_MatchesAceSerializer() + { + // ACE's GameMessageCharacterError: opcode then Writer.Write((uint)error). + byte[] body = AceWireWriter.GameMessage(CharacterError.Opcode) + .Write((uint)CharacterError.Code.Delete) + .ToArray(); + + byte[] expected = + [ + 0x59, 0xF6, 0x00, 0x00, // opcode 0xF659 LE + 0x06, 0x00, 0x00, 0x00, // CHAR_ERROR_DELETE = 6 LE + ]; + + Assert.Equal(expected, body); + + CharacterError.Parsed parsed = CharacterError.Parse(body); + Assert.Equal(CharacterError.Code.Delete, parsed.AsCode); + } + + [Fact] + public void Parse_WrongOpcode_Throws() + { + byte[] bytes = new byte[4]; + BinaryPrimitives.WriteUInt32LittleEndian(bytes, 0xDEADBEEFu); + + Assert.Throws(() => CharacterError.Parse(bytes)); + } + + [Fact] + public void Parse_Truncated_Throws() + { + byte[] bytes = new byte[4]; // just the opcode, missing the error code + BinaryPrimitives.WriteUInt32LittleEndian(bytes, CharacterError.Opcode); + + Assert.Throws(() => CharacterError.Parse(bytes)); + } + + [Fact] + public void Parse_EmptyBody_Throws() + { + Assert.Throws(() => CharacterError.Parse([])); + } +} diff --git a/tests/AcDream.Core.Net.Tests/Messages/CharacterRestoreTests.cs b/tests/AcDream.Core.Net.Tests/Messages/CharacterRestoreTests.cs new file mode 100644 index 00000000..432b425c --- /dev/null +++ b/tests/AcDream.Core.Net.Tests/Messages/CharacterRestoreTests.cs @@ -0,0 +1,123 @@ +using System.Buffers.Binary; +using AcDream.Core.Net.Messages; + +namespace AcDream.Core.Net.Tests.Messages; + +public sealed class CharacterRestoreTests +{ + [Fact] + public void BuildRequestBody_ExactByteSequence_OpcodeThenGuidOnly() + { + byte[] body = CharacterRestore.BuildRequestBody(0x50000001u); + + byte[] expected = + [ + 0xD9, 0xF7, 0x00, 0x00, // opcode 0xF7D9 LE + 0x01, 0x00, 0x00, 0x50, // guid 0x50000001 LE + ]; + + Assert.Equal(expected, body); + Assert.Equal(8, body.Length); + } + + [Fact] + public void Parse_SuccessResponse_PopulatesAllTrailingFields() + { + // Mirrors ACE's GameMessageCharacterRestore: opcode, flag=1 (Ok), + // guid, String16L name, secondsGreyedOut. + var w = AceWireWriter.GameMessage(CharacterRestore.ResponseOpcode) + .Write(1u) + .WriteGuid(0x50000002u) + .WriteString16L("+Acdream") + .Write(0u); + + CharacterRestore.Parsed parsed = CharacterRestore.Parse(w.ToArray()); + + Assert.Equal(1u, parsed.VerificationFlag); + Assert.True(parsed.IsOk); + Assert.Equal(0x50000002u, parsed.Guid); + Assert.Equal("+Acdream", parsed.Name); + Assert.Equal(0u, parsed.SecondsGreyedOut); + } + + [Fact] + public void Parse_SuccessResponse_NonzeroSecondsGreyedOutPreserved() + { + var w = AceWireWriter.GameMessage(CharacterRestore.ResponseOpcode) + .Write(1u) + .WriteGuid(0x50000003u) + .WriteString16L("Restored") + .Write(45u); + + CharacterRestore.Parsed parsed = CharacterRestore.Parse(w.ToArray()); + + Assert.Equal(45u, parsed.SecondsGreyedOut); + } + + [Fact] + public void Parse_FailureShapedResponse_LeavesTrailingFieldsNull() + { + // Retail's colliding CharacterCreateResponse shape: a non-Ok flag + // (here 3 = NameInUse) has NO trailing guid/name/seconds on the + // wire at all — GameMessageCharacterCreateResponse.cs only writes + // them "if (response == ... .Ok)". Parse must not try to read past + // the flag in this case. + var w = AceWireWriter.GameMessage(CharacterRestore.ResponseOpcode) + .Write(3u); // CharacterGenerationVerificationResponse.NameInUse + + CharacterRestore.Parsed parsed = CharacterRestore.Parse(w.ToArray()); + + Assert.Equal(3u, parsed.VerificationFlag); + Assert.False(parsed.IsOk); + Assert.Null(parsed.Guid); + Assert.Null(parsed.Name); + Assert.Null(parsed.SecondsGreyedOut); + } + + [Fact] + public void Parse_WrongOpcode_Throws() + { + byte[] bytes = new byte[4]; + BinaryPrimitives.WriteUInt32LittleEndian(bytes, 0xDEADBEEFu); + + Assert.Throws(() => CharacterRestore.Parse(bytes)); + } + + [Fact] + public void Parse_TruncatedAfterFlag_Throws() + { + // Claims success (flag=1) but the body ends before the guid. + var w = AceWireWriter.GameMessage(CharacterRestore.ResponseOpcode).Write(1u); + + Assert.Throws(() => CharacterRestore.Parse(w.ToArray())); + } + + [Fact] + public void Parse_TruncatedBeforeFlag_Throws() + { + var w = AceWireWriter.GameMessage(CharacterRestore.ResponseOpcode); + + Assert.Throws(() => CharacterRestore.Parse(w.ToArray())); + } + + [Fact] + public void RequestThenResponse_RoundTrips_GuidIdentity() + { + const uint guid = 0x50000009u; + byte[] request = CharacterRestore.BuildRequestBody(guid); + + // The request itself carries only the guid; re-derive it the same + // way a caller would to confirm nothing was lost in the builder. + uint requestedGuid = BinaryPrimitives.ReadUInt32LittleEndian(request.AsSpan(4)); + Assert.Equal(guid, requestedGuid); + + var w = AceWireWriter.GameMessage(CharacterRestore.ResponseOpcode) + .Write(1u) + .WriteGuid(guid) + .WriteString16L("RoundTrip") + .Write(0u); + CharacterRestore.Parsed response = CharacterRestore.Parse(w.ToArray()); + + Assert.Equal(requestedGuid, response.Guid); + } +} From 4338b1c1f37135c635f6d5adc38c57c12f8e0f23 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 14 Aug 2026 16:01:52 +0200 Subject: [PATCH 2/3] =?UTF-8?q?fix(net):=20Campaign=20LA=20LA7a=20review?= =?UTF-8?q?=20fixes=20=E2=80=94=20AD-97=20register=20row,=20corrected=20re?= =?UTF-8?q?store=20justification?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Opus retail-lens review decoded the PDB-paired binary at CPlayerSystem::RestoreCharacter@0x0055d760 and refuted the uninitialized-edx justification: the two extra arguments are real push imm32 of a constant PStringBase (BN mis-renders them, but they pack to >=4 bytes each), so retail 0xF7D9 is >=16 bytes where ours is 8. The guid-only CODE stands (ACE reads only the guid; holtburger consensus) but it is an adaptation, not a corrected decompile — filed as divergence register AD-97 and the doc comment now states the true mechanism. Also from the review: the 0xF643 conditional-parse doc now names BOTH ACE flag-only failure branches (NameInUse + Corrupt); CharacterError 0x08 doc corrected (ACE misnames it ServerCrash2 — the port corrects an ACE misnaming; ACE omits three values, not four); LA7b hazard notes added (ACE silent no-reply on unknown restore guid; retail SendToLogon vs SendToControl routing; NumErrors never rendered); two review-nit tests (flag=0 Undef flag-only, non-Ok body with trailing bytes ignored). Core.Net suite: 953 passed / 0 failed. Co-Authored-By: Claude Fable 5 --- .../retail-divergence-register.md | 1 + .../Messages/CharacterDelete.cs | 7 ++ .../Messages/CharacterError.cs | 23 +++++-- .../Messages/CharacterRestore.cs | 64 +++++++++++-------- .../Messages/CharacterRestoreTests.cs | 39 +++++++++++ 5 files changed, 100 insertions(+), 34 deletions(-) diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index fc668ffe..7fd0b897 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -189,6 +189,7 @@ readiness/requeue adaptation. See | AD-92 | **Filed 2026-08-13 at the #376/#388 review fix round (blast M6 / mechanism M4).** Two switcher adaptations with no retail counterpart: (1) the fullscreen refresh rate is the monitor's HIGHEST for the picked WxH — retail passed the device mode's own refresh as-is (`Device::ForceDisplayResolution`); (2) an invalid/unsupported fullscreen request is a logged refusal that leaves the window unchanged — retail attempted the switch and surfaced the device error. The persisted-flag divergence a refusal leaves behind is ISSUES #392. | `src/AcDream.App/Settings/DisplayModeSwitching.cs` (`TryFindRefreshRate`, the refusal paths); `src/AcDream.App/Settings/RuntimeSettingsTargets.cs` (`Apply`'s refused-mode logging) | Highest-refresh is strictly better on modern variable-refresh panels (retail predates them); refuse-and-log is #388's own no-crash requirement. | A capture comparing retail's exact chosen refresh for a mode will differ; a server/tooling flow expecting an error dialog on an invalid mode sees a console line instead. | `Device::ForceDisplayResolution @gmClient::Init 0x004047af`; docs/research/2026-08-13-376-388-{mechanism,blast}-review.md | | AD-94 | **Filed 2026-08-14 at the secure-trade feature.** Retail's `Event_AcceptTrade` payload (`Trade::Pack @0x005B9FF0`) appends two `PackableList` staged-item lists after the six fixed fields; acdream sends both as ZERO-COUNT lists. ACE parses and then discards the ENTIRE payload (`HandleActionAcceptTrade()` takes zero arguments — server trade state is fully self-derived; lane B §quirks), so the difference is unobservable against ACE; a byte-capture comparison against a real retail client would differ from offset 40. | `src/AcDream.Core.Net/Messages/TradeRequests.cs` (`BuildAcceptTrade`) | The `ContentProfile` pack layout was not byte-verified (ACE never reads it — no reader to check against), and guessing a wire struct violates the workflow; zero-count lists are well-formed `PackableList`s. | A future server that actually validates the accept echo would see empty item lists and could refuse or desync the accept. | `Trade::Pack @0x005B9FF0`; `GameActionAcceptTrade.cs:11-16`; `docs/research/2026-08-14-trade-laneB-wire.md` Table 1 | | AD-96 | **Filed 2026-08-14 at the OP8 re-gate fix round (key-name display).** Retail's `GetNameFromKey_Internal @0x00687800` falls back from the DAT string tables (key enum 4 → `0x2300000A`, meta enum 5 → `0x2300000B`) to the OS keyboard layout's own key name via DirectInput `IDirectInputDevice8::GetObjectInfo` (`tszName` — "SKIFT" on a Swedish layout). acdream reads the SAME layout-resident name data through Win32 `GetKeyNameTextW` instead (no DirectInput device exists in-process); on non-Windows hosts there is no OS lookup at all and the DIK-suffix spelling shows (un-localized English, e.g. "LSHIFT"). Mouse chords keep the pre-existing enum spelling — retail names them through the DirectInput mouse device. | `src/AcDream.App/Platform/PlatformKeyNameProvider.cs`; `src/AcDream.App/UI/Layout/RetailKeyNames.cs` (`Describe`, the mouse-device early-out) | GetKeyNameText and DirectInput's key names both come from the active keyboard-layout tables; adding a DirectInput device solely for name strings would be a heavyweight, dead-end dependency. Linux graphical work is parked at Slice L1. | A key whose GetKeyNameTextW name differs from DirectInput's `tszName` on some layout shows a slightly different caption than retail did; Linux graphical shows English DIK-suffix names where retail-on-Wine would localize; a mouse-chord caption reads as the Silk enum, not retail's device string. | `CInputManager_WIN32::GetNameFromKey_Internal @0x00687800`; `GetNameFromKey @0x00687F40`; `ControlSpecification::GetDIKName @0x0068ACB0`; `DBCache::GetDIDFromEnumStatic` category-4 probe 2026-08-14 (`KeyboardConfigLiveMountProbeTests.ProbeKeyboardFontsAndKeyNameStrings`) | +| AD-97 | **Filed 2026-08-14 at Campaign LA slice LA7a (character-restore request tail).** Retail's `CharacterRestore` request (`0xF7D9`) is ≥16 bytes: `CPlayerSystem::RestoreCharacter @0x0055d760` is, in the PDB-paired binary, `push 0x008173B4; push 0x008173B4; push guid; call Proto_UI::SendAdminRestoreCharacter @0x00546cf0`, and the callee packs BOTH constant `PStringBase*` arguments (`PStringBase::Pack @0x004fc6f0` emits ≥4 bytes even empty). Binary Ninja renders the two pushes as an uninitialized `edx` local plus `this` — a rendering artifact around constant `0x008173B4` (4 of its 5 other pseudo-C appearances sit in provably-broken decompiles), but the arguments are real. acdream sends the 8-byte guid-only form. What the two constant strings contain is unresolved (a live cdb `db poi(0x008173b4)` would settle it). | `src/AcDream.Core.Net/Messages/CharacterRestore.cs` (`BuildRequestBody`) | ACE reads only `ReadUInt32()` and ignores any tail (`CharacterHandler.cs:331-385`), and holtburger ships guid-only from a real client command path against ACE successfully — the tail is unread by every server we can test against, and packing two strings whose CONTENT we cannot verify would be a guess. | A byte-capture comparison against a real retail client differs from offset 8; a future server that validates the full retail shape would reject our 8-byte request. | `CPlayerSystem::RestoreCharacter @0x0055d760` (binary bytes, not the BN rendering); `Proto_UI::SendAdminRestoreCharacter @0x00546cf0`; `PStringBase::Pack @0x004fc6f0`; ACE `CharacterHandler.cs:331-385`; holtburger `character_selection.rs:79-82`; LA7a Opus review F1 (2026-08-14) | | AD-93 | **Filed 2026-08-13 at social gate round 2, item 5 (the refused-drop notice port).** Two narrow gaps in the `ServerSaysAttemptFailed @0x0058EAE0` port: (1) **latched-guid preference** — retail's 0x00A0 dispatcher (`@0x0055B342`) PREFERS `prevRequestObjectID` over the wire guid when picking the item to name; acdream's `InventoryTransactionState.OnMoveFailed` instead REQUIRES the wire guid to match the latch (unobservable against ACE, which always sends the request's own guid on 0x00A0, and it protects a stale latch from mislabeling an unrelated failure — acdream has no retail-style latch timeout). (2) **unlatched request kinds** — retail latches `IR_MOVE`/`IR_WIELD` too; acdream's kind enum has no Move/Wield rows because wields ride `AutoWieldController` outside the single-request gate, so a refused wield/3D-move shows only the generic `HandleFailureEvent` leg, never "The X can't be wielded/moved". | `src/AcDream.Core/Items/InventoryTransactionState.cs` (`OnMoveFailed`); `src/AcDream.Core/Chat/InventoryFailureMessages.cs` (`Compose`'s absent Move/Wield rows); `src/AcDream.App/UI/ItemInteractionController.cs` (`OnInventoryRequestFailed`) | The match requirement is the compensating guard for the missing latch timeout; adding Wield/Move kinds means routing those sends through the single-request gate they deliberately bypass today — a behavior change beyond this gate item. | Only observable against a server that sends 0x00A0 with a guid that differs from the request's item (ACE never does), or on a refused wield/move, which shows no "can't be wielded/moved" verb line where retail would show one. | `ACCWeenieObject::ServerSaysAttemptFailed @0x0058EAE0`; the 0x00A0 dispatcher `@0x0055B342`; `ACCWeenieObject::RecordRequest @0x0058C220`; `docs/research/2026-08-13-confirm-and-weenie-error-display.md` §2 | --- diff --git a/src/AcDream.Core.Net/Messages/CharacterDelete.cs b/src/AcDream.Core.Net/Messages/CharacterDelete.cs index 6fa07037..f02caa4d 100644 --- a/src/AcDream.Core.Net/Messages/CharacterDelete.cs +++ b/src/AcDream.Core.Net/Messages/CharacterDelete.cs @@ -43,6 +43,13 @@ namespace AcDream.Core.Net.Messages; /// /// /// +/// Routing note for LA7b: retail transmits this request via +/// Proto_UI::SendToLogon (the restore request rides +/// SendToControl); ACE sends its acknowledgement and the follow-up +/// refreshed CharacterList on GameMessageGroup.UIQueue. +/// +/// +/// /// After the ack, ACE immediately follows with a fresh /// so the roster reflects the character's new pending-delete state /// (CharacterHandler.CharacterDelete, diff --git a/src/AcDream.Core.Net/Messages/CharacterError.cs b/src/AcDream.Core.Net/Messages/CharacterError.cs index 7d2c70ce..40589d72 100644 --- a/src/AcDream.Core.Net/Messages/CharacterError.cs +++ b/src/AcDream.Core.Net/Messages/CharacterError.cs @@ -43,11 +43,15 @@ namespace AcDream.Core.Net.Messages; /// It is a strict superset of ACE's ACE.Server.Network.Enum.CharacterError /// (references/ACE/Source/ACE.Server/Network/Enum/CharacterError.cs): /// retail additionally names 0x2 (LoggedOn), 0x7 (NoPremade), -/// 0x8 (AccountInUse), and 0x16 (CharacterIsBooted), none of -/// which ACE's server ever sends but all of which retail's client can -/// receive from a genuine retail server — per the project's -/// property-enum-divergence lesson, we port the complete oracle, not just -/// what today's one server implementation emits. ACE's per-value doc +/// and 0x16 (CharacterIsBooted) — three values ACE omits entirely, +/// none of which ACE's server ever sends but all of which retail's client +/// can receive from a genuine retail server. At 0x8 the port additionally +/// CORRECTS an ACE misnaming: ACE defines 0x8 as ServerCrash2 with a +/// doc comment duplicating 0x4's ID_CHAR_ERROR_SERVER_CRASH text, +/// but retail's header names 0x8 CHAR_ERROR_ACCOUNT_IN_USE — the +/// header wins. Per the project's property-enum-divergence lesson, we port +/// the complete oracle, not just what today's one server implementation +/// emits. ACE's per-value doc /// comments (themselves sourced from the client's ID_CHAR_ERROR_* /// string table) are folded in below where they exist. One retail member, /// FORCE_charError_32_BIT = 0x7FFFFFFF, is a compiler @@ -111,7 +115,11 @@ public static class CharacterError /// 0x07 — CHAR_ERROR_NO_PREMADE. Retail-only; no ACE member. NoPremade = 0x07, - /// 0x08 — CHAR_ERROR_ACCOUNT_IN_USE. Retail-only; no ACE member. + /// + /// 0x08 — CHAR_ERROR_ACCOUNT_IN_USE. ACE misnames this value + /// ServerCrash2 (its doc comment duplicates 0x04's text); + /// retail's header is the authority. See the class doc comment. + /// AccountInUse = 0x08, /// @@ -228,7 +236,8 @@ public static class CharacterError /// sentinel (the array-bound idiom, one past the last real code) — /// never sent on the wire as an actual error. Kept for verbatim /// completeness of the enum range; do not treat a received 0x19 - /// as meaningful. + /// as meaningful, and LA7b's error-to-string mapping must not + /// render it as a user-facing message. /// NumErrors = 0x19, } diff --git a/src/AcDream.Core.Net/Messages/CharacterRestore.cs b/src/AcDream.Core.Net/Messages/CharacterRestore.cs index a40858e2..794cc50d 100644 --- a/src/AcDream.Core.Net/Messages/CharacterRestore.cs +++ b/src/AcDream.Core.Net/Messages/CharacterRestore.cs @@ -8,27 +8,36 @@ namespace AcDream.Core.Net.Messages; /// (opcode 0xF643). /// /// -/// Request — guid-only, by reference consensus. The decompiled call -/// site (Proto_UI::SendAdminRestoreCharacter at 0x00546cf0, -/// declared with three parameters — a u32 and two PStringBase<char> -/// pointers — and packing two strings after the u32) LOOKS like it sends -/// guid + two strings. It does not: its only real caller, -/// CPlayerSystem::RestoreCharacter at 0x0055d760, declares -/// class PStringBase<char>* edx; as a local and passes it -/// straight through UNINITIALIZED as the second argument, and passes -/// this (a CPlayerSystem*, not a string) as the third. Both -/// are textbook decompiler register-corruption artifacts (uninitialized -/// register reuse + a mistyped extra parameter from an over-declared -/// callee signature), not real arguments the real call site ever -/// supplied. ACE +/// Request — guid-only, an ADAPTATION (register row AD-97). Retail +/// really does send more than the guid. The PDB-paired binary at +/// CPlayerSystem::RestoreCharacter@0x0055d760 is 26 bytes: +/// push 0x008173B4; push 0x008173B4; push guid; +/// call Proto_UI::SendAdminRestoreCharacter@0x00546cf0 — two REAL +/// constant PStringBase<char>* arguments (Binary Ninja renders +/// them as an uninitialized edx local and this; that +/// rendering is the artifact, the two push imm32 are not). +/// SendAdminRestoreCharacter packs both +/// (PStringBase::Pack@0x004fc6f0 emits ≥4 bytes even for an empty +/// string), so retail's request is ≥16 bytes where ours is 8. We send +/// guid-only because ACE /// (CharacterHandler.CharacterRestore, -/// ACE.Server/Network/Handlers/CharacterHandler.cs:331-385, reads -/// only ReadUInt32()) and holtburger +/// ACE.Server/Network/Handlers/CharacterHandler.cs:331-385) reads +/// only ReadUInt32() and ignores any tail, and holtburger /// (holtburger-protocol/src/messages/character/types.rs::CharacterRestoreRequestData, -/// guid-only) independently agree on guid-only. We follow the two -/// independent, uncorrupted references (design spec §11 item 4 — wire -/// consensus, no divergence-register row needed: this isn't a deviation -/// from retail, it's picking the correct reading of a corrupted decompile). +/// sent from a real client command path) ships guid-only against ACE +/// successfully. The omitted tail is a recorded retail deviation — +/// divergence register AD-97. +/// +/// +/// +/// LA7b hazards. (1) ACE's restore handler has a SILENT no-reply +/// path: an unknown guid hits +/// Characters.SingleOrDefault(...) == null → return; — no 0xF643, +/// no 0xF659. Selection state must never await a restore reply +/// unconditionally. (2) Routing: ACE sends the response on +/// GameMessageGroup.UIQueue; retail transmits the request via +/// Proto_UI::SendToControl (the delete request goes via +/// SendToLogon) — relevant when LA7b picks the outbound queue. /// /// /// @@ -55,12 +64,13 @@ namespace AcDream.Core.Net.Messages; /// /// /// -/// But retail's CharacterRestore handler can ALSO reply on this same +/// But ACE's CharacterRestore handler can ALSO reply on this same /// opcode via the character-CREATE response path when restore itself fails -/// (e.g. SendCharacterCreateResponse(session, CharacterGenerationVerificationResponse.NameInUse) -/// when the freed name collides) — that shape is flag-only, with NO -/// trailing fields (GameMessageCharacterCreateResponse.cs: the guid / -/// name / trailing u32 are only written if (response == ... .Ok)). +/// — TWO real branches: NameInUse (the freed name collided) and +/// Corrupt (SaveCharacter returned false). Both shapes are +/// flag-only, with NO trailing fields +/// (GameMessageCharacterCreateResponse.cs: the guid / name / +/// trailing u32 are only written if (response == ... .Ok)). /// mirrors that conditionality: the trailing three /// fields are read only when verificationFlag == 1. Because the two /// message families are wire-identical when they collide, a caller cannot @@ -95,9 +105,9 @@ public static class CharacterRestore /// /// Build the body bytes for an outbound CharacterRestore request. - /// Layout: opcode(4) + characterGuid(4). Guid-only — see the class doc - /// comment for why the decompiled call site's apparent extra strings - /// are not real. + /// Layout: opcode(4) + characterGuid(4). Guid-only — an adaptation of + /// retail's ≥16-byte shape; see the class doc comment and divergence + /// register AD-97. /// public static byte[] BuildRequestBody(uint characterGuid) { diff --git a/tests/AcDream.Core.Net.Tests/Messages/CharacterRestoreTests.cs b/tests/AcDream.Core.Net.Tests/Messages/CharacterRestoreTests.cs index 432b425c..1b81b5ac 100644 --- a/tests/AcDream.Core.Net.Tests/Messages/CharacterRestoreTests.cs +++ b/tests/AcDream.Core.Net.Tests/Messages/CharacterRestoreTests.cs @@ -74,6 +74,45 @@ public sealed class CharacterRestoreTests Assert.Null(parsed.SecondsGreyedOut); } + [Fact] + public void Parse_UndefFlagZero_FlagOnlyBody_LeavesTrailingFieldsNull() + { + // LA7a review test-coverage nit: flag 0 (Undef) is a non-Ok value + // distinct from the NameInUse case — the conditional must treat it + // as flag-only too. + var w = AceWireWriter.GameMessage(CharacterRestore.ResponseOpcode) + .Write(0u); // CharacterGenerationVerificationResponse.Undef + + CharacterRestore.Parsed parsed = CharacterRestore.Parse(w.ToArray()); + + Assert.Equal(0u, parsed.VerificationFlag); + Assert.False(parsed.IsOk); + Assert.Null(parsed.Guid); + Assert.Null(parsed.Name); + Assert.Null(parsed.SecondsGreyedOut); + } + + [Fact] + public void Parse_NonOkBodyWithTrailingBytes_IgnoresRatherThanMisreads() + { + // LA7a review test-coverage nit: a non-Ok body that DOES carry + // trailing bytes (unknown server variant / padding) must not be + // misread as character fields — the conditional stops at the flag + // and the extra bytes are ignored. + var w = AceWireWriter.GameMessage(CharacterRestore.ResponseOpcode) + .Write(3u) // NameInUse + .Write(0xDEADBEEFu) + .Write(0x12345678u); + + CharacterRestore.Parsed parsed = CharacterRestore.Parse(w.ToArray()); + + Assert.Equal(3u, parsed.VerificationFlag); + Assert.False(parsed.IsOk); + Assert.Null(parsed.Guid); + Assert.Null(parsed.Name); + Assert.Null(parsed.SecondsGreyedOut); + } + [Fact] public void Parse_WrongOpcode_Throws() { From 0c8643a7f2c557968c8dee395705968151371e31 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 14 Aug 2026 16:05:51 +0200 Subject: [PATCH 3/3] =?UTF-8?q?docs:=20AD-97=20wording=20nit=20from=20LA7a?= =?UTF-8?q?=20narrow=20re-review=20=E2=80=94=20correct=20artifact-appearan?= =?UTF-8?q?ce=20count?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- docs/architecture/retail-divergence-register.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 7fd0b897..f9b38e6b 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -189,7 +189,7 @@ readiness/requeue adaptation. See | AD-92 | **Filed 2026-08-13 at the #376/#388 review fix round (blast M6 / mechanism M4).** Two switcher adaptations with no retail counterpart: (1) the fullscreen refresh rate is the monitor's HIGHEST for the picked WxH — retail passed the device mode's own refresh as-is (`Device::ForceDisplayResolution`); (2) an invalid/unsupported fullscreen request is a logged refusal that leaves the window unchanged — retail attempted the switch and surfaced the device error. The persisted-flag divergence a refusal leaves behind is ISSUES #392. | `src/AcDream.App/Settings/DisplayModeSwitching.cs` (`TryFindRefreshRate`, the refusal paths); `src/AcDream.App/Settings/RuntimeSettingsTargets.cs` (`Apply`'s refused-mode logging) | Highest-refresh is strictly better on modern variable-refresh panels (retail predates them); refuse-and-log is #388's own no-crash requirement. | A capture comparing retail's exact chosen refresh for a mode will differ; a server/tooling flow expecting an error dialog on an invalid mode sees a console line instead. | `Device::ForceDisplayResolution @gmClient::Init 0x004047af`; docs/research/2026-08-13-376-388-{mechanism,blast}-review.md | | AD-94 | **Filed 2026-08-14 at the secure-trade feature.** Retail's `Event_AcceptTrade` payload (`Trade::Pack @0x005B9FF0`) appends two `PackableList` staged-item lists after the six fixed fields; acdream sends both as ZERO-COUNT lists. ACE parses and then discards the ENTIRE payload (`HandleActionAcceptTrade()` takes zero arguments — server trade state is fully self-derived; lane B §quirks), so the difference is unobservable against ACE; a byte-capture comparison against a real retail client would differ from offset 40. | `src/AcDream.Core.Net/Messages/TradeRequests.cs` (`BuildAcceptTrade`) | The `ContentProfile` pack layout was not byte-verified (ACE never reads it — no reader to check against), and guessing a wire struct violates the workflow; zero-count lists are well-formed `PackableList`s. | A future server that actually validates the accept echo would see empty item lists and could refuse or desync the accept. | `Trade::Pack @0x005B9FF0`; `GameActionAcceptTrade.cs:11-16`; `docs/research/2026-08-14-trade-laneB-wire.md` Table 1 | | AD-96 | **Filed 2026-08-14 at the OP8 re-gate fix round (key-name display).** Retail's `GetNameFromKey_Internal @0x00687800` falls back from the DAT string tables (key enum 4 → `0x2300000A`, meta enum 5 → `0x2300000B`) to the OS keyboard layout's own key name via DirectInput `IDirectInputDevice8::GetObjectInfo` (`tszName` — "SKIFT" on a Swedish layout). acdream reads the SAME layout-resident name data through Win32 `GetKeyNameTextW` instead (no DirectInput device exists in-process); on non-Windows hosts there is no OS lookup at all and the DIK-suffix spelling shows (un-localized English, e.g. "LSHIFT"). Mouse chords keep the pre-existing enum spelling — retail names them through the DirectInput mouse device. | `src/AcDream.App/Platform/PlatformKeyNameProvider.cs`; `src/AcDream.App/UI/Layout/RetailKeyNames.cs` (`Describe`, the mouse-device early-out) | GetKeyNameText and DirectInput's key names both come from the active keyboard-layout tables; adding a DirectInput device solely for name strings would be a heavyweight, dead-end dependency. Linux graphical work is parked at Slice L1. | A key whose GetKeyNameTextW name differs from DirectInput's `tszName` on some layout shows a slightly different caption than retail did; Linux graphical shows English DIK-suffix names where retail-on-Wine would localize; a mouse-chord caption reads as the Silk enum, not retail's device string. | `CInputManager_WIN32::GetNameFromKey_Internal @0x00687800`; `GetNameFromKey @0x00687F40`; `ControlSpecification::GetDIKName @0x0068ACB0`; `DBCache::GetDIDFromEnumStatic` category-4 probe 2026-08-14 (`KeyboardConfigLiveMountProbeTests.ProbeKeyboardFontsAndKeyNameStrings`) | -| AD-97 | **Filed 2026-08-14 at Campaign LA slice LA7a (character-restore request tail).** Retail's `CharacterRestore` request (`0xF7D9`) is ≥16 bytes: `CPlayerSystem::RestoreCharacter @0x0055d760` is, in the PDB-paired binary, `push 0x008173B4; push 0x008173B4; push guid; call Proto_UI::SendAdminRestoreCharacter @0x00546cf0`, and the callee packs BOTH constant `PStringBase*` arguments (`PStringBase::Pack @0x004fc6f0` emits ≥4 bytes even empty). Binary Ninja renders the two pushes as an uninitialized `edx` local plus `this` — a rendering artifact around constant `0x008173B4` (4 of its 5 other pseudo-C appearances sit in provably-broken decompiles), but the arguments are real. acdream sends the 8-byte guid-only form. What the two constant strings contain is unresolved (a live cdb `db poi(0x008173b4)` would settle it). | `src/AcDream.Core.Net/Messages/CharacterRestore.cs` (`BuildRequestBody`) | ACE reads only `ReadUInt32()` and ignores any tail (`CharacterHandler.cs:331-385`), and holtburger ships guid-only from a real client command path against ACE successfully — the tail is unread by every server we can test against, and packing two strings whose CONTENT we cannot verify would be a guess. | A byte-capture comparison against a real retail client differs from offset 8; a future server that validates the full retail shape would reject our 8-byte request. | `CPlayerSystem::RestoreCharacter @0x0055d760` (binary bytes, not the BN rendering); `Proto_UI::SendAdminRestoreCharacter @0x00546cf0`; `PStringBase::Pack @0x004fc6f0`; ACE `CharacterHandler.cs:331-385`; holtburger `character_selection.rs:79-82`; LA7a Opus review F1 (2026-08-14) | +| AD-97 | **Filed 2026-08-14 at Campaign LA slice LA7a (character-restore request tail).** Retail's `CharacterRestore` request (`0xF7D9`) is ≥16 bytes: `CPlayerSystem::RestoreCharacter @0x0055d760` is, in the PDB-paired binary, `push 0x008173B4; push 0x008173B4; push guid; call Proto_UI::SendAdminRestoreCharacter @0x00546cf0`, and the callee packs BOTH constant `PStringBase*` arguments (`PStringBase::Pack @0x004fc6f0` emits ≥4 bytes even empty). Binary Ninja renders the two pushes as an uninitialized `edx` local plus `this` — a rendering artifact around constant `0x008173B4` (all 3 of its other pseudo-C appearances sit in provably-broken decompiles), but the arguments are real. acdream sends the 8-byte guid-only form. What the two constant strings contain is unresolved (a live cdb `db poi(0x008173b4)` would settle it). | `src/AcDream.Core.Net/Messages/CharacterRestore.cs` (`BuildRequestBody`) | ACE reads only `ReadUInt32()` and ignores any tail (`CharacterHandler.cs:331-385`), and holtburger ships guid-only from a real client command path against ACE successfully — the tail is unread by every server we can test against, and packing two strings whose CONTENT we cannot verify would be a guess. | A byte-capture comparison against a real retail client differs from offset 8; a future server that validates the full retail shape would reject our 8-byte request. | `CPlayerSystem::RestoreCharacter @0x0055d760` (binary bytes, not the BN rendering); `Proto_UI::SendAdminRestoreCharacter @0x00546cf0`; `PStringBase::Pack @0x004fc6f0`; ACE `CharacterHandler.cs:331-385`; holtburger `character_selection.rs:79-82`; LA7a Opus review F1 (2026-08-14) | | AD-93 | **Filed 2026-08-13 at social gate round 2, item 5 (the refused-drop notice port).** Two narrow gaps in the `ServerSaysAttemptFailed @0x0058EAE0` port: (1) **latched-guid preference** — retail's 0x00A0 dispatcher (`@0x0055B342`) PREFERS `prevRequestObjectID` over the wire guid when picking the item to name; acdream's `InventoryTransactionState.OnMoveFailed` instead REQUIRES the wire guid to match the latch (unobservable against ACE, which always sends the request's own guid on 0x00A0, and it protects a stale latch from mislabeling an unrelated failure — acdream has no retail-style latch timeout). (2) **unlatched request kinds** — retail latches `IR_MOVE`/`IR_WIELD` too; acdream's kind enum has no Move/Wield rows because wields ride `AutoWieldController` outside the single-request gate, so a refused wield/3D-move shows only the generic `HandleFailureEvent` leg, never "The X can't be wielded/moved". | `src/AcDream.Core/Items/InventoryTransactionState.cs` (`OnMoveFailed`); `src/AcDream.Core/Chat/InventoryFailureMessages.cs` (`Compose`'s absent Move/Wield rows); `src/AcDream.App/UI/ItemInteractionController.cs` (`OnInventoryRequestFailed`) | The match requirement is the compensating guard for the missing latch timeout; adding Wield/Move kinds means routing those sends through the single-request gate they deliberately bypass today — a behavior change beyond this gate item. | Only observable against a server that sends 0x00A0 with a guid that differs from the request's item (ACE never does), or on a refused wield/move, which shows no "can't be wielded/moved" verb line where retail would show one. | `ACCWeenieObject::ServerSaysAttemptFailed @0x0058EAE0`; the 0x00A0 dispatcher `@0x0055B342`; `ACCWeenieObject::RecordRequest @0x0058C220`; `docs/research/2026-08-13-confirm-and-weenie-error-display.md` §2 | ---