Skip to content

fix: add missing item finding functionality to CharacterModel (#2685)#2737

Open
rinickolous wants to merge 3 commits into
develop/v13-onlyfrom
bugfix/2685
Open

fix: add missing item finding functionality to CharacterModel (#2685)#2737
rinickolous wants to merge 3 commits into
develop/v13-onlyfrom
bugfix/2685

Conversation

@rinickolous
Copy link
Copy Markdown
Collaborator

Fixes #2685

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes the broken global item-finding helpers (GURPS.findSkill, GURPS.findAdDisad, GURPS.findAttack, etc.) by routing lookups through CharacterModel (and a small adapter) instead of calling methods directly on the Actor document, addressing #2685.

Changes:

  • Added src/module/util/find-item.ts to resolve an Actor document to a CharacterModel and provide findSkill/findSpell/findSkillSpell/findAdDisad/findAttack helpers.
  • Implemented skill/spell lookup methods on CharacterModel (regex-based, best-level selection per list).
  • Updated src/module/gurps.js to expose these helpers on the GURPS global and use them in OTF “test-exists” handling.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/module/util/find-item.ts New adapter/helper module that resolves Actor→CharacterModel and exposes item-finding functions used by GURPS.*.
src/module/gurps.js Wires the new helpers into the GURPS global API and replaces removed in-file implementations.
src/module/actor/data/character.ts Adds CharacterModel lookup methods for skills/spells (and a shared regex builder) used by the new global helpers.
Comments suppressed due to low confidence (1)

src/module/util/find-item.ts:26

  • New item-finding behavior is introduced via find-item.ts and the new CharacterModel lookup methods, but there are no unit tests covering the key cases that caused #2685 (passing an Actor document vs actor.system, and handling null/undefined actor inputs). Adding a small Vitest suite for these helpers would help prevent regressions in global APIs like GURPS.findSkill/findAttack.
export function findSkill(actor: CharacterOrActor, name: string): Item.OfType<ItemType.Skill> | null {
  return resolveCharacterModel(actor)?.findSkill(name) ?? null
}

/* ---------------------------------------- */

export function findSpell(actor: CharacterOrActor, name: string): Item.OfType<ItemType.Spell> | null {
  return resolveCharacterModel(actor)?.findSpell(name) ?? null
}

Comment thread src/module/util/find-item.ts Outdated
Comment thread src/module/actor/data/character.ts Outdated
rinickolous and others added 2 commits May 16, 2026 17:47
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Base automatically changed from bugfix/2734 to develop/v13-only May 29, 2026 20:04
Comment thread src/module/gurps.js
return item
}

GURPS.findAdDisad = findAdDisad
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module utils should add these methods to the GURPS global in its init() method.

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.

V1.0.0 GURPS.findSkill, GURPS.findAdDisad and GURPS.findAttack throws error

3 participants