Honor trainer battleTheme override (fixes #945)

trainers.battleTheme validated and merged onto the trainer record but was
never read: battle music came solely from data.audio.battle[kind] where
kind is computeMusicKind()'s final/gym/trainer/wild.  Route both battle-
theme start sites through a single choke point:

- BattleState:playBattleTheme() cues Music.playBattle with the override
  (self.trainer.battleTheme via battleTheme()), defaulting to the kind
  when unset, so vanilla fights and #782's non-gym Giovanni are unchanged.
- BattleState:enter() and OverworldController:pushBattle() both call it.
- Music.playBattle gains an optional 4th song arg that overrides the kind
  default, and real call sites now populate the music.select trainerId.
- Victory jingles stay kind-based: a custom battle theme has no derivable
  win-variant.

New ROM-free T2 suite tests/engine/trainer_battle_theme_bug945.lua covers
mod load, override resolution, the choke point, and the nil-override
parity gate.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Shane McGovern
2026-08-07 10:53:42 +01:00
parent 112120e8fe
commit ed8a89c5ce
5 changed files with 220 additions and 7 deletions
+21 -2
View File
@@ -1411,6 +1411,26 @@ function BattleState:computeMusicKind()
return "wild"
end
-- a mod-set per-trainer battle theme (trainers.battleTheme, an audio.songs
-- id); nil for vanilla trainers, so the kind default is untouched (#782)
function BattleState:battleTheme()
local trainer = self.trainer
if trainer and trainer.battleTheme then return trainer.battleTheme end
return nil
end
-- the battle-theme cue for this battle: the mod-set trainer battleTheme
-- when the class has one, else the kind default. The single choke point
-- both the transition-wipe start (OverworldController:pushBattle) and
-- enter() route through, so a per-trainer override can't drift between
-- them. self.musicKind is set by enter(); pushBattle runs before that,
-- so compute it here when absent.
function BattleState:playBattleTheme()
require("src.core.Music").playBattle(self.data,
self.musicKind or self:computeMusicKind(),
self.trainer and self.trainer.id, self:battleTheme())
end
-- side tables mirror the singles battlers; called before every
-- battler-switch notification so sides[i].battlers[1] stays honest
function BattleState:syncSides()
@@ -1462,7 +1482,6 @@ function BattleState:enter()
.. Strings("%s blacked\nout!", name), blackedOut))
return
end
local Music = require("src.core.Music")
self.musicKind = self:computeMusicKind()
if self.isGymLeader then
require("src.world.PikachuFollower")
@@ -1472,7 +1491,7 @@ function BattleState:enter()
-- (audio/play_battle_music.asm runs before the transition, and
-- Music.play no-ops on the same song); this covers battles pushed
-- without a transition (link battles, scripted pushes)
Music.playBattle(self.data, self.musicKind)
self:playBattleTheme()
-- intro presentation (SlidePlayerAndEnemySilhouettesOnScreen): both
-- sides slide in; the trainer pics stay up until the send-outs
-- BATTLE BG "world" drops this battle's opacity so StateStack keeps drawing
+4 -3
View File
@@ -368,11 +368,12 @@ function Music.setSurfing(data, surfing)
if play then Music.play(data, play, nil, { reason = "map" }) end
end
-- battle themes; kind = "wild"|"trainer"|"gym"|"final"
function Music.playBattle(data, kind, trainerId)
-- battle themes; kind = "wild"|"trainer"|"gym"|"final". `song`, when
-- given, overrides the kind's default -- a mod-set trainer battleTheme.
function Music.playBattle(data, kind, trainerId, song)
local b = data.audio and data.audio.battle
if b then
Music.play(data, b[kind] or b.wild, nil,
Music.play(data, song or b[kind] or b.wild, nil,
{ reason = "battle", kind = kind, trainerId = trainerId })
end
end
+3
View File
@@ -577,6 +577,9 @@ R.trainers = {
aiMods = f.opt(f.any),
aiClass = f.opt(f.id("ai_classes")),
brain = f.opt(f.fn),
-- Per-trainer battle theme (an audio.songs id): overrides the
-- kind-based default (wild/trainer/gym/final) for this trainer's
-- battles. The victory jingle stays kind-based.
battleTheme = f.opt(f.id("music")),
},
example = 'mod.content.trainers:patch("OPP_BROCK", { baseMoney = 99 })',
+2 -2
View File
@@ -760,8 +760,8 @@ function OverworldState:pushBattle(battle)
local enemyLevel = battle.enemy and battle.enemy.mon and battle.enemy.mon.level or 0
-- the battle theme starts with the wipe, not after it
-- (audio/play_battle_music.asm runs before the transition)
if battle.computeMusicKind then
require("src.core.Music").playBattle(Game.data, battle:computeMusicKind())
if battle.playBattleTheme then
battle:playBattleTheme()
end
-- The fade back in from white on the way out is BattleState:finish()'s
@@ -0,0 +1,190 @@
-- Issue #945: a mod's per-trainer battleTheme (trainers.battleTheme, an
-- audio.songs id) was validated and merged onto the trainer record but
-- never read -- battle music came solely from data.audio.battle[kind] where
-- kind is computeMusicKind()'s final/gym/trainer/wild. Both battle-theme
-- start sites (OverworldController:pushBattle's pre-wipe cue and
-- BattleState:enter) now route through BattleState:playBattleTheme(), which
-- hands the override to Music.playBattle's new song arg. A nil override
-- keeps the kind default, so vanilla trainer fights -- and #782's non-gym
-- Giovanni -- are unchanged.
package.path = "./?.lua;./?/init.lua;" .. package.path
local T = require("tests.modkit")
-- ------- love audio stub: file-backed songs only (mod_audio pattern)
local love = _G.love or {}
_G.love = love
love.audio = love.audio or {}
local assets = {
["assets/theme.ogg"] = true,
["assets/alt.ogg"] = true,
}
local sources = {}
local Source = {}
Source.__index = Source
function Source:play() end
function Source:stop() end
function Source:setLooping() end
function Source:setVolume() end
function Source:setFilter() end
love.audio.newSource = function(what, mode)
if type(what) == "string" and not assets[what] then
error("could not open file " .. what, 0)
end
local src = setmetatable({ file = what, mode = mode, queueable = false }, Source)
sources[#sources + 1] = src
return src
end
love.audio.newQueueableSource = function()
local src = setmetatable({ queueable = true }, Source)
sources[#sources + 1] = src
return src
end
local Music = require("src.core.Music")
local Runtime = require("src.mods.Runtime")
local Font = require("src.render.Font")
local TypeChart = require("src.battle.TypeChart")
local Pokemon = require("src.pokemon.Pokemon")
local SaveData = require("src.core.SaveData")
local BattleState = require("src.battle.BattleState")
-- ------- the fix-945 mod: register a song and point a trainer class at it
local MOD = {
["mods/fix_youngster_theme/manifest.json"] = [[{
"id": "fix_youngster_theme",
"name": "Fix Youngster Theme",
"version": "1.0.0",
"entry": "main.lua",
"api": 2
}]],
["mods/fix_youngster_theme/main.lua"] = [[
local mod = ...
mod.content.music:register("Music_ModTheme", { file = "assets/theme.ogg" })
mod.content.trainers:patch("OPP_FIX_YOUNGSTER", {
battleTheme = "Music_ModTheme",
})
]],
}
local function newGame(data)
local save = SaveData.newGame()
save.player.name = "RED"
save.player.rival = "GARY"
save.party = { Pokemon.new(data, "FIXMON_A", 30) }
return { data = data, save = save,
stack = { top = function() return nil end,
push = function() end, pop = function() end } }
end
-- record every cue the music.select hook sees
local function hookRecorder(seen)
Runtime.hooks:wrap("music.select", function(nextLink, song, ctx)
seen[#seen + 1] = { song = song, kind = ctx.kind,
trainerId = ctx.trainerId }
return nextLink(song, ctx)
end, nil, "bug945")
end
-- a playBattle spy that records (kind, trainerId, song) without touching audio
local function spyPlayBattle()
local calls = {}
local real = Music.playBattle
Music.playBattle = function(data, kind, trainerId, song)
calls[#calls + 1] = { kind = kind, trainerId = trainerId, song = song }
end
return calls, function() Music.playBattle = real end
end
-- ------- the modded class resolves and the override reaches the cue
local Data = T.fixtures.fresh()
Font.load(Data)
TypeChart.load(Data)
local run = T.sdk.loadMods({ "mods/fix_youngster_theme" },
{ data = Data, fs = T.sdk.memfs(MOD) })
T.eq(#run.errors, 0, "the battleTheme mod loads without validation errors")
T.eq(Data.trainers.OPP_FIX_YOUNGSTER.battleTheme, "Music_ModTheme",
"the patch lands on the trainer record")
-- give the kind defaults a home so the no-override fallback is observable
Data.audio = Data.audio or {}
Data.audio.battle = Data.audio.battle or {
wild = "Music_DefaultWild", trainer = "Music_DefaultTrainer",
}
Data.audio.songs = Data.audio.songs or {}
Data.audio.songs.Music_DefaultWild = { file = "assets/alt.ogg" }
Data.audio.songs.Music_DefaultTrainer = { file = "assets/alt.ogg" }
local battle = BattleState.newTrainer(newGame(Data), "OPP_FIX_YOUNGSTER", 1)
T.eq(battle:battleTheme(), "Music_ModTheme",
"battleTheme() resolves the per-trainer override")
T.eq(battle:computeMusicKind(), "trainer",
"a plain trainer fight is still trainer-kind")
local calls, restore = spyPlayBattle()
battle:playBattleTheme()
T.eq(#calls, 1, "playBattleTheme cues the theme once")
T.eq(calls[1].kind, "trainer", "the cue carries the computed kind")
T.eq(calls[1].trainerId, "OPP_FIX_YOUNGSTER", "the cue carries the trainer id")
T.eq(calls[1].song, "Music_ModTheme", "the override label wins over the kind default")
-- enter() sets self.musicKind before playing; playBattleTheme honors it
battle.musicKind = "gym"
battle:playBattleTheme()
T.eq(calls[2].kind, "gym", "a pre-set musicKind (the enter path) is used as-is")
restore()
-- ------- Music.playBattle: override arg wins; nil falls back to the default
local seen = {}
hookRecorder(seen)
Music.reload()
Music.playBattle(Data, "trainer", "OPP_FIX_YOUNGSTER", "Music_ModTheme")
T.eq(seen[1].song, "Music_ModTheme", "the override arg is played")
T.eq(seen[1].kind, "trainer", "the hook sees the battle kind")
T.eq(seen[1].trainerId, "OPP_FIX_YOUNGSTER", "the hook sees the trainer id")
Music.reload()
Music.playBattle(Data, "trainer", "OPP_FIX_YOUNGSTER")
T.eq(seen[2].song, "Music_DefaultTrainer",
"no override falls back to the kind's default song")
T.eq(seen[2].trainerId, "OPP_FIX_YOUNGSTER",
"the hook still sees the trainer id on the default path")
-- ------- a vanilla class has no override, so the kind default is untouched
local DataV = T.fixtures.fresh()
Font.load(DataV)
TypeChart.load(DataV)
DataV.audio = {
battle = { wild = "Music_DefaultWild", trainer = "Music_DefaultTrainer" },
songs = {
Music_DefaultWild = { file = "assets/alt.ogg" },
Music_DefaultTrainer = { file = "assets/alt.ogg" },
},
}
local battleV = BattleState.newTrainer(newGame(DataV), "OPP_FIX_YOUNGSTER", 1)
T.eq(battleV:battleTheme(), nil, "a vanilla trainer class has no override")
local callsV, restoreV = spyPlayBattle()
battleV:playBattleTheme()
T.eq(callsV[1].kind, "trainer", "vanilla cue keeps the trainer kind")
T.eq(callsV[1].song, nil, "vanilla passes no override, so the default plays (#782)")
restoreV()
local seenV = {}
hookRecorder(seenV)
Music.reload()
Music.playBattle(DataV, "trainer", "OPP_FIX_YOUNGSTER")
T.eq(seenV[1].song, "Music_DefaultTrainer",
"vanilla battles play the kind default, not a per-trainer theme (#782)")
T.finish("trainer battle theme bug945")