mirror of
https://github.com/bryanthaboi/gen1recomp.git
synced 2026-08-12 08:21:02 +02:00
Fix two latent bugs surfaced by static analysis
Both are code paths that never run in a green test today but crash or
misbehave the moment a mod or a link failure exercises them.
1. Music.lua: applyVolume built its `music.volume` hook context from the
private `state` table, but was defined *above* `local state = {...}`, so
those reads bound to the nil global `state`. Any mod registering the
music.volume hook crashed with "attempt to index a nil value (global
'state')" the first time a volume was applied. Forward-declare `state`
above applyVolume. Regression test drives a file-backed song through the
hook and asserts the context resolves.
2. Tournament.lua: `local battle, why = isHost and newHost() or newGuest()`
had two defects. The and/or idiom truncates a call to its first result,
so `why` (the specific failure reason) was always dropped and every link
failure showed the generic "Link battle can't start" instead of e.g.
"same mods on both games". Worse, when a host's newHost() returned nil,
the `or` fell through and wrongly called newGuest() as the host. Split
into an explicit if/else so the reason is preserved and each role calls
its own constructor.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q6bFAiQyZ5jDmewsbB4LG9
This commit is contained in:
+8
-1
@@ -23,6 +23,13 @@ local volumeScale = 1
|
|||||||
local FILTER_HIGHGAIN = { 0.4, 0.16, 0.064 }
|
local FILTER_HIGHGAIN = { 0.4, 0.16, 0.064 }
|
||||||
local filterLevel = 0
|
local filterLevel = 0
|
||||||
|
|
||||||
|
-- Forward-declared here so applyVolume (below) closes over the real playback
|
||||||
|
-- state rather than a nil global: the table literal is assigned further down,
|
||||||
|
-- but a `local state = {}` there would leave every reference above it bound
|
||||||
|
-- to the global `state`. Before this, registering the `music.volume` mod
|
||||||
|
-- hook crashed applyVolume on `state.current` (a nil index).
|
||||||
|
local state
|
||||||
|
|
||||||
local function applyVolume(src)
|
local function applyVolume(src)
|
||||||
if not src then return end
|
if not src then return end
|
||||||
local vol = VOLUME * volumeScale
|
local vol = VOLUME * volumeScale
|
||||||
@@ -63,7 +70,7 @@ local function applyFilter(src)
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
local state = {
|
state = {
|
||||||
current = nil, -- song label
|
current = nil, -- song label
|
||||||
chip = false, -- the playing song is a synthesized channel program
|
chip = false, -- the playing song is a synthesized channel program
|
||||||
source = nil, -- currently playing source
|
source = nil, -- currently playing source
|
||||||
|
|||||||
+10
-3
@@ -384,9 +384,16 @@ function Tournament:update(dt)
|
|||||||
else
|
else
|
||||||
self.pendingBattleOpts.seed = msg.seed
|
self.pendingBattleOpts.seed = msg.seed
|
||||||
end
|
end
|
||||||
local battle, why = self.isHost
|
-- Split rather than `cond and newHost() or newGuest()`: the and/or
|
||||||
and LinkBattle.newHost(self.game, self.net, self.pendingBattleOpts)
|
-- idiom truncates a call to its first result, so the second return
|
||||||
or LinkBattle.newGuest(self.game, self.net, self.pendingBattleOpts)
|
-- (the specific reason) was always dropped and every failure showed
|
||||||
|
-- the generic fallback instead of "same mods on both games" etc.
|
||||||
|
local battle, why
|
||||||
|
if self.isHost then
|
||||||
|
battle, why = LinkBattle.newHost(self.game, self.net, self.pendingBattleOpts)
|
||||||
|
else
|
||||||
|
battle, why = LinkBattle.newGuest(self.game, self.net, self.pendingBattleOpts)
|
||||||
|
end
|
||||||
if not battle then
|
if not battle then
|
||||||
self:exitWith(why or Strings("Link battle\ncan't start."))
|
self:exitWith(why or Strings("Link battle\ncan't start."))
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -0,0 +1,65 @@
|
|||||||
|
-- Regression: the `music.volume` mod hook must not crash on Music's private
|
||||||
|
-- `state`. applyVolume (src/core/Music.lua) builds its hook context from
|
||||||
|
-- state.current/mapSong/onBike/... but was defined above the `local state`
|
||||||
|
-- table, so those reads bound to the nil global `state` instead -- a mod that
|
||||||
|
-- registered music.volume hit "attempt to index a nil value (global 'state')"
|
||||||
|
-- the first time any volume was applied. This drives a file-backed song
|
||||||
|
-- through Music with the hook installed and asserts the context resolves.
|
||||||
|
-- ROM-free: a fake audio source, no data/generated/.
|
||||||
|
-- luajit tests/engine/music_volume_hook_state.lua
|
||||||
|
|
||||||
|
package.path = "./?.lua;./?/init.lua;" .. package.path
|
||||||
|
|
||||||
|
local T = require("tests.harness")
|
||||||
|
love = require("tests.love_stub")
|
||||||
|
|
||||||
|
-- minimal audio source: only the methods Music calls on a file song
|
||||||
|
local Source = {}
|
||||||
|
Source.__index = Source
|
||||||
|
function Source:play() self.playing = true end
|
||||||
|
function Source:stop() self.playing = false end
|
||||||
|
function Source:pause() self.playing = false end
|
||||||
|
function Source:isPlaying() return self.playing end
|
||||||
|
function Source:setLooping(v) self.looping = v end
|
||||||
|
function Source:setVolume(v) self.volume = v end
|
||||||
|
function Source:setFilter() end
|
||||||
|
love.audio = {
|
||||||
|
newSource = function(file) return setmetatable({ file = file }, Source) end,
|
||||||
|
}
|
||||||
|
|
||||||
|
local Runtime = require("src.mods.Runtime")
|
||||||
|
local Music = require("src.core.Music")
|
||||||
|
|
||||||
|
local events = require("src.mods.Events").new()
|
||||||
|
local hooks = require("src.mods.Hooks").new()
|
||||||
|
Runtime.install(events, hooks)
|
||||||
|
|
||||||
|
-- one file-backed song is enough; the chip path would pull in `bit`
|
||||||
|
local data = { audio = { songs = { TEST = { file = "test.ogg" } } } }
|
||||||
|
|
||||||
|
-- record every context the hook is handed, and scale the volume so the
|
||||||
|
-- return value is exercised too
|
||||||
|
local calls = {}
|
||||||
|
hooks:wrap("music.volume", function(_next, vol, ctx)
|
||||||
|
calls[#calls + 1] = ctx
|
||||||
|
return vol * 0.5
|
||||||
|
end, nil, "voltest")
|
||||||
|
|
||||||
|
T.check(Runtime.wantsHook("music.volume"), "the music.volume hook is registered")
|
||||||
|
|
||||||
|
-- Before the fix this call raised inside applyVolume; reaching the next line
|
||||||
|
-- at all is the core of the regression.
|
||||||
|
Music.play(data, "TEST")
|
||||||
|
T.check(#calls >= 1, "applyVolume ran the hook during play without crashing")
|
||||||
|
|
||||||
|
-- A second application, now that the song is current: state must resolve to
|
||||||
|
-- the real playback table, so the context carries the live song + scale.
|
||||||
|
local before = #calls
|
||||||
|
Music.setVolumeLevel(5)
|
||||||
|
T.check(#calls > before, "setVolumeLevel re-applies volume through the hook")
|
||||||
|
local ctx = calls[#calls]
|
||||||
|
T.check(type(ctx) == "table", "the hook receives a context table")
|
||||||
|
T.eq(ctx.song, "TEST", "ctx.song is the live song (state resolved, not nil)")
|
||||||
|
T.eq(ctx.optionScale, 5 / 7, "ctx.optionScale reflects the 0-7 volume level")
|
||||||
|
|
||||||
|
T.finish("music_volume_hook_state")
|
||||||
Reference in New Issue
Block a user