Commit graph

3 commits

Author SHA1 Message Date
Erik
ed30808720 fix(net): FA1 review round -- zero-id tree rejection, monarch clear, 0x001F builder
Applies both MUST-FIX items and the code-facing SHOULD-FIX items from the
dual-lens FA1 review (docs/research/2026-08-12-fa1-review-mechanism.md,
docs/research/2026-08-12-fa1-review-blast.md):

Mechanism MF-1 / blast SF-2: AllegianceHierarchy::Add @0x005B6E90 wraps its
entire body in `if (_id != 0)` -- a record whose own id is zero discards
the WHOLE message, for both the monarch and a child record, and this is
also what makes treeParent == 0 unconditionally fatal for a non-monarch
record. ReadAllegianceProfileBody now rejects CharacterId == 0 on both
paths; four new boundary tests in AllegianceProfileVersionGateTests.cs
(zero-id monarch, zero-id child, zero treeParent, plus the existing
orphan/self-parent/duplicate trio).

Mechanism MF-2: added the missing 0x001F AllegianceUpdateRequest builder --
the structural twin of the fellowship 0x00A6 this slice already repaired --
with golden-vector tests for both on/off.

Mechanism SF-1 / blast SF-3: UnPack's last act before returning success
forces the monarch's MayPassupExperience to false regardless of the wire
bit or the HasPackedLevel-absent legacy-compat fallback. Ported at the end
of the record loop; the pre-existing HasPackedLevel-absent test moved off
the monarch record (which the new clear makes indistinguishable from "the
fallback never fired") onto a vassal record, and a new test proves the
monarch clear fires even when the wire bit explicitly asks for true.

Mechanism SF-2: removed ParseFellowshipDisband's invented body-length
validation -- retail's DispatchUI_Disband reads only the opcode and never
inspects a trailing body. The parser now always succeeds; the matching
test flips from asserting rejection to asserting acceptance.

Mechanism SF-3: added the D5 `<<1` shareLoot-shape test at the 0x02C0
FellowshipUpdateFellow site -- previously only pinned at 0x02BE, so a
future split of the shared ReadFellow helper could silently reintroduce a
bool read on this leg undetected.

Mechanism SF-5: renumbered the version-gate comments in
ReadAllegianceProfileBody to the true AllegianceVersion enum values
(1-11, matching acclient.h's SpokespersonAdded..ApprovedVassal) instead of
wire-appearance order, which only reached 10 and silently dropped gate 5
(BannedCharactersAdded, which is real but gates nothing in UnPack -- now
called out explicitly). Fixed the stale "lane B §12" citation in
SocialActions.cs to the actual master-table row.

Blast SF-1: pinned the two retail-faithful but user-visible behavior
changes FA1 made to the ALREADY-LIVE `@allegiance info` command --
reversed vassal print order (3-vassal test through
FormatAllegianceInfoLines) and malformed-tree silent-drop (test at the
GameEventWiring registration layer, which is `if (info is null) return;`).

Blast SF-4: fixed a doc comment citing a nonexistent `ConfirmationResponseTests`
class; the actual class is `ConfirmationTripleTests`.

Blast SF-5: cross-referenced the confirmation-triple discriminator's split
representation (ConfirmationType on the response leg only; bare uint on
the two inbound legs production actually reads) at both sites, so FA4
inherits a stated decision rather than an unexplained inconsistency.

Full Release suite: 13,158 passed / 4 skipped / 0 failed (13,162 total),
up from the pre-fix-round 13,149/4/0 (+9 tests this round).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-12 00:46:42 +02:00
Erik
7be86f47f6 fix(net): FA1 -- repair fellowship builders, add 0x0290/0x0291, allegiance Kick
Campaign FA slice FA1 (lane B field-order sections §3.1-§3.7, lane C §3.2,
§1.4). Two latent acdream builder defects repaired, both cited in lane B
§5.2:

- BuildFellowshipCreate (0x00A2) invented a nonexistent "openness" byte
  and silently sent it as the low byte of shareXP -- ACE would read an
  INVERTED shareXP value. Corrected to retail's real shape: [str16L
  name][u32 shareXP]. shareXP is the FellowshipShareXP character option,
  not a dialog checkbox.
- BuildFellowshipUpdate(open:) mislabeled 0x00A6 as fellowship openness;
  it is FellowshipUpdateRequest -- panel VISIBILITY. Renamed to
  BuildFellowshipUpdateRequest(panelOpen:); the wire bytes were already
  correct, only the name/doc were wrong. ACE gates the whole 0x02C0
  member-vitals stream on this message (lane B §4.5) -- a prerequisite
  for live vitals once FA4 wires the panel.

Two builders added that acdream never had at all:

- BuildFellowshipAssignNewLeader (0x0290) -- retail's leader-Quit path
  sends this before 0x00A3 disband=0 (lane B §2.5).
- BuildFellowshipChangeOpenness (0x0291) -- the REAL openness toggle.
- AllegianceRequests.BuildKick -- wire-identical to BuildBreak (both are
  Event_BreakAllegiance 0x001E); named separately so FA2's panel command
  surface can distinguish "break from patron" from "kick a vassal" (lane
  C §1.4). AllegianceInfoRequest (0x027B) was already live via
  ClientCommandRequests.BuildAllegianceInfoRequest -- not duplicated.

Wrong-shape tests at SocialActionsTests.cs:53-105 re-pinned with
hand-computed golden byte vectors deriving each field from the cited
lane-B sections (not generated by calling the builder under test, per the
OP1 convention this file already follows for BuildSetCharacterOptions).
AllegianceRequestsTests.cs gained golden vectors for the existing
Swear/Break builders plus the new Kick alias.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-12 00:12:47 +02:00
Erik
62cf755e7d feat(allegiance): Phase H.2 AllegianceRequests + AllegianceTree model
Client-side allegiance data model + outbound swear/break actions.
The inbound AllegianceUpdate blob (0x0020) is complex and is deferred;
the tree API here is designed so the handler can push nodes in when
the blob parser lands.

Wire layer:
- AllegianceRequests.BuildSwear (0x001D): single uint32 patronGuid.
- AllegianceRequests.BuildBreak (0x001E): single uint32 targetGuid
  (works for both breaking from patron and breaking away a vassal;
  the server picks behavior based on the relationship).

Core layer (AcDream.Core/Allegiance):
- AllegianceNode: Guid, Name, PatronGuid, Rank (clamped 0..10),
  VassalGuids list.
- AllegianceTree: Dictionary-backed, events on TreeChanged.
  - SetMonarch: registers the root (no patron).
  - UpsertNode: adds/refreshes + auto-inserts into parent's vassal list.
  - RemoveNode: removes from parent list too; descendants are left with
    dangling patron pointers for the UI to hide (next AllegianceUpdate
    refreshes).
  - GetAncestors: walks up to monarch, cycle-detected for defense.
  - GetDescendants: BFS-order flattening.
- AllegianceMath.ComputePassup: retail XP formula
  (50+22.5×loyalty)/291 × (1+RT/730×IG/720) × earned,
  clamped at 0.

Tests (11 new):
- Tree: SetMonarch fires TreeChanged, UpsertNode auto-populates parent
  vassal list, rank clamp at 10, RemoveNode cleans parent list,
  GetAncestors chain, cycle-safe walk, GetDescendants BFS order.
- Math: Passup known-value check (1000 XP, 10 loyalty, 100 RT/IG
  days → ~963 XP), negative clamp.
- Wire: Swear + Break byte-exact encoding.

Build green, 613 tests pass (up from 602).

Next: wire inbound AllegianceUpdate (0x0020) + AllegianceInfoResponse
(0x027C) handlers once the blob parser lands. Chat "Allegiance"
Turbine channel joining (r11 §2.1 step 9) layers on top of
Phase H.1 chat infrastructure.

Ref: r11 §1 (tree structure + rank cap), §2 (swear/break wire),
§3.2 (XP passup formula).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-18 17:17:45 +02:00