fix(spells): iterate integrants directly instead of trusting displayOrder#1
Open
justinritchie wants to merge 1 commit into
Open
fix(spells): iterate integrants directly instead of trusting displayOrder#1justinritchie wants to merge 1 commit into
justinritchie wants to merge 1 commit into
Conversation
…rder The Beacon sheet's store.spells.displayOrder is a UI-ordering hint that's incomplete on Warlock characters: it omits cantrips and Mystic Arcanum (level 6+) entries, and sometimes whole levels. Diagnosed against a Warlock 13 character that had 44 spells stored as type=Spell integrants but only 14 (L1-L3) referenced from displayOrder. Fix: roll20_list_spells now walks the integrants map directly, filtering by type === 'Spell' and grouping by level. Returns all spells the sheet holds regardless of which spellcasting context (Pact Magic, Mystic Arcanum, subclass) they belong to. Sort: by level ASC, then alphabetical by name within each level (was display-order before, which is now gone). Stable + predictable. Tested against Warlock 13 / Great Old One Patron: returns 44 spells across L0-L6, including Eldritch Blast (L0), Eyebite (L6 Mystic Arcanum), Doomtide (L4), and the full L4-L5 spellbook.
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
1 participant
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
roll20_list_spellswas returning a partial spell list on Beacon characters — Mystic Arcanum spells, cantrips, and most level 4-5 entries were silently missing. The bug surfaced on a Warlock 13 / Great Old One Patron character that had 44 spells stored astype=Spellintegrants but only 14 were being returned.Root cause
getSpellsDisplayOrder()readsstore.spells.displayOrder, which is a UI-ordering hint, not an authoritative spell list. On Warlock sheets in particular it leaves out:These spells exist on the sheet — they just aren't referenced from
displayOrder. They're stored alongside everything else instore.integrants.integrantswithtype: "Spell"and a numericlevelfield.Fix
listSpellsnow walksintegrantsdirectly, filters bytype === "Spell", and groups bylevel. Replaces the oldgrid.forEach((uids, level) => ...)loop. Output sort: by level ASC, then alphabetical by name within each level.Test results
Tested live against the diagnostic Warlock 13 character:
Why not also fix
getSpellsDisplayOrder?The displayOrder grid serves a real purpose for the Beacon UI (preserving the player's drag-and-drop ordering inside each level), so the function stays in
client.tsfor consumers that want UI-ordered output. We just don't trust it for "is this spell on the sheet?" — that's what the integrants map is for.Files changed
src/tools/character.ts—listSpellsbody rewritten to iterate integrants. Net: +27 / -24.🤖 Generated with Claude Code