fix/retail-compat: group flow, boards click, walk-crash CTD, profile-click UX#7
Open
Caeldeth wants to merge 13 commits into
Open
fix/retail-compat: group flow, boards click, walk-crash CTD, profile-click UX#7Caeldeth wants to merge 13 commits into
Caeldeth wants to merge 13 commits into
Conversation
baughj
reviewed
May 17, 2026
| @@ -3,7 +3,15 @@ | |||
| "allow": [ | |||
Member
There was a problem hiding this comment.
remove and add .claude to .gitignore?
| return; | ||
| case 4: //Hybrasyl `RecruitInfo` — full WriteInfo block. Map to Chaos enum ShowGroupBox. | ||
| { | ||
| var recruiter = reader.ReadString8(); |
Member
There was a problem hiding this comment.
this shit is making me screech (vs a library)
Collaborator
Author
There was a problem hiding this comment.
Documented as a section to replace when new networking library is available.
Captures the three retail-compatibility bugs targeted by this branch (N/S doors, group flow, board-click) along with confirmed symptoms, suspect commits, ground-truth Hybrasyl enum values, test targets (Rucesion 40,42 + Piet 29,12 + lod505 19,3), and the phase-by-phase plan with required review gates. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tion Investigation pass concluded the bug is not a regression on c0237ef / d74127b / 9bb61a6 — the client's outbound click path is genuinely axis-agnostic, c0237ef was reverted via the 9d3ed3f upstream merge, DoorTable covers every doors.md N/S pair, and Hybrasyl's 0x43 handler is byte-identical across main / develop / feature/doors. Reframed as missing-behavior likely present since inception: - Inbound 0x32 handling works (other-player door opens render fine and become walkable for this client). - Outbound click reaches retail but doesn't toggle for N/S; legacy DA client opens these same doors on retail. - Leading hypothesis (per user): retail validates trailing payload bytes on 0x43 ClickType=3 that Chaos.Client doesn't emit. Hybrasyl reads only the first 5 bytes and silently consumes any tail, so Hybrasyl tolerates our short packet; retail does not. - Ruled out: auto-walk/facing requirement, sotp pathfinder gating, DoorTable gaps, feature/doors server changes. Notes the ClickTile-on-any-foreground divergence as intentional (ClickTile is being expanded for targeting); do not gate on interactable detection. Hands off with three concrete unblock paths: wire capture, prior RE documentation, or brute-force append experiments. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… evidence Walk crash (new Phase 4): - User-supplied stack trace pinpoints DivideByZeroException at AnimationSystem.GetSteppedWalkOffset:654 (`y = x * startY / startX`) triggered by AdvanceWalk during WorldScreen.Update. - Root cause: GetWalkOffset returns Vector2.Zero for any non-cardinal Direction, leaving startX = 0; subsequent line 654 divides by it. The dialog/item just sets walk state with a bad Direction; the crash is the next-frame consequence. - Documents minimal proposed fix (early return when startX == 0) with rationale, and follow-up audit of GetWalkOffset's silent Vector2.Zero default. Doors (Phase 1, packet-capture finding): - E/W door click on retail receives 0x0C CreatureWalk (len 18) + 0x0E RemoveEntity (len 13). N/S receives nothing. - Retail's door model is entity-based (invisible door entity on the tile, opened by walking it off + removing). Hybrasyl's 0x32 Door is a different model. HandleDoor (0x32) is dead code on the retail path; existing CreatureWalk/RemoveEntity handlers cover door-open by accident. - Confirms the trailing-payload hypothesis: retail's 0x43 handler rejects N/S clicks before any 0x0C/0x0E is emitted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The branch's scoping doc is now canonical in the unified Hybrasyl document repo. Drop it here so a future merge to main doesn't resurrect it. The 24 sibling docs that exist on this branch's tree (because they predate the doc move on main) will be removed when the branch eventually merges or rebases onto main — main's deletion takes precedence in 3-way merge since this branch never modified them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Restructures the double-click-on-entity dispatch in HandleWorldClick so Aisling clicks (self and other) flow through a single EnableProfileClick gate rather than treating self as unconditional. - Self double-click → self-profile (when EnableProfileClick is on). - Other Aisling double-click → ClickEntity (server replies with their profile). Previously gated but no-op for most users since the default was off. - Non-Aisling entity click path is unchanged in behavior. Default EnableProfileClick flipped to true so other-Aisling profile viewing works out of the box; users can still disable both behaviors via the setting. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…onger render as male WorldState.AddOrUpdateAisling derived Gender from BodySprite via an inline ternary that only matched BodySprite.Female and FemaleGhost. The remaining female variants — FemaleInvis, FemaleHead, BlankFemale — fell through to the default Gender.Male, so any female character in invisible state (and head/blank states) rendered with male body, hair, face, and equipment EPF variants. Switches to the canonical DataUtilities.DetermineGender helper which already covers all 13 BodySprite enum values correctly. Single source of truth for gender derivation; no more drift between callers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…w reader The upstream DisplayGroupInviteConverter has its Deserialize and Serialize branches functionally swapped against the actual Hybrasyl/retail wire shape: Invite=1 attempts to read full DisplayGroupBoxInfo (which Hybrasyl never emits), while ShowGroupBox=4 ignores the box info Hybrasyl always carries. The class-slot order in Deserialize is also wrong (Monk before Rogue). Result: group invite popups never appeared on Hybrasyl. HandleDisplayGroupInvite now bypasses Client.Deserialize and reads the wire directly via SpanReader, dispatching on the raw subtype byte. Subtypes 1 and 4 populate DisplayGroupInviteArgs with the correct shape; subtypes 2 (Member) and 5 (RecruitAsk) log + drop pending retail packet capture. See chaos-networking-removal-direction.md "Tactical workarounds" for the bypass log and post-Phase-3 fold-back plan. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…bove panels Three visual fixes for the group flow: - Group invite popup ZIndex raised to 10 so it floats above StatusBook, GroupTabControl, NPC dialogs, etc. Matches DisconnectPopup's level. - HUD CGroup indicator and StatusBook GroupBtn now route through WorldState.UserOptions.Toggle(12) so all three toggle paths (HUD, profile, F4 settings) stay in sync. - The case-12 SettingToggled handler now optimistically updates WorldHud and StatusBook visuals on click before sending 0x2F. Hybrasyl's 0x2F handler doesn't push a SelfProfile back unless the toggle actually leaves a group, so wait-for-server visual updates desync against the server's flag flip. Subsequent SelfProfile updates reconcile if state diverges. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ConnectionManager already writes inbound packets to NoticeDebugLog
("inbound opcode=0xNN len=… handled=…"); the symmetric outbound
side was missing, so during retail-compat investigation we couldn't
distinguish "client never sent the packet" from "client sent but
retail ignored it."
Logs opcode, length, and the first 64 bytes of payload (truncated
to keep the log readable for long packets). Logging happens BEFORE
encryption so the hex matches what wire-format docs/RE describe —
the Crypto.ClientEncrypt step would otherwise scramble the bytes.
This is "Temporary diagnostic" by NoticeDebugLog's own description;
remove or gate the call when the door retail-compat work concludes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…posts
Retail's 0x43 type=3 coord-click handler validates a trailing flag byte
that distinguishes floor-aligned sprites (signposts, frames, ground items)
from above-tile sprites (door panels, awnings). The existing 5-byte
ClickTile sends no flag, retail defaults to 0 ("above-tile click"),
and the signpost dispatch silently drops because signposts are
floor-aligned. Hybrasyl ignores the trailing byte entirely and looks
up Map.Signposts[(x,y)] directly, which is why single-click works
on Hybrasyl but spam-clicking does nothing on retail.
Wire shape verified by Comhaigne's 0x43 RE refresh (commit 633063a):
[0x43][0x03][u16 x BE][u16 y BE][u8 flag]
flag = 1 for floor-aligned sprites, 0 for above-tile sprites. The flag
is the WorldObject_Static +0x80 anchor convention (inverted) on the
clicked sprite.
Adds ClickFloorTile(x, y) in ConnectionManager that bypasses the sealed
ClickArgs/ClickConverter — same pattern as the doors fix's ClickDoor.
Wires OnRootDoubleClick to fall back to ClickFloorTile when no entity
is hit and the tile has foreground; matches retail's UX where signposts
open on double-click. Hybrasyl's single-click QoL keeps the existing
5-byte ClickTile path unchanged.
Smoke-tested 2026-04-29 on lod505 (19, 3) against both Hybrasyl
(single-click) and retail (double-click) — board opens correctly.
F7 mail flow regression-checked, unaffected.
See chaos-networking-removal-direction.md "Tactical workarounds
(pre-removal)" for the third entry in the Chaos.Networking bypass
log (after doors, group).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rong art) Commit 6f50fc3 fixed gender derivation so BodySprite.FemaleInvis correctly resolved to Gender.Female, but exposed a separate asset bug: wb003 in khanwad is not an invisible body sprite — it contains unrelated "guard outfit" art. So female-invis characters rendered female hair/face/equipment overlays on top of guard-outfit body. mb003 is the correct invisible silhouette and is gender-neutral when invisible (no skin shows), so it composites cleanly under w-prefix overlays. Extends the existing "shield always loads from male archive" carve-out at AislingRenderer:1240/921 to also force male archive for the body 'b' layer when spriteId == 3. Both call sites now route through a new ForceMaleArchive helper so the wb003 hack is documented in exactly one place, with an explicit revert condition: remove the spriteId == 3 branch if/when a .datf asset pack ships correct wb003 art. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
main's .gitignore (line 403, via 6e504d6) already has the broad `.claude` rule, but the file remained tracked from before that rule was added. Untrack it so the ignore actually takes effect on this branch and matches the convention on main / feature/item-asset-packs. Addresses review comment from baughj on PR #7. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…MOVE WHEN Tightens the in-code workaround marker on the raw 0x63 deserializer to match the bar set by ClickFloorTile / ForceMaleArchive: - Promotes the inline `//` block to a proper XML <summary> + <remarks>. - Adds an explicit TACTICAL WORKAROUND label and a REMOVE WHEN clause tying replacement to the new Hybrasyl networking library owning DisplayGroupInviteArgs locally (per chaos-networking-removal-direction.md). - Cross-references the other two pre-removal bypasses (ClickFloorTile, ClickDoor) so a future reader sees the full set. - Strengthens the subtype 2 / 5 callouts with TODO markers so they grep easily. Comment-only — no behavior change. Addresses review comment from baughj on PR #7. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
e5fa6d9 to
58f5392
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Multiple retail-compat and quality-of-life fixes that landed across the
fix/retail-compatbranch. Investigation context for each lives inComhaigne/docs/plans/hybrasyl.client/retail-compat-scoping.md.Group flow — restored end-to-end on Hybrasyl + retail.
Chaos.Networking.DisplayGroupInviteConvertervia custom raw0x63deserializer inConnectionManager.HandleDisplayGroupInvite. The upstream converter has itsDeserialize/Serializebranches functionally swapped against the actual Hybrasyl/retail wire shape (Invite=1 reads box info that isn't there; ShowGroupBox=4 ignores box info that is) and reads class slots in the wrong order (Monk before Rogue vs. the actual W/Wiz/R/P/M emission). Subtypes 1 (Ask) and 4 (RecruitInfo) populateDisplayGroupInviteArgscorrectly so downstream code is unchanged; subtypes 2 (Member) and 5 (RecruitAsk) are logged + dropped pending retail packet capture.0x2Fhandler doesn't push a SelfProfile back unless the toggle leaves a group, so wait-for-server desyncs the icon. All three click sites (HUD, profile, F4 settings) now route throughWorldState.UserOptions.Toggle(12)for sync.ZIndex = 10so it floats above StatusBook / GroupTabControl / NPC dialogs (matches DisconnectPopup level).Boards — messageboard signpost click now works on retail (and continues to work on Hybrasyl).
ConnectionManager.ClickFloorTile(x, y)sends the 7-byte coord click[0x43][0x03][u16 x BE][u16 y BE][0x01]with the trailing flag byte set to 1 (floor-aligned anchor convention). Wire shape verified by Comhaigne's0x43RE refresh — without the flag byte retail defaults to 0 (above-tile) and silently drops the signpost dispatch.OnRootDoubleClickfalls back toClickFloorTilewhenGetEntityAtScreenreturns null andTileHasForegroundis true. Matches retail UX where signposts open on double-click; Hybrasyl single-click QoL keeps the existing 5-byteClickTilepath unchanged.ClickArgs/ClickConverter— same pattern as the doors fix'sClickDoor(folds back together post-Phase-3).Walk-crash CTD —
AnimationSystem.GetSteppedWalkOffsetdivided bystartXwithout guardingstartX == 0. WhenStartWalkwas called with a non-cardinal direction (item-induced dialog while walking),WalkStartOffsetwasVector2.Zeroand the nextUpdateframe crashed. Added an early-return guard.Female-invisible body sprite — two-layer rendering bug for female characters in invisible state.
WorldState.AddOrUpdateAislingonly checkedBodySprite.Female/FemaleGhostfor gender, missing theFemaleInvis/FemaleHead/BlankFemalecases. Switched to the canonicalDataUtilities.DetermineGenderhelper covering all 13 enum values.BodySprite.FemaleInvisresolved towb003— but wb003 in khanwad contains wrong art (a "guard outfit"), not an invisible body. mb003 is the correct invisible silhouette and is gender-neutral when invisible (no skin shows), so it composites cleanly under w-prefix overlays. Extended the existing "shield always loads from male archive" carve-out atAislingRenderer:1240/921to also force male archive for the body 'b' layer whenspriteId == 3. Both call sites route through a newForceMaleArchivehelper documenting the wb003 hack and naming the revert condition (a.datfasset pack shipping correct wb003 art).Profile-click UX — reworked single-click on aislings to default
EnableProfileClick = trueand route Self profile through a clean event chain.Outbound packet logging —
GameClient.Sendnow logs opcode + length + first 64 hex bytes viaNoticeDebugLog, mirroring the existing inbound log. Useful for the next retail-compat investigation.Test plan
Verified end-to-end on Hybrasyl QA against
feature/doorsand against retail (da0.kru.com) where applicable:/group <self>on HybrasylGetSteppedWalkOffsetNotes for reviewers
Chaos.Networkingbypasses landing here (group0x63deserializer, boardsClickFloorTile) are the second and third workarounds in thechaos-networking-removal-direction.md"Tactical workarounds (pre-removal)" log. They fold back together with the doors fix'sClickDoor(PR fix(doors): retail-compat door clicks + tile-resolution Y-comp #6) once Phase 3 ownsClickArgs/DisplayGroupInviteArgslocally.ForceMaleArchivebody-'b' carve-out for spriteId 3 is an asset-bug workaround, not a protocol/architecture change. Revert the spriteId-3 branch inForceMaleArchiveonce a.datfasset pack ships correct wb003 art.NoticeDebugLog's own description but kept in for the next retail-compat investigation. Cheap when the log target isn't enabled.Member=2andRecruitAsk=5group subtypes are documented as "log + drop pending retail packet capture" — Hybrasyl never emits these so we have no reference shape; legacy retail must accept some shape but it's never been documented.🤖 Generated with Claude Code