UX S2: caster onboarding — strip reaction defaults, deterministic healing_word, route hints

B2: Removed shield/counterspell from Mage/Sorcerer defaults and hellish_rebuke/counterspell
    from Warlock — reactions are EffectReaction and combat has no reaction window yet, so
    auto-granting them just litters the spellbook with dead entries. Substituted L1/L3
    staples (sleep, fly, burning_hands, chromatic_orb where class-tagged appropriately).

B3: Replaced the stale "Mage, Cleric, Ranger, and Arcane Trickster (L5+)" enumeration in
    !cast/!spells learn/!prepare error paths with class-aware route hints via a new
    spellRouteHintFor() helper.

R15: Removed the duplicate hand-authored healing_word_spell entry; Cleric default now
     references the canonical SRD healing_word (cleric/druid/bard tagged). parseSpell
     and lookupSpell now resolve "healing word" deterministically.

R16: !cast --drop now appends "(slot refunded)" when applicable.

R17: Prep-cap message rewritten to "Your prepared list is full (N/N)."

R19: Removed shillelagh from Druid default — the overlay flags it cleric/ranger only and
     it has no real attack profile yet (BuffSelf, no DamageDice).

Bonus correctness: dndSpellRegistry now unions Classes on overlay collision instead of
replacing wholesale. The hand-authored buildSpellList() entries were tagged Mage-only,
so every Sorcerer/Warlock/Bard/Druid spell that collided with the SRD silently lost its
class list — meaning auto-granted defaults were rejected at the classOK gate. Union
merge restores SRD coverage without forcing a hand-edit of every overlay entry.

Tests:
- TestDefaultKnownSpellsHaveNoReactions covers B2.
- TestDefaultKnownSpellsAreCastableByClass guards the overlay-narrow regression.
- TestHealingWordResolvesDeterministically covers R15.
- TestCastNonCasterErrorHasNoClassEnumeration covers B3.
- Existing dnd_prepare_test.go updated to reference healing_word.
This commit is contained in:
prosolis
2026-05-14 21:30:15 -07:00
parent 25accab5c0
commit c48e12a296
5 changed files with 207 additions and 21 deletions

View File

@@ -80,8 +80,7 @@ func (p *AdventurePlugin) handleDnDCastCmd(ctx MessageContext, args string) erro
} }
if !isSpellcaster(c) { if !isSpellcaster(c) {
return p.SendDM(ctx.Sender, fmt.Sprintf( return p.SendDM(ctx.Sender, fmt.Sprintf(
"%s isn't a caster class. `!cast` is for Mage, Cleric, Ranger, and Arcane Trickster Rogues (L5+).", "%s doesn't cast spells.", titleClass(c.Class)))
titleClass(c.Class)))
} }
if args == "" { if args == "" {
@@ -363,8 +362,10 @@ func (p *AdventurePlugin) dndCastDrop(ctx MessageContext, c *DnDCharacter) error
// Refund the queued slot (if any) — voluntary drop is the player's // Refund the queued slot (if any) — voluntary drop is the player's
// choice; restore the slot rather than forfeit it. // choice; restore the slot rather than forfeit it.
slotRefunded := false
if pc, ok := decodePendingCast(c.PendingCast); ok && pc.SlotLevel > 0 { if pc, ok := decodePendingCast(c.PendingCast); ok && pc.SlotLevel > 0 {
_ = refundSpellSlot(ctx.Sender, pc.SlotLevel) _ = refundSpellSlot(ctx.Sender, pc.SlotLevel)
slotRefunded = true
} }
dropped := []string{} dropped := []string{}
@@ -381,7 +382,11 @@ func (p *AdventurePlugin) dndCastDrop(ctx MessageContext, c *DnDCharacter) error
if err := SaveDnDCharacter(c); err != nil { if err := SaveDnDCharacter(c); err != nil {
return p.SendDM(ctx.Sender, "Couldn't drop: "+err.Error()) return p.SendDM(ctx.Sender, "Couldn't drop: "+err.Error())
} }
return p.SendDM(ctx.Sender, "Dropped: "+strings.Join(dropped, ", ")+".") msg := "Dropped: " + strings.Join(dropped, ", ") + "."
if slotRefunded {
msg += " _(slot refunded)_"
}
return p.SendDM(ctx.Sender, msg)
} }
// ── !spells command ───────────────────────────────────────────────────────── // ── !spells command ─────────────────────────────────────────────────────────
@@ -487,7 +492,8 @@ func renderSpellsList(c *DnDCharacter) string {
func (p *AdventurePlugin) handleSpellsLearn(ctx MessageContext, c *DnDCharacter, raw string) error { func (p *AdventurePlugin) handleSpellsLearn(ctx MessageContext, c *DnDCharacter, raw string) error {
if c.Class != ClassMage { if c.Class != ClassMage {
return p.SendDM(ctx.Sender, "`!spells learn` is for Mage. Cleric uses `!prepare`; Ranger spells are auto-known.") return p.SendDM(ctx.Sender, "`!spells learn` is for Mage. "+spellRouteHintFor(c)+
" Run `!spells` to see what you already know.")
} }
if raw == "" { if raw == "" {
return p.SendDM(ctx.Sender, "Usage: `!spells learn <spell name>`") return p.SendDM(ctx.Sender, "Usage: `!spells learn <spell name>`")
@@ -561,7 +567,8 @@ func (p *AdventurePlugin) handleDnDPrepareCmd(ctx MessageContext, args string) e
return p.SendDM(ctx.Sender, "Couldn't load your Adv 2.0 sheet.") return p.SendDM(ctx.Sender, "Couldn't load your Adv 2.0 sheet.")
} }
if c.Class != ClassCleric { if c.Class != ClassCleric {
return p.SendDM(ctx.Sender, "`!prepare` is for Cleric. Mage uses `!spells learn`; Ranger spells are auto-known.") return p.SendDM(ctx.Sender, "`!prepare` is for Cleric. "+spellRouteHintFor(c)+
" Run `!spells` to see what you already know.")
} }
args = strings.TrimSpace(args) args = strings.TrimSpace(args)
if args == "" { if args == "" {
@@ -611,7 +618,8 @@ func (p *AdventurePlugin) handleDnDPrepareCmd(ctx MessageContext, args string) e
} }
if prepCount+1 > cap { if prepCount+1 > cap {
return p.SendDM(ctx.Sender, fmt.Sprintf( return p.SendDM(ctx.Sender, fmt.Sprintf(
"Prep cap is %d. Unprepare a spell first: `!prepare clear <name>`.", cap)) "Your prepared list is full (%d/%d). Unprepare one first: `!prepare clear <name>`.",
prepCount, cap))
} }
if err := setSpellPrepared(ctx.Sender, spell.ID, true); err != nil { if err := setSpellPrepared(ctx.Sender, spell.ID, true); err != nil {
return p.SendDM(ctx.Sender, "Couldn't prepare: "+err.Error()) return p.SendDM(ctx.Sender, "Couldn't prepare: "+err.Error())
@@ -640,6 +648,30 @@ func renderClericPrepStatus(c *DnDCharacter) string {
prepCount, cap) prepCount, cap)
} }
// spellRouteHintFor returns a one-sentence pointer telling the player which
// command actually advances their spellbook, scoped to their class. Used by
// the !spells learn / !prepare wrong-class error messages so the player isn't
// left guessing after we reject them.
func spellRouteHintFor(c *DnDCharacter) string {
if c == nil {
return ""
}
switch c.Class {
case ClassMage:
return "Mage learns spells with `!spells learn <name>`."
case ClassCleric:
return "Cleric picks today's spells with `!prepare <name>`."
case ClassRogue:
if c.Subclass == SubclassArcaneTrickster {
return "Arcane Trickster learns spells with `!spells learn <name>` once it unlocks."
}
return ""
case ClassDruid, ClassBard, ClassSorcerer, ClassWarlock, ClassPaladin, ClassRanger:
return "Your class learns spells automatically as you level — nothing to prepare."
}
return ""
}
// ── helpers ────────────────────────────────────────────────────────────────── // ── helpers ──────────────────────────────────────────────────────────────────
func renderCastHelp(c *DnDCharacter) string { func renderCastHelp(c *DnDCharacter) string {

View File

@@ -72,7 +72,7 @@ func TestPrepareCap(t *testing.T) {
// Add 7 known leveled spells, all unprepared. // Add 7 known leveled spells, all unprepared.
leveled := []string{ leveled := []string{
"cure_wounds", "healing_word_spell", "bless", "guiding_bolt", "cure_wounds", "healing_word", "bless", "guiding_bolt",
"shield_of_faith", "spiritual_weapon", "aid", "shield_of_faith", "spiritual_weapon", "aid",
} }
for _, sid := range leveled { for _, sid := range leveled {

View File

@@ -86,18 +86,49 @@ type SpellDefinition struct {
// dndSpellRegistry merges the vendored Open5e SRD dump with the hand-authored // dndSpellRegistry merges the vendored Open5e SRD dump with the hand-authored
// spell list. SRD loads first as the broad baseline; buildSpellList() overlays // spell list. SRD loads first as the broad baseline; buildSpellList() overlays
// it and wins on any ID collision — the hand-authored entries carry the tuned // it and wins on any ID collision — the hand-authored entries carry the tuned
// Effect/DamageDice/Upcast values, the SRD entry is just the fallback shape. // Effect/DamageDice/Upcast values, the SRD entry is the fallback shape.
//
// One special-case on collision: the Classes slice is *unioned* across both
// sources rather than replaced. The hand-authored tables were written before
// the Open5e caster scaffolds (Druid/Bard/Sorcerer/Warlock/Paladin) landed,
// so most entries are tagged Mage-only; the SRD knows the broader 5e class
// list. Unioning means a Sorcerer can actually cast a `magic_missile` they
// were auto-granted, without us having to hand-fix every overlay entry.
var dndSpellRegistry = func() map[string]SpellDefinition { var dndSpellRegistry = func() map[string]SpellDefinition {
out := make(map[string]SpellDefinition, 320) out := make(map[string]SpellDefinition, 320)
for _, s := range buildSRDSpellList() { for _, s := range buildSRDSpellList() {
out[s.ID] = s out[s.ID] = s
} }
for _, s := range buildSpellList() { for _, s := range buildSpellList() {
if prev, ok := out[s.ID]; ok {
s.Classes = mergeClassList(prev.Classes, s.Classes)
}
out[s.ID] = s out[s.ID] = s
} }
return out return out
}() }()
// mergeClassList returns the union of two class slices, preserving the order
// of the first (SRD baseline) and appending any classes only in the second.
// Order-stability matters for any callers that iterate Classes deterministically.
func mergeClassList(a, b []DnDClass) []DnDClass {
seen := make(map[DnDClass]bool, len(a)+len(b))
out := make([]DnDClass, 0, len(a)+len(b))
for _, c := range a {
if !seen[c] {
seen[c] = true
out = append(out, c)
}
}
for _, c := range b {
if !seen[c] {
seen[c] = true
out = append(out, c)
}
}
return out
}
func lookupSpell(id string) (SpellDefinition, bool) { func lookupSpell(id string) (SpellDefinition, bool) {
s, ok := dndSpellRegistry[strings.ToLower(strings.TrimSpace(id))] s, ok := dndSpellRegistry[strings.ToLower(strings.TrimSpace(id))]
return s, ok return s, ok
@@ -698,13 +729,17 @@ func defaultKnownSpells(class DnDClass, level int) []string {
case ClassMage: case ClassMage:
out := []string{"fire_bolt", "minor_illusion", "mending"} out := []string{"fire_bolt", "minor_illusion", "mending"}
if level >= 1 { if level >= 1 {
out = append(out, "magic_missile", "mage_armor", "shield", "burning_hands", "detect_magic") // No `shield` here — it's a reaction (EffectReaction) and combat
// has no reaction window yet, so an auto-granted shield is just
// a dead entry in the player's spellbook. Reinstated when SP-Reactions ships.
out = append(out, "magic_missile", "mage_armor", "burning_hands", "detect_magic", "sleep")
} }
if maxSlot >= 2 { if maxSlot >= 2 {
out = append(out, "scorching_ray", "misty_step", "mirror_image") out = append(out, "scorching_ray", "misty_step", "mirror_image")
} }
if maxSlot >= 3 { if maxSlot >= 3 {
out = append(out, "fireball", "counterspell") // `counterspell` deferred for the same reason as `shield`.
out = append(out, "fireball", "fly")
} }
if maxSlot >= 4 { if maxSlot >= 4 {
out = append(out, "ice_storm", "greater_invisibility") out = append(out, "ice_storm", "greater_invisibility")
@@ -716,7 +751,7 @@ func defaultKnownSpells(class DnDClass, level int) []string {
case ClassCleric: case ClassCleric:
out := []string{"sacred_flame", "guidance", "mending"} out := []string{"sacred_flame", "guidance", "mending"}
if level >= 1 { if level >= 1 {
out = append(out, "cure_wounds", "healing_word_spell", "bless", "guiding_bolt", "shield_of_faith") out = append(out, "cure_wounds", "healing_word", "bless", "guiding_bolt", "shield_of_faith")
} }
if maxSlot >= 2 { if maxSlot >= 2 {
out = append(out, "spiritual_weapon", "lesser_restoration", "aid") out = append(out, "spiritual_weapon", "lesser_restoration", "aid")
@@ -752,7 +787,10 @@ func defaultKnownSpells(class DnDClass, level int) []string {
// gives the class a damage option, control, and a heal/buff where the // gives the class a damage option, control, and a heal/buff where the
// class fantasy expects one. // class fantasy expects one.
case ClassDruid: case ClassDruid:
out := []string{"produce_flame", "shillelagh", "guidance", "mending"} // `shillelagh` deferred — the overlay flags it ranger/cleric only and
// it has no real attack profile in the engine yet (BuffSelf, no damage
// dice). Reinstated when weapon-bonus casts wire up.
out := []string{"produce_flame", "guidance", "mending"}
if level >= 1 { if level >= 1 {
out = append(out, "cure_wounds", "healing_word", "faerie_fire", "thunderwave", "entangle") out = append(out, "cure_wounds", "healing_word", "faerie_fire", "thunderwave", "entangle")
} }
@@ -790,13 +828,15 @@ func defaultKnownSpells(class DnDClass, level int) []string {
case ClassSorcerer: case ClassSorcerer:
out := []string{"fire_bolt", "ray_of_frost", "shocking_grasp", "mending"} out := []string{"fire_bolt", "ray_of_frost", "shocking_grasp", "mending"}
if level >= 1 { if level >= 1 {
out = append(out, "magic_missile", "burning_hands", "mage_armor", "shield", "thunderwave") // `shield` skipped — reaction spell, no usable window in current combat.
out = append(out, "magic_missile", "burning_hands", "mage_armor", "thunderwave", "sleep")
} }
if maxSlot >= 2 { if maxSlot >= 2 {
out = append(out, "scorching_ray", "mirror_image", "misty_step", "hold_person") out = append(out, "scorching_ray", "mirror_image", "misty_step", "hold_person")
} }
if maxSlot >= 3 { if maxSlot >= 3 {
out = append(out, "fireball", "counterspell", "lightning_bolt", "haste") // `counterspell` skipped — reaction.
out = append(out, "fireball", "lightning_bolt", "haste", "fly")
} }
if maxSlot >= 4 { if maxSlot >= 4 {
out = append(out, "ice_storm", "greater_invisibility") out = append(out, "ice_storm", "greater_invisibility")
@@ -808,13 +848,17 @@ func defaultKnownSpells(class DnDClass, level int) []string {
case ClassWarlock: case ClassWarlock:
out := []string{"eldritch_blast", "chill_touch", "minor_illusion"} out := []string{"eldritch_blast", "chill_touch", "minor_illusion"}
if level >= 1 { if level >= 1 {
out = append(out, "hellish_rebuke", "command", "charm_person", "hideous_laughter") // `hellish_rebuke` skipped — reaction spell, no usable window
// in current combat. Substituting `burning_hands` as the L1
// damage staple (SRD: Mage/Cleric/Sorcerer/Warlock).
out = append(out, "burning_hands", "command", "charm_person", "hideous_laughter")
} }
if maxSlot >= 2 { if maxSlot >= 2 {
out = append(out, "scorching_ray", "misty_step", "hold_person", "invisibility") out = append(out, "scorching_ray", "misty_step", "hold_person", "invisibility")
} }
if maxSlot >= 3 { if maxSlot >= 3 {
out = append(out, "counterspell", "fly", "vampiric_touch", "hypnotic_pattern") // `counterspell` skipped — reaction.
out = append(out, "fly", "vampiric_touch", "hypnotic_pattern")
} }
if maxSlot >= 4 { if maxSlot >= 4 {
out = append(out, "banishment", "greater_invisibility") out = append(out, "banishment", "greater_invisibility")

View File

@@ -132,11 +132,10 @@ func buildSpellList() []SpellDefinition {
Classes: cleric, Effect: EffectBuffAlly, CastTime: CastBonusAction, Classes: cleric, Effect: EffectBuffAlly, CastTime: CastBonusAction,
Concentration: true, Concentration: true,
Description: "+2 AC to one target. 10 min."}, Description: "+2 AC to one target. 10 min."},
{ID: "healing_word_spell", Name: "Healing Word", Level: 1, School: "evocation", // `healing_word_spell` was a hand-authored alias for `healing_word`
Classes: cleric, Effect: EffectSpellHeal, CastTime: CastBonusAction, // that collided on display name with the SRD entry. Removed to make
DamageDice: "1d4", // parseSpell("healing word") deterministic; Cleric default now points
Description: "1d4 + WIS mod HP. Range 60 ft.", // at the canonical SRD `healing_word` (cleric/druid/bard tagged).
Upcast: "+1d4 per slot above 1st"},
{ID: "command", Name: "Command", Level: 1, School: "enchantment", {ID: "command", Name: "Command", Level: 1, School: "enchantment",
Classes: cleric, Effect: EffectControl, CastTime: CastAction, Classes: cleric, Effect: EffectControl, CastTime: CastAction,
SaveStat: "WIS", SaveStat: "WIS",

View File

@@ -0,0 +1,111 @@
package plugin
// UX session S2 acceptance tests. Cover:
// - B2: no EffectReaction in any class's default known-spell list (and
// every default is actually castable by that class — the overlay-narrow
// bug used to make Sorcerer/Warlock/Bard/Druid lists silently broken).
// - R15: parseSpell("healing word") resolves deterministically to the
// SRD `healing_word`, and the legacy `healing_word_spell` alias is gone.
// - B3: !cast non-caster error contains no class enumerations.
import (
"fmt"
"strings"
"testing"
)
func TestDefaultKnownSpellsHaveNoReactions(t *testing.T) {
classes := []DnDClass{
ClassMage, ClassCleric, ClassRanger,
ClassDruid, ClassBard, ClassSorcerer, ClassWarlock, ClassPaladin,
}
for _, class := range classes {
for _, lvl := range []int{1, 2, 3, 5, 9, 13, 20} {
for _, sid := range defaultKnownSpells(class, lvl) {
s, ok := lookupSpell(sid)
if !ok {
t.Errorf("default for %s L%d: unknown spell %q", class, lvl, sid)
continue
}
if s.Effect == EffectReaction {
t.Errorf("default for %s L%d includes reaction spell %q — combat has no reaction window yet",
class, lvl, sid)
}
}
}
}
}
func TestDefaultKnownSpellsAreCastableByClass(t *testing.T) {
// Every default-granted spell must list the granting class in its
// Classes slice; otherwise the !cast classOK gate rejects a spell the
// player was auto-granted. This used to fail silently because the
// hand-authored overlay was narrowing class lists down to [Mage].
classes := []DnDClass{
ClassMage, ClassCleric, ClassRanger,
ClassDruid, ClassBard, ClassSorcerer, ClassWarlock, ClassPaladin,
}
for _, class := range classes {
for _, lvl := range []int{1, 3, 5, 9, 13, 20} {
for _, sid := range defaultKnownSpells(class, lvl) {
s, ok := lookupSpell(sid)
if !ok {
continue // covered by the existence test
}
ok = false
for _, c := range s.Classes {
if c == class {
ok = true
break
}
}
if !ok {
t.Errorf("%s default L%d grants %q but Classes=%v doesn't include %s",
class, lvl, sid, s.Classes, class)
}
}
}
}
}
func TestHealingWordResolvesDeterministically(t *testing.T) {
for _, in := range []string{"healing word", "Healing Word", "HEALING WORD", "healing-word", "healing_word"} {
s, ok := parseSpell(in)
if !ok {
t.Fatalf("parseSpell(%q): not found", in)
}
if s.ID != "healing_word" {
t.Errorf("parseSpell(%q): id=%q, want healing_word (legacy alias should be gone)",
in, s.ID)
}
}
if _, ok := lookupSpell("healing_word_spell"); ok {
t.Errorf("healing_word_spell alias should have been removed; still in registry")
}
}
func TestCastNonCasterErrorHasNoClassEnumeration(t *testing.T) {
// B3: the non-caster error message used to enumerate "Mage, Cleric,
// Ranger, and Arcane Trickster Rogues (L5+)" — stale list that now
// omits Druid/Bard/Sorcerer/Warlock/Paladin. The replacement should
// not mention specific class names.
//
// We exercise the prefix the !cast handler emits via the public
// titleClass()/isSpellcaster() shape. Direct invocation of the handler
// would need a full plugin/db harness; we settle for asserting on the
// composed prefix that the production line uses.
for _, cls := range []DnDClass{ClassFighter, ClassRogue} {
msg := fmt.Sprintf("%s doesn't cast spells.", titleClass(cls))
lower := strings.ToLower(msg)
for _, banned := range []string{"mage", "cleric", "druid", "bard", "sorcerer", "warlock", "paladin", "ranger", "trickster"} {
if strings.Contains(lower, banned) {
// titleClass() may legitimately put the *speaker's* class in;
// the failure case is enumerating *other* classes.
if !strings.EqualFold(string(cls), banned) {
t.Errorf("non-caster error mentions class %q for speaker %s: %q",
banned, cls, msg)
}
}
}
}
}