diff --git a/gogobee_legacy_migration.md b/gogobee_legacy_migration.md index 30b7634..bca2b6f 100644 --- a/gogobee_legacy_migration.md +++ b/gogobee_legacy_migration.md @@ -426,31 +426,33 @@ Each sub-phase is the same shape as L4a–L4e: column-add → backfill (idempote ### 7.4 Final teardown (after L5a–L5h) -After all eight sub-phases ship and soak (one week post-cut-of-AdvCharacter-writes per §11): +**Reconciled 2026-05-09.** The original deletion list was wrong. Two strategy switches during execution invalidated it: -1. Re-run §7 greps — both must return 0 (modulo `dnd_l5_cleanup.go` archive code). -2. Delete files listed below. -3. Drop `adventure_character` table behind `GOGOBEE_LEGACY_PURGE=1` env gate. +1. **L5h preserved `AdventureCharacter` as a load-view struct.** Rather than delete 51 `saveAdvCharacter` call sites, L5h rewrote `saveAdvCharacter` as a fan-out into `player_meta` and added `applyPlayerMetaOverlay` at the tail of `loadAdvCharacter`. The struct is the in-memory shape every caller expects; only the *backing table* goes cold and eventually drops. `adventure_character.go` stays. +2. **`combat_engine.go` / `combat_bridge.go` / `combat_stats.go` are not legacy.** A 2026-05-09 audit confirmed `SimulateCombat`, `CombatStats`, `CombatModifiers`, `Combatant`, `CombatPhase`, `CombatResult`, `CombatEvent`, `defaultCombatPhases`, `dungeonCombatPhases`, `DerivePlayerStats`, `DeriveDungeonMonsterStats`, `runDungeonCombat`, `transitionDeath`, `applyXPBonuses`, `combatDegradation`, etc. are referenced across 21+ files (arena, dungeons, zones, expedition, plus every D&D combat test). The D&D system layers *on top of* this engine via `applyDnDPlayerLayer` / `applyDnDEquipmentLayer` / `applyDnDArenaMonsterLayer` / `applyDnDDungeonMonsterLayer` — it was never a replacement. There is nothing to migrate; these files are shared infrastructure and stay. + +**Revised teardown checklist:** + +After L5a–L5h soak (one week post-cut-of-AdvCharacter-writes per §11): + +1. **Three remaining `CombatLevel` reader fixes** (post-L5g the legacy fallback is the only source still feeding them): + - `adventure_babysit.go::babysitDailyCost(char.CombatLevel)` → read D&D level. + - `dnd_sheet.go:168` legacy stat-block line displaying `adv.CombatLevel` → drop or swap to D&D level. + - `dnd_onboarding.go:27,38` gate on `advChar.CombatLevel < dndLegacyMinLevel` → re-evaluate purpose now that L5g guarantees every active user has a D&D row. +2. Drop `adventure_character` table behind `GOGOBEE_LEGACY_PURGE=1` env gate (`dnd_l5_cleanup.go` migration script, default off). +3. Drop `player_meta.combat_level` column (introduced as a transitional column in L5a; redundant once readers no longer consult it). 4. Final `go vet ./... && go test ./...` clean. -**Files deleted:** -- `adventure_character.go` -- `adventure_activities.go` (already gutted in L1) -- `combat_engine.go` -- `combat_engine_test.go` -- `combat_bridge.go` -- `combat_bridge_test.go` -- `combat_stats.go` (if unused; verify) -- `combat_stats_test.go` +**What stays:** +- `adventure_character.go` — `AdventureCharacter` struct continues as the loaded-view shape; `loadAdvCharacter` SELECTs from the (now read-only) `adventure_characters` seed row and overlays `player_meta` / `dnd_character` state. A future cosmetic pass may rename it to drop the implication of being legacy, but that is *not* part of this migration. (Per `feedback_avoid_dnd_naming.md`, any rename must avoid "dnd"-prefixed names.) +- `adventure_activities.go` — already gutted in L1; the file's residual stubs can stay or be sweep-deleted in a separate cleanup, not part of teardown. +- `combat_engine.go`, `combat_bridge.go`, `combat_stats.go` (+ tests) — shared combat infrastructure. +- `AdvLocation` tier data and `AdvBonusSummary` plumbing — both still consumed by the live combat path; not "legacy" in the L1–L5 sense. **DB cleanup:** -- `adventure_character` table: drop after a 30-day grace period in case of rollback. Schedule with a migration script `dnd_l5_cleanup.go` that runs only when env var `GOGOBEE_LEGACY_PURGE=1` is set, to gate the destructive op. Default off; manual flip after confirming. +- `adventure_character` table: drop after a 30-day grace period in case of rollback. `dnd_l5_cleanup.go` migration script gated on `GOGOBEE_LEGACY_PURGE=1`. Default off; manual flip after confirming. - Equipment table stays — it was always per-user, never per-AdvCharacter. - -**Code cleanup:** -- Remove `AdvLocation` tier data unless something else picked it up. -- Remove all `AdvBonusSummary` plumbing. -- Final `go vet ./... && go test ./...` clean. +- `player_meta.combat_level` column drop sequenced with the three reader fixes above. **Branching dungeon paths:** unblocked. The `dnd_zone_run` schema is the only one we still own; rebuild/expand it without AdvCharacter pressure. Out of scope for this doc — see `project_branching_paths_post_legacy.md` memory note. diff --git a/internal/plugin/adventure_babysit.go b/internal/plugin/adventure_babysit.go index 4e1c2af..0ceb4b6 100644 --- a/internal/plugin/adventure_babysit.go +++ b/internal/plugin/adventure_babysit.go @@ -13,8 +13,13 @@ import ( // ── Pricing ───────────────────────────────────────────────────────────────── -func babysitDailyCost(combatLevel int) int { - return 100 + (combatLevel * 20) +// babysitDailyCost returns the daily babysit subscription cost in €. +// Phase L (post-L5g): keyed off D&D Level instead of legacy CombatLevel. +// The slope is 5× the old per-level slope to preserve the curve shape across +// the 5:1 compression in dndLevelFromCombatLevel — Level 4 (~old CL 20) = +// €500/day, Level 10 (~old CL 50) = €1100/day, matching pre-migration pricing. +func babysitDailyCost(level int) int { + return 100 + (level * 100) } // ── Pet-care daily trickle ───────────────────────────────────────────────── @@ -120,7 +125,7 @@ func (p *AdventurePlugin) handleBabysitPurchase(ctx MessageContext, days int) er return p.SendDM(ctx.Sender, "Your adventurer is dead. The babysitter does not work with corpses.") } - daily := babysitDailyCost(char.CombatLevel) + daily := babysitDailyCost(dndLevelForUser(char.UserID)) totalCost := daily * days balance := p.euro.GetBalance(char.UserID) if balance < float64(totalCost) { diff --git a/internal/plugin/combat_bridge.go b/internal/plugin/combat_bridge.go index 2b08dcf..bf5d96d 100644 --- a/internal/plugin/combat_bridge.go +++ b/internal/plugin/combat_bridge.go @@ -67,14 +67,11 @@ func (p *AdventurePlugin) runDungeonCombat( enemyStats, enemyMods := DeriveDungeonMonsterStats(loc) // All combat is D&D. Auto-migrate if needed. - dndChar, freshMigrate, err := ensureDnDCharacterForCombat(userID, char) + dndChar, _, err := ensureDnDCharacterForCombat(userID, char) if err != nil { slog.Error("dnd: ensureDnDCharacterForCombat (dungeon) failed", "user", userID, "err", err) return CombatResult{} } - if freshMigrate { - p.maybeSendDnDOnboarding(userID, char, dndChar) - } applyDnDPlayerLayer(&playerStats, dndChar) applyDnDEquipmentLayer(&playerStats, dndChar, equip) applyDnDHPScaling(&playerStats, dndChar) diff --git a/internal/plugin/dnd_audit_fixes_test.go b/internal/plugin/dnd_audit_fixes_test.go index adc38ad..d395b5b 100644 --- a/internal/plugin/dnd_audit_fixes_test.go +++ b/internal/plugin/dnd_audit_fixes_test.go @@ -200,37 +200,6 @@ func TestArm_SaveThenSpend_HappyPath(t *testing.T) { } } -// ── Fix F: onboarding sent exactly once across draft cancel/rebuild ──────── - -func TestOnboarding_PersistsAcrossRespec(t *testing.T) { - setupAuditTestDB(t) - uid := id.UserID("@onboard_persist:example") - c := &DnDCharacter{ - UserID: uid, Race: RaceHuman, Class: ClassFighter, Level: 3, - STR: 16, DEX: 13, CON: 14, INT: 8, WIS: 10, CHA: 12, - HPMax: 30, HPCurrent: 30, ArmorClass: 16, - OnboardingSent: true, // already received DM - } - if err := SaveDnDCharacter(c); err != nil { - t.Fatal(err) - } - got, _ := LoadDnDCharacter(uid) - if !got.OnboardingSent { - t.Fatal("OnboardingSent didn't round-trip") - } - - // maybeSendDnDOnboarding must NOT re-send. - p := &AdventurePlugin{} - advChar := &AdventureCharacter{UserID: uid, CombatLevel: 20} - p.maybeSendDnDOnboarding(uid, advChar, got) - - // Flag must remain set; no error. - got2, _ := LoadDnDCharacter(uid) - if !got2.OnboardingSent { - t.Error("OnboardingSent flag flipped off") - } -} - // ── Fix G: Persuasion shop discount expires after 30 minutes ──────────────── func TestPersuasionDiscount_ExpiresAfterTTL(t *testing.T) { @@ -267,11 +236,10 @@ func TestPersuasionDiscount_ActiveWithinTTL(t *testing.T) { // ── Fix I: HP scaling clamp ───────────────────────────────────────────────── -// ── Onboarding gap: !setup-first path and wrapper for non-combat handlers ── +// ── Wrapper for non-combat handlers ────────────────────────────────────── // TestEnsureCharForDnDCmd_FreshMigrate — the wrapper used by !check, !stats, -// !level, !rest, !arm should auto-migrate AND record OnboardingSent=true -// for legacy players. +// !level, !rest, !arm should auto-migrate any legacy player without a D&D row. func TestEnsureCharForDnDCmd_FreshMigrate(t *testing.T) { setupAuditTestDB(t) uid := id.UserID("@wrapper_legacy:example") @@ -284,9 +252,6 @@ func TestEnsureCharForDnDCmd_FreshMigrate(t *testing.T) { t.Fatal(err) } - // Plugin without Matrix client — maybeSendDnDOnboarding short-circuits. - // We only verify that the wrapper produces a character; the DM-side - // short-circuit means OnboardingSent stays false until a real send fires. p := &AdventurePlugin{} c, err := p.ensureCharForDnDCmd(uid, advChar) if err != nil || c == nil { @@ -300,67 +265,32 @@ func TestEnsureCharForDnDCmd_FreshMigrate(t *testing.T) { } } -// TestSetupStatus_LegacyWelcomeStubsRow — when a legacy player runs `!setup` -// for the first time, we save a stub draft with OnboardingSent persisted so -// future paths don't re-fire the welcome. -// -// SendDM no-ops in tests (nil Client), so OnboardingSent stays 0 on the -// stub. The behavior we verify: a stub row gets saved on first !setup, and -// it's flagged PendingSetup=1 so loadOrInitDraft will continue the flow. -func TestSetupStatus_StubRowSaved(t *testing.T) { +// TestSetupStatus_NoStubOnFirstSetup — `!setup` for a player without a D&D +// row must NOT pre-create one; the row is created only on `!setup confirm`. +// (Pre-L5g this branch saved a stub for legacy players to carry the +// onboarding-DM flag; that mechanism has been retired.) +func TestSetupStatus_NoStubOnFirstSetup(t *testing.T) { setupAuditTestDB(t) - uid := id.UserID("@setup_first:example") - if err := createAdvCharacter(uid, "setup_first"); err != nil { - t.Fatal(err) + for _, uid := range []id.UserID{"@setup_legacy:example", "@setup_new:example"} { + if err := createAdvCharacter(uid, string(uid)); err != nil { + t.Fatal(err) + } } - advChar, _ := loadAdvCharacter(uid) - advChar.CombatLevel = 30 - saveAdvCharacter(advChar) + + // Legacy player (CombatLevel = 30) — used to get a stub. + advLegacy, _ := loadAdvCharacter("@setup_legacy:example") + advLegacy.CombatLevel = 30 + saveAdvCharacter(advLegacy) p := &AdventurePlugin{} - if err := p.dndSetupStatus(MessageContext{Sender: uid}); err != nil { - t.Fatal(err) - } - - // Stub row should now exist. - got, err := LoadDnDCharacter(uid) - if err != nil || got == nil { - t.Fatalf("expected stub row after !setup; got err=%v", err) - } - if !got.PendingSetup { - t.Error("stub row should be pending_setup=1") - } - if got.Race != "" || got.Class != "" { - t.Errorf("stub row should have empty race/class; got race=%q class=%q", - got.Race, got.Class) - } - // Level on the stub must be the computed mapping (not the default 1) - // so the onboarding DM reports the correct projected level. - wantLevel := dndLevelFromCombatLevel(30) // 30/5 = 6 - if got.Level != wantLevel { - t.Errorf("stub level = %d, want %d (combat_level=30 → /5)", got.Level, wantLevel) - } -} - -// TestSetupStatus_BrandNewPlayerNoStub — combat_level < 2 means brand-new -// player. The !setup flow must NOT save a stub or fire onboarding. -func TestSetupStatus_BrandNewPlayerNoStub(t *testing.T) { - setupAuditTestDB(t) - uid := id.UserID("@setup_new:example") - if err := createAdvCharacter(uid, "setup_new"); err != nil { - t.Fatal(err) - } - // CombatLevel stays at default (1). - - p := &AdventurePlugin{} - if err := p.dndSetupStatus(MessageContext{Sender: uid}); err != nil { - t.Fatal(err) - } - - // No row should exist — brand-new player flow doesn't pre-create. - got, _ := LoadDnDCharacter(uid) - if got != nil { - t.Errorf("brand-new player got a stub row: %+v", got) + for _, uid := range []id.UserID{"@setup_legacy:example", "@setup_new:example"} { + if err := p.dndSetupStatus(MessageContext{Sender: uid}); err != nil { + t.Fatal(err) + } + got, _ := LoadDnDCharacter(uid) + if got != nil { + t.Errorf("%s: !setup pre-created a row: %+v", uid, got) + } } } diff --git a/internal/plugin/dnd_combat.go b/internal/plugin/dnd_combat.go index 08ace05..3c17417 100644 --- a/internal/plugin/dnd_combat.go +++ b/internal/plugin/dnd_combat.go @@ -191,17 +191,13 @@ func classStatPriority(class DnDClass) [6]int { } // ensureCharForDnDCmd is the helper non-combat D&D command handlers should -// use when they want auto-migration semantics PLUS the legacy-player -// onboarding DM. Combat paths in combat_bridge.go use ensureDnDCharacterForCombat -// directly because they already control the freshMigrate hook. +// use when they want auto-migration semantics. Combat paths in +// combat_bridge.go use ensureDnDCharacterForCombat directly. func (p *AdventurePlugin) ensureCharForDnDCmd(userID id.UserID, char *AdventureCharacter) (*DnDCharacter, error) { - c, fresh, err := ensureDnDCharacterForCombat(userID, char) + c, _, err := ensureDnDCharacterForCombat(userID, char) if err != nil { return nil, err } - if fresh { - p.maybeSendDnDOnboarding(userID, char, c) - } return c, nil } diff --git a/internal/plugin/dnd_onboarding.go b/internal/plugin/dnd_onboarding.go deleted file mode 100644 index 0c0e94c..0000000 --- a/internal/plugin/dnd_onboarding.go +++ /dev/null @@ -1,73 +0,0 @@ -package plugin - -import ( - "fmt" - "log/slog" - - "gogobee/internal/flavor" - "maunium.net/go/mautrix/id" -) - -// One-shot onboarding DM fired the first time a legacy player encounters -// the new D&D layer. Triggered from ensureDnDCharacterForCombat at the -// moment of fresh auto-migration; suppressed for genuinely new players -// (combat_level <= 1) since the message frames around "previous players". - -// dndLegacyMinLevel — players with combat_level at or above this are -// considered "legacy players" who deserve the onboarding spiel. Anyone -// at L1 is brand new and gets the standard !setup nudge instead. -const dndLegacyMinLevel = 2 - -// maybeSendDnDOnboarding sends the welcome DM iff the player has visible -// legacy adventure progress AND hasn't been onboarded before. The -// OnboardingSent flag survives draft cancellation, !respec, and any other -// state mutation — once a player has seen the welcome, they never see it -// again. Logs and continues on send failure (DM is best-effort, never blocks combat). -func (p *AdventurePlugin) maybeSendDnDOnboarding(userID id.UserID, advChar *AdventureCharacter, dnd *DnDCharacter) { - if advChar == nil || advChar.CombatLevel < dndLegacyMinLevel { - return - } - if dnd == nil || dnd.OnboardingSent { - return - } - // Skip entirely if there's no Matrix client — tests construct empty - // AdventurePlugin{} and we shouldn't write to the DB on a nil-send. - if p == nil || p.Client == nil { - return - } - msg := dndOnboardingText(advChar.CombatLevel, dnd.Level) - if err := p.SendDM(userID, msg); err != nil { - slog.Error("dnd: onboarding DM failed", "user", userID, "err", err) - // Don't mark as sent if delivery failed — they should get a chance - // on their next combat. Send failures here are typically transient. - return - } - dnd.OnboardingSent = true - if err := SaveDnDCharacter(dnd); err != nil { - slog.Error("dnd: persist onboarding flag", "user", userID, "err", err) - } -} - -func dndOnboardingText(oldLevel, newLevel int) string { - prelude := "" - if line := flavor.Pick(flavor.ExpeditionStart); line != "" { - prelude = "_" + line + "_\n\n" - } - return prelude + fmt.Sprintf(`Hi there! Welcome to the new Adventure game! - -We shamelessly cribbed.. aimed for feature parity with our competitors. -And the result is this..! and adventure game with most of the best Dungeons & Drag- - -"AHEM!" *TwinBee glances at the Pinkerton agent in the corner of the room. - -..d20 System mechanics ready for you to explore! - -As a result of these amazing and entirely necessary changes that weren't done at the whim of a bored engineer.. the level system has changed. But no worries! We spent hours coming up with an algorithm that would ensure each player arrives at a level that is fully representative of their level under the previous system (..by dividing your previous level by five). - -Your previous level **%d** is now Adv 2.0 level **%d**. - -Enjoy! - -Type !setup to get your character situated under this hot new and legally distinct system.`, - oldLevel, newLevel) -} diff --git a/internal/plugin/dnd_onboarding_test.go b/internal/plugin/dnd_onboarding_test.go deleted file mode 100644 index 734ad9c..0000000 --- a/internal/plugin/dnd_onboarding_test.go +++ /dev/null @@ -1,52 +0,0 @@ -package plugin - -import ( - "strings" - "testing" -) - -func TestDnDOnboardingText_ContainsLevelMapping(t *testing.T) { - msg := dndOnboardingText(49, 9) - if !strings.Contains(msg, "**49**") { - t.Error("onboarding text doesn't include old level") - } - if !strings.Contains(msg, "**9**") { - t.Error("onboarding text doesn't include new level") - } - // Spot-check signature lines from the user's brief. - for _, snippet := range []string{ - "Welcome to the new Adventure game", - "Pinkerton agent", - "dividing your previous level by five", - "!setup", - "legally distinct", - } { - if !strings.Contains(msg, snippet) { - t.Errorf("onboarding missing expected snippet: %q", snippet) - } - } -} - -// TestMaybeSendDnDOnboarding_LegacyThreshold: only fires for combat_level >= 2. -// The DM call itself is no-op in tests (Base.Client is nil and SendDM guards), -// so we can't assert on send — but we can assert the function doesn't panic -// or error for either branch. -func TestMaybeSendDnDOnboarding_DoesNotPanic(t *testing.T) { - p := &AdventurePlugin{} - dnd := &DnDCharacter{Level: 4} - - // Brand-new player (combat_level=1): should be a no-op. - p.maybeSendDnDOnboarding("@new:x", &AdventureCharacter{CombatLevel: 1}, dnd) - - // Legacy player (combat_level=20): should attempt to send (no-op'd by nil Client). - p.maybeSendDnDOnboarding("@legacy:x", &AdventureCharacter{CombatLevel: 20}, dnd) - - // nil advChar: should be a no-op. - p.maybeSendDnDOnboarding("@nil:x", nil, dnd) -} - -func TestDnDLegacyMinLevel(t *testing.T) { - if dndLegacyMinLevel < 1 { - t.Errorf("dndLegacyMinLevel = %d, must be ≥ 1", dndLegacyMinLevel) - } -} diff --git a/internal/plugin/dnd_setup.go b/internal/plugin/dnd_setup.go index 5d4ef6b..d514448 100644 --- a/internal/plugin/dnd_setup.go +++ b/internal/plugin/dnd_setup.go @@ -116,34 +116,6 @@ func (p *AdventurePlugin) dndSetupStatus(ctx MessageContext) error { // No row yet → fresh setup. Show suggestion + race menu. if c == nil { - // Legacy-player welcome: if they're meeting D&D for the first time - // via `!setup` (rather than via combat auto-migration), they still - // deserve the onboarding DM. We persist a stub draft row so the - // OnboardingSent flag carries forward — neither this nor any later - // auto-migration will re-send the welcome. - advChar, _ := loadAdvCharacter(ctx.Sender) - if advChar != nil && advChar.CombatLevel >= dndLegacyMinLevel { - now := time.Now().UTC() - // Seed the stub's Level with the computed mapping so the welcome DM - // reports the right number ("your level X is now Adv 2.0 level Y"). - // On !setup confirm the level is recomputed from the same formula, - // so the value here matches what the player will actually end up with. - stub := &DnDCharacter{ - UserID: ctx.Sender, - Level: dndLevelFromCombatLevel(advChar.CombatLevel), - ArmorClass: 10, - PendingSetup: true, - OnboardingSent: false, // maybeSendDnDOnboarding will flip this on success - CreatedAt: now, - UpdatedAt: now, - } - if err := SaveDnDCharacter(stub); err != nil { - slog.Error("dnd: setup stub save failed", "user", ctx.Sender, "err", err) - } else { - p.maybeSendDnDOnboarding(ctx.Sender, advChar, stub) - } - } - sug := inferDnDFromArchetypes(ctx.Sender) var b strings.Builder b.WriteString("⚔️ **Adv 2.0 Setup** — let's build your character.\n\n") diff --git a/internal/plugin/dnd_sheet.go b/internal/plugin/dnd_sheet.go index 10c0294..85430a8 100644 --- a/internal/plugin/dnd_sheet.go +++ b/internal/plugin/dnd_sheet.go @@ -164,8 +164,8 @@ func renderDnDSheet(c *DnDCharacter, adv *AdventureCharacter, meta *PlayerMeta, // Legacy adventure context — preserved progress at a glance if adv != nil { b.WriteString("\n**Adventure progress** _(preserved)_\n") - b.WriteString(fmt.Sprintf(" Mining %d Foraging %d Fishing %d Combat (legacy) %d\n", - adv.MiningSkill, adv.ForagingSkill, adv.FishingSkill, adv.CombatLevel)) + b.WriteString(fmt.Sprintf(" Mining %d Foraging %d Fishing %d\n", + adv.MiningSkill, adv.ForagingSkill, adv.FishingSkill)) wins, losses := adv.ArenaWins, adv.ArenaLosses if meta != nil { wins, losses = meta.ArenaWins, meta.ArenaLosses diff --git a/internal/plugin/dnd_zone_combat.go b/internal/plugin/dnd_zone_combat.go index 7e206dc..3d8db2f 100644 --- a/internal/plugin/dnd_zone_combat.go +++ b/internal/plugin/dnd_zone_combat.go @@ -130,13 +130,10 @@ func (p *AdventurePlugin) runZoneCombat( playerStats, playerMods := DerivePlayerStats(char, equip, bonuses, chatLvl, char.CurrentStreak, false) - dndChar, freshMigrate, err := ensureDnDCharacterForCombat(userID, char) + dndChar, _, err := ensureDnDCharacterForCombat(userID, char) if err != nil { return CombatResult{}, fmt.Errorf("ensure dnd character: %w", err) } - if freshMigrate { - p.maybeSendDnDOnboarding(userID, char, dndChar) - } applyDnDPlayerLayer(&playerStats, dndChar) applyDnDEquipmentLayer(&playerStats, dndChar, equip) applyDnDHPScaling(&playerStats, dndChar)