[+] 高仿旧框下一曲目随机图片提示功能#111
Conversation
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
|
fuck github. |
|
come the fuck on wtf github? |
|
你好 @WUGqnwvMQPzl,看起来你可能想使用 Gemini Code Assist 的某个命令。请注意, 你可以尝试以下命令:
如果你想让我审查代码,请使用 |
|
你好 @WUGqnwvMQPzl,你标记了我,但没有提供具体的命令。如果你想让我执行某个操作,请使用 你可以尝试以下命令:
如果你想让我审查代码,请使用 |
|
/gemini review |
There was a problem hiding this comment.
Code Review
这个 PR 增加了一个新功能,在下一首曲目开始前显示一张随机提示图片,以模仿旧版游戏的体验。实现方式是使用 Harmony 来 patch 游戏逻辑,这对于这类 mod 来说是合适的。代码整体结构良好,但在正确性、性能和可维护性方面有几个可以改进的地方。
发现的主要问题包括:
- 随机数生成中存在一个 bug,导致其中一个语音提示永远不会被播放。
- 存在性能问题,游戏资源在频繁调用的方法中被重复加载,而不是被缓存起来。
- 针对两个不同游戏进程的 patch 之间存在大量重复代码。
- 一些硬编码的数值和数组大小使得代码不够健壮,也更难维护。
- 代码中存在不专业的注释,应该用更具说明性的注释代替。
我已经为以上每一点提供了具体的建议。解决这些问题将使代码更正确、性能更好、更易于维护。
| { | ||
| if (CheckNextTrackProcess(____mode) && !_timeCounterChanged) | ||
| { | ||
| ____timeCounter = 5f; |
| [HarmonyPatch(typeof(KaleidxScopeFadeProcess), "OnStart")] | ||
| public static void KS_OnStart_Postfix(ProcessBase ___toProcess, List<KaleidxScopeFadeController> ___mainControllerList) | ||
| { | ||
| if (___toProcess.GetType() != typeof(MusicSelectProcess) || GameManager.MusicTrackNumber < 2) // WTF SBGA??? |
|
bugbot run |
|
Skipping Bugbot: Unable to authenticate your request. Please make sure Bugbot is properly installed and configured for this repository. |
Co-authored-by: 凌莞~(=^▽^=) <opensource@c5y.moe>
There was a problem hiding this comment.
2 issues found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="AquaMai.Mods/Fancy/NextTrackTips.cs">
<violation number="1" location="AquaMai.Mods/Fancy/NextTrackTips.cs:197">
P2: The window-creation, sound-effect, and voice-line logic is duplicated almost verbatim between `OnStart_Postfix` and `KS_OnStart_Postfix`. This duplication has already caused a subtle divergence (the off-by-one in the final-track voice check). Consider extracting the shared per-player setup into a helper method.</violation>
<violation number="2" location="AquaMai.Mods/Fancy/NextTrackTips.cs:218">
P2: Inconsistent last-track voice condition: `OnStart_Postfix` uses `MusicTrackNumber + 1U == GetMaxTrackCount()` while this KaleidxScopeFade counterpart uses `MusicTrackNumber == GetMaxTrackCount()`. One of these conditions is off-by-one, causing the final-track voice cue to play at the wrong time in one of the two code paths.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| [EnableGameVersion(25000, noWarn: true)] | ||
| [HarmonyPostfix] | ||
| [HarmonyPatch(typeof(KaleidxScopeFadeProcess), "OnStart")] | ||
| public static void KS_OnStart_Postfix(ProcessBase ___toProcess, List<KaleidxScopeFadeController> ___mainControllerList) |
There was a problem hiding this comment.
P2: The window-creation, sound-effect, and voice-line logic is duplicated almost verbatim between OnStart_Postfix and KS_OnStart_Postfix. This duplication has already caused a subtle divergence (the off-by-one in the final-track voice check). Consider extracting the shared per-player setup into a helper method.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At AquaMai.Mods/Fancy/NextTrackTips.cs, line 197:
<comment>The window-creation, sound-effect, and voice-line logic is duplicated almost verbatim between `OnStart_Postfix` and `KS_OnStart_Postfix`. This duplication has already caused a subtle divergence (the off-by-one in the final-track voice check). Consider extracting the shared per-player setup into a helper method.</comment>
<file context>
@@ -0,0 +1,257 @@
+ [EnableGameVersion(25000, noWarn: true)]
+ [HarmonyPostfix]
+ [HarmonyPatch(typeof(KaleidxScopeFadeProcess), "OnStart")]
+ public static void KS_OnStart_Postfix(ProcessBase ___toProcess, List<KaleidxScopeFadeController> ___mainControllerList)
+ {
+ if (___toProcess.GetType() != typeof(MusicSelectProcess) || GameManager.MusicTrackNumber < 2) // WTF SBGA???
</file context>
|
|
||
| Mai2.Voice_Partner_000001.Cue nextTrackVoice = UnityEngine.Random.Range(0, 2) == 0 ? Mai2.Voice_Partner_000001.Cue.VO_000151 : Mai2.Voice_Partner_000001.Cue.VO_000152; | ||
| // WTF SBGA?? | ||
| if (GameManager.MusicTrackNumber == GameManager.GetMaxTrackCount()) |
There was a problem hiding this comment.
P2: Inconsistent last-track voice condition: OnStart_Postfix uses MusicTrackNumber + 1U == GetMaxTrackCount() while this KaleidxScopeFade counterpart uses MusicTrackNumber == GetMaxTrackCount(). One of these conditions is off-by-one, causing the final-track voice cue to play at the wrong time in one of the two code paths.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At AquaMai.Mods/Fancy/NextTrackTips.cs, line 218:
<comment>Inconsistent last-track voice condition: `OnStart_Postfix` uses `MusicTrackNumber + 1U == GetMaxTrackCount()` while this KaleidxScopeFade counterpart uses `MusicTrackNumber == GetMaxTrackCount()`. One of these conditions is off-by-one, causing the final-track voice cue to play at the wrong time in one of the two code paths.</comment>
<file context>
@@ -0,0 +1,257 @@
+
+ Mai2.Voice_Partner_000001.Cue nextTrackVoice = UnityEngine.Random.Range(0, 2) == 0 ? Mai2.Voice_Partner_000001.Cue.VO_000151 : Mai2.Voice_Partner_000001.Cue.VO_000152;
+ // WTF SBGA??
+ if (GameManager.MusicTrackNumber == GameManager.GetMaxTrackCount())
+ nextTrackVoice = Mai2.Voice_Partner_000001.Cue.VO_000153;
+ SoundManager.PlayPartnerVoice(nextTrackVoice, i);
</file context>
|
|
||
| #region KaleidxScopeFadeProcess Patch | ||
| [EnableGameVersion(25000, noWarn: true)] | ||
| [EnableGameVersion(25000, 26499, noWarn: true)] |
There was a problem hiding this comment.
据我所知,这里的内容大概会在这个类加载的时候被 jit 解析,让我验证一下这么写到底会不会有问题
No description provided.