Skip to content

Feature/character lifecycle - #129

Open
T4tsuk3i wants to merge 8 commits into
stanuwu:masterfrom
T4tsuk3i:feature/character-lifecycle
Open

T4tsuk3i wants to merge 8 commits into
stanuwu:masterfrom
T4tsuk3i:feature/character-lifecycle

Conversation

@T4tsuk3i

@T4tsuk3i T4tsuk3i commented Sep 18, 2026 •

Copy link
Copy Markdown

What this adds

Character creation and deletion from the character-select screen,
against 0.5.1's investment-store persistence.

Update after review

The review point was that the select-screen hold re-added a deleted client
hook and should be a server implementation instead. The hook
(character_select_hold.cpp, its lifecycle registration, declarations and
project entry) is removed, and the hold now comes from what the server does.

The client was leaving the select screen because the server invented a pick
the player never made. create_character marked the new character selected,
and delete_character handed the selection to a survivor. Moving the account
onto a character is what a real pick (opcodes 504/505) does, so the step had
nothing left to wait for.

  • Create onto a roster that already has characters, and delete, change the
    roster without selecting anything. Only the player picks.
  • The opcode 501/502 handlers no longer stage a SelectCharacter Family-4
    move for that invented pick. They arm the account resync they already arm
    and name the revision it publishes (family4Version + 1), the same "name
    the following resync" contract the claim path uses, so the reply names a
    publication that actually arrives.
  • With nothing selected now a real, expected state, the Family-0 appearance
    and Family-3 roster resync lanes return without publishing instead of
    failing. The Family-4 account frame still goes out, so the roster count
    moves immediately.

The bugs, and how each was found

1. Delete disconnected the client with a Bungie network error.
opcode 502 answered with a bare echo, not the status-pair shape
middleware::web_service::encode_response produces everywhere else.
opcode 501 answered with the right shape but hardcoded
status.value = kNoFamily4Publication unconditionally, correct for its
original "echo the already-selected character" use, wrong once the
request actually mutates the roster. The client blocks after these
requests waiting for a Family-4 publish at the version the response
names; either bug leaves it waiting on one that never comes, and it
times out into a disconnect.

Fixed by moving the handling into bap_service_body.cpp next to the
existing opcode 505 (change-character) block, and encoding a real
status pair. The log shows a clean ev=ws502 stage=delete result=ok /
ev=ws501 stage=create result=ok pair for every request.

2. Created character never actually appeared. With (1) fixed, create
returned success but the character never showed up client-side. The log
showed a tight, permanent retry loop instead:

server level=warn ev=family4 stage=prepare result=fail step=character_encode
server level=warn ev=queuez stage=peer_resync result=fail reason=family4

repeating every ~50ms, forever. Tracing the failing character::encode
call into equipment_summary_builder.cpp found the actual guard:
evaluation.divisor <= 0 unconditionally rejects the encode. divisor
is the count of the 8 weapon/armor slots with something equipped; a
created character starts with none, so it can never pass this check and
can never be published.

Fixed by seeding all 17 native equipment slots on creation, sourced from
this build's own authored default character's item hashes
(resources/database/investment_defaults.sql), each given a freshly
allocated instance SOID via the same collision-avoidance scan
ensure_character_subclasses already uses.

3. Every created character had the same gear regardless of class.
The fix above initially reused one fixed 17-item set for every class. A
Hunter came out wearing a Titan subclass and Titan armor. Fixed by
splitting the loadout into 10 slots that are class-agnostic (weapons and
cosmetic/utility slots) and 7 that are not (armor, class item, subclass,
and the emblem variant), chosen per class from known item definition
hashes.

4. Create/delete loaded straight into orbit instead of refreshing the
roster.
Removing the Family-4 publish staging entirely had no effect on
this, which showed the response was not the cause. The cause was the
server moving the selection on create and delete, as described in the
update above. A deleted character that logged ev=ws502 stage=delete result=ok was followed within a second by successfully changed world to: orbit_d2 and falling back to a new character.

Existing bug in Sunrise since 0.3

The head doesn't render on a created character, independent of the
show/hide-helmet setting. Race/gender/class all display correctly on the
character-select screen, so identity data is going through; this looks like
a narrower rendering issue, not a data problem. Not root-caused yet.

Verification

Built via clang-cl/lld-link cross-compiling to x86_64-pc-windows-msvc, then
deployed and tested in-game. No client hook is installed in the tested
build.

  • With characters on the roster, the select screen stays up at boot until
    the player acts (about 54 s in one run, no hook, no setup:orbit).
  • Deleting a character leaves the select screen up and the roster one
    shorter, with ev=ws502 stage=delete result=ok and no orbit transition.
  • Creating a character from the select screen leaves it up, with
    ev=ws501 stage=create result=ok.

Not covered by this build's test runs: picking a character after a
create or delete, and a relaunch with several characters.

The game does not offer deleting the last remaining character, so a
delete-everything path is not exercised through the UI.

## Summary

0.5.0's opcode-501 handler is a stub: it returns whatever character is
already selected and never parses the request body, so the create-character
flow does nothing. Opcode-502 (delete) doesn't exist at all. This makes
both real:

- state::create_character / state::delete_character (new,
  state_account_roster_runtime.cpp) do the actual roster mutation: pick an
  unused SOID offset from the account's own primary key (reusing a freed
  SOID after a delete without ever colliding with a living character),
  build the new character, validate the whole account, and persist it --
  following the same lock/read/validate/write/unlock shape already used by
  set_selected_character and ensure_character_subclasses in this file.
- opcode501_request_codec parses the create-character request's race/
  gender/class identity triple from its bit-packed payload.
- opcode502 (new) parses the bare 64-bit delete-character target id.
- web_service_runtime.cpp wires both into the dispatch that used to just
  echo opcode501 back and never handle opcode502 at all.

## Scope note

This build has no per-character authored appearance to capture and no
settings-authored default-loadout template to seed from -- both fields dev
tooling elsewhere relies on don't exist on this CharacterState. A created
character starts with an empty loadout, the same fallback path this
account model already uses when no template matches. Capturing the
player's authored face and seeding a starting loadout are natural
follow-ups, not included here.

## Effects

- Creating a character now actually adds a row to the roster and persists
  it; deleting one now actually removes it, preserving surviving
  characters' SOIDs (an object the client already holds under an old SOID
  never gets silently renamed).
- Verified: fresh clean build (CMake + clang-cl, Release x64) links with
  zero warnings from any of the new or modified files.
The real client blocks after opcode 501/502 waiting for a Family-4
publish at the version the response names. The stock opcode501 codec
hardcoded that value to kNoFamily4Publication (correct for its original
"echo the selected character" use, wrong once the request actually
creates or deletes a character), and opcode502 answered with a bare
echo with no version at all. Both left the client waiting on a
publication that was never staged, which times out into a disconnect.

Moved the actual opcode 501/502 handling into bap_service_body.cpp,
alongside the existing opcode 505 change-character block it mirrors:
stage a queuez::SelectCharacter transaction onto the new/surviving
character, encode the real staged Family-4 version in the response,
and arm a full account resync as a safety net if staging fails or the
frame gets dropped. opcode501_codec::encode_response now takes the
status pair as a parameter instead of hardcoding it, so the existing
stock call site (character-select echo) keeps its original behavior
explicitly instead of implicitly.
The Family-4 character encoder refuses to publish a character with
nothing equipped: build_equipment_summary rejects a light-evaluation
divisor of zero (nothing to average), which is exactly what an
empty-equipment character produces. A created character therefore
existed in the database but could never actually be sent to the
client, leaving the server retrying the same failing encode forever
and the character never appearing.

Seeds all 17 native equipment slots with the same items this build's
own single authored default character already carries (copied from
investment_defaults.sql's seed rows), each given a freshly allocated
instance SOID so multiple created characters never collide. Item
definitions carry no class restriction in this codebase, so the same
proven-valid set is safe regardless of the requested class.
… set

The starter loadout added in the previous commit granted every created
character the same 17 items, sourced from this build's one single
authored default character. That character is a fixed class, so every
other class got its gear and subclass -- a Hunter equipped with a
Titan subclass and Titan armor.

Item definitions carry no class-restriction field in this codebase, so
nothing caught this at the data layer, but the real client still
renders subclass abilities/animations and armor per class. Split the
loadout into 10 slots that are genuinely class-agnostic (weapons,
ghost, ship, vehicle, clan banner, emote, finisher, artifact -- cross-
checked identical across all three classes in the T4tsuk3i fork's own
per-class templates) and 7 that are not (helmet, gauntlets, chest,
legs, class item, subclass, and the class-flavored emblem variant),
now indexed by the created character's own class.
…rbit

Without this, the sign-in boot step's enter handler clears its own stay
flag on the first frame, so the client always falls straight through the
character-select screen into orbit -- with whatever character happens to
be selected -- immediately after create or delete rather than showing the
updated roster. This is the piece client-stability-fixes carries that
character-lifecycle actually depends on for its own responses (which
correctly say nothing needs to be waited on) to be honored: the client
has to still be on the character-select step for "stay here" to mean
anything.

Ported standalone rather than pulling in the rest of the client-
stability-fixes branch, which bundles five unrelated bootflow fixes this
feature doesn't need.
@stanuwu

stanuwu commented Sep 27, 2026

Copy link
Copy Markdown
Owner

this adds back a deleted client hook, should use a proper server implementation instead

@T4tsuk3i

Copy link
Copy Markdown
Author

ok will do working on it might take a bit of time

Both character encoders (family-4 and the family-3/family-0 character
record) wrote a fixed 36-byte header for every character, unconditionally
-- authored client-side ids, not extracted data, identical for every
character regardless of race or gender. Real per-character appearance
(face, hair, skin, etc.) was never captured: the creation request's
appearance header was parsed only far enough to read race/gender/class,
then the remaining 234 bits were explicitly skipped.

Adds the missing half: parses the full appearance header from the
creation request (8/16/32-bit fields, three different biases, mapped
into the record's own little-endian byte layout), stores it per
character, and has both encoders use it when present, falling back to
the fixed header only for a character that predates this (existing
saves keep working via a schema migration).

Persistence: appearance_header/appearance_header_valid added to the
characters table (schema v2 -> v3, migrated in place for existing
databases). The generic Statement bind/column helpers gained BLOB
support for fixed-size byte arrays, since nothing previously stored in
this database needed anything but integers, floats, or text.
Stan's review on the select-screen hold is that it re-adds a deleted client
hook and should be a server implementation instead. The hook is deleted here;
the hold is carried by what the server publishes.

The client was walking off the screen because the server invented a pick the
player never made:

- create_character marked the new character selected and cleared every other
  flag, and delete_character handed the selection to a survivor whenever the
  deleted slot held it. Moving the account onto a character by itself is what
  the real pick (opcodes 504/505) does, so the step had nothing left to wait
  for. Both now change the roster without selecting anything; only the player
  picks.
- The opcode 501/502 handlers staged a queuez::SelectCharacter Family-4 move to
  publish that invented pick. They now arm the account resync they already arm
  and name the revision it publishes (queuezState.family4Version + 1) -- the
  same "name the following resync" contract the claim path below them uses --
  so the reply names a publication that actually arrives instead of one that
  moves the selection.

Two account-resync companions treated "nothing selected" as a failure. That is
now a real, expected state (it is what the select screen is), so the Family-0
appearance and Family-3 roster lanes return without publishing instead of
failing, which also stops the ability-refresh retry lane from re-running
against a screen that is waiting for its own pick. The Family-4 account frame
still goes out, so the roster count moves immediately.

Removes Sunrise/src/client/hooks/bootflow/character_select_hold.cpp, its
lifecycle registration, its declarations, and its project entry.
@T4tsuk3i

Copy link
Copy Markdown
Author

@stanuwu thanks for the review. The client hook is removed and the select-screen hold is now done on the server: the server no longer selects a character on create or delete, so the step has no pick to leave on. I rewrote the description above to match. Could you take another look?

@T4tsuk3i
T4tsuk3i force-pushed the feature/character-lifecycle branch from 4d43259 to 34a9c6a Compare October 12, 2026 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants