Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求引入了一个全新的背景音乐（BGM）管理器，旨在显著提升音频播放功能。它利用双HTMLAudioElement实例实现音轨间的无缝交叉渐入渐出，并集成了GSAP库以实现精确、动画化的音量控制。这次重构取代了之前手动的BGM管理方式，为背景音乐播放提供了更强大和灵活的控制，包括播放、暂停、停止、淡入淡出和恢复功能，并支持可配置的淡入淡出时长。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors background music management by introducing a new BgmManager class that utilizes gsap for smooth audio transitions, replacing previous direct DOM manipulation and custom fade logic. This change is integrated across several components, including the playBgm function, playVideo script, AudioContainer, and ExtraBgm UI. The review highlights several areas for improvement: the play method in BgmManager should prevent unnecessary audio restarts when the source is unchanged, a useEffect hook in AudioContainer needs its dependency array updated to include bgmVol and bgmEnter, type safety can be improved by specifying Event for onError parameters, and _onTimeUpdate should use _audio.currentSrc for more reliable source checking.
| const fade = options.fade ?? 0; | ||
| if (options.volume !== undefined) this._targetVolume = options.volume; | ||
| if (options.loop !== undefined) this.loop = options.loop; |
There was a problem hiding this comment.
当前 play 方法在被调用时，即使传入的 src 与当前播放的音轨相同，也会重新加载并播放音频，这会导致音乐从头开始。在某些场景下（例如，仅音量变化时），这可能不是预期的行为。
建议在 play 方法的开头增加一个判断：如果请求的 src 与当前正在播放的音轨相同且并未暂停，那么应该只调整音量，而不是重启整个音轨。这能让 play 方法在重复调用时表现更符合预期，尤其是在 React useEffect 中使用时。
const fade = options.fade ?? 0;
if (options.volume !== undefined) this._targetVolume = options.volume;
if (options.loop !== undefined) this.loop = options.loop;
// If src is provided and is the same as the current one, just adjust volume
if (options.src && options.src === this._audio.src && !this._audio.paused) {
await this._setVolume({ index: this._currentIndex, volume: this._targetVolume, fade });
return;
}| bgmManager.play({ src: stageStore.bgm.src, volume: bgmVol, fade: bgmEnter }); | ||
| } | ||
| }, [isShowTitle, titleBgm, stageStore.bgm.src, bgmVol, bgmEnter]); | ||
| }, [isEnterGame, isShowTitle, titleBgm, stageStore.bgm.src]); |
There was a problem hiding this comment.
这个 useEffect 的依赖项数组不完整。它在内部使用了 bgmVol 和 bgmEnter 变量，但没有将它们声明为依赖项。这会导致当音量或淡入淡出时间变化时，组件不会重新运行效果，从而产生 bug。
请将它们添加到依赖项数组中，以确保 BGM 的行为与应用状态保持同步。我在另一条评论中建议修改 bgmManager 以防止音乐在音量变化时重启，应用该建议后，此处的修改才是安全的。
| }, [isEnterGame, isShowTitle, titleBgm, stageStore.bgm.src]); | |
| }, [isEnterGame, isShowTitle, titleBgm, stageStore.bgm.src, bgmVol, bgmEnter]); |
| nextAudio.removeEventListener('error', onError); | ||
| resolve(null); | ||
| }; | ||
| const onError = (e: any) => { |
There was a problem hiding this comment.
为了增强代码的类型安全性和可读性，建议为 onError 回调函数中的事件参数 e 指定更明确的类型 Event，而不是使用 any。
| const onError = (e: any) => { | |
| const onError = (e: Event) => { |
| } | ||
|
|
||
| private _onTimeUpdate = () => { | ||
| if (!this._audio.src || this._progressListeners.size === 0) return; |
There was a problem hiding this comment.
在 _onTimeUpdate 方法中，使用 !this._audio.src 来判断是否加载了音频源可能不够可靠。当通过 removeAttribute('src') 清除音源后，src 属性可能会指向当前页面的 URL。建议使用 this._audio.currentSrc 进行判断，当没有媒体加载时，它会是一个空字符串，这样更健壮。
| if (!this._audio.src || this._progressListeners.size === 0) return; | |
| if (!this._audio.currentSrc || this._progressListeners.size === 0) return; |
新的 BGM 管理器使用双 Audio 以实现交叉渐入渐出， gsap 控制音量曲线。同时还添加了更多的控制方法。
目前还有一些地方仍未解决：