[+] 高仿旧框下一曲目随机图片提示功能 #111
[+] 高仿旧框下一曲目随机图片提示功能 #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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
这个 PR 增加了一个新功能，在下一首曲目开始前显示一张随机提示图片，以模仿旧版游戏的体验。实现方式是使用 Harmony 来 patch 游戏逻辑，这对于这类 mod 来说是合适的。代码整体结构良好，但在正确性、性能和可维护性方面有几个可以改进的地方。
发现的主要问题包括：
- 随机数生成中存在一个 bug，导致其中一个语音提示永远不会被播放。
- 存在性能问题，游戏资源在频繁调用的方法中被重复加载，而不是被缓存起来。
- 针对两个不同游戏进程的 patch 之间存在大量重复代码。
- 一些硬编码的数值和数组大小使得代码不够健壮，也更难维护。
- 代码中存在不专业的注释，应该用更具说明性的注释代替。
我已经为以上每一点提供了具体的建议。解决这些问题将使代码更正确、性能更好、更易于维护。
| if (!CheckNextTrackProcess(____mode)) | ||
| return; | ||
|
|
||
| var commonWindowPref = Resources.Load<GameObject>("Process/Generic/GenericProcess").transform.Find("Canvas/Main/MessageRoot/HorizontalSplitWindow").gameObject.GetComponent<CommonWindow>(); |
There was a problem hiding this comment.
Resources.Load 可能是一个缓慢的操作。在 OnStart 这种可能会被频繁执行（例如，在每首曲目之间）的方法中调用它会影响性能。更好的做法是在初始化时加载一次并缓存这个预制件（prefab）。此问题在第 202 行同样存在。
建议：
- 添加一个静态字段：
private static CommonWindow _commonWindowPrefab; - 在
Initialize()方法中加载预制件，并添加错误处理：var genericProcessPrefab = Resources.Load<GameObject>("Process/Generic/GenericProcess"); if (genericProcessPrefab == null) { MelonLogger.Error("[NextTrackTips] Failed to load GenericProcess prefab."); return false; } _commonWindowPrefab = genericProcessPrefab.transform.Find("Canvas/Main/MessageRoot/HorizontalSplitWindow").GetComponent<CommonWindow>(); if (_commonWindowPrefab == null) { MelonLogger.Error("[NextTrackTips] Failed to find CommonWindow component in prefab."); return false; }
- 在这里和
KS_OnStart_Postfix中使用缓存的_commonWindowPrefab。
| // Play the sound effects and voice line | ||
| SoundManager.PlaySE(Mai2.Mai2Cue.Cue.JINGLE_NEXT_TRACK, i); | ||
|
|
||
| Mai2.Voice_Partner_000001.Cue nextTrackVoice = UnityEngine.Random.Range(0, 1) == 0 ? Mai2.Voice_Partner_000001.Cue.VO_000151 : Mai2.Voice_Partner_000001.Cue.VO_000152; |
There was a problem hiding this comment.
UnityEngine.Random.Range(int, int) 的第二个参数是排他的（exclusive）。这意味着 UnityEngine.Random.Range(0, 1) 将永远只返回 0。因此，VO_000152 这个语音提示将永远不会被播放。
为了实现 50/50 的概率，你应该使用 UnityEngine.Random.Range(0, 2)。
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;| // Play the sound effects and voice line | ||
| SoundManager.PlaySE(Mai2.Mai2Cue.Cue.JINGLE_NEXT_TRACK, i); | ||
|
|
||
| Mai2.Voice_Partner_000001.Cue nextTrackVoice = UnityEngine.Random.Range(0, 1) == 0 ? Mai2.Voice_Partner_000001.Cue.VO_000151 : Mai2.Voice_Partner_000001.Cue.VO_000152; |
There was a problem hiding this comment.
UnityEngine.Random.Range(int, int) 的第二个参数是排他的（exclusive）。这意味着 UnityEngine.Random.Range(0, 1) 将永远只返回 0。因此，VO_000152 这个语音提示将永远不会被播放。
为了实现 50/50 的概率，你应该使用 UnityEngine.Random.Range(0, 2)。
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;| private static readonly CommonWindow[] _hackyWindows = new CommonWindow[2]; | ||
| private static IMessageMonitor[] _genericMonitorRefs = new IMessageMonitor[2]; |
There was a problem hiding this comment.
数组 _hackyWindows 和 _genericMonitorRefs 被初始化为硬编码的大小 2。虽然游戏通常可能在 2 个监视器上运行，但依赖这一点会使代码变得脆弱。如果监视器的数量发生变化，可能会导致 IndexOutOfRangeException，因为循环是基于 ____monitors.Length 进行迭代的。
更健壮的做法是使用 List<T>，或者在首次需要时（例如在 OnStart 方法中）动态确定数组大小。
例如，对于 _hackyWindows，可以在 OnStart_Postfix 中这样做：
_hackyWindows = new CommonWindow[____monitors.Length];
然后继续执行循环。
对于 _genericMonitorRefs，它在 GenericProcess_OnStart_Postfix 中被重新赋值，但在 GenericProcess_OnRelease_Postfix 中被重置为一个固定大小的数组。更好的做法是在释放时将其设置为 Array.Empty<IMessageMonitor>() 或 null。
| ); | ||
|
|
||
| // Some hacks to force the layout and "fix" spacing | ||
| var winLayout = window.transform.Find("IMG_Window").gameObject.GetComponent<HorizontalLayoutGroup>(); |
There was a problem hiding this comment.
这里的 .gameObject 访问是多余的。你可以直接在 Transform 或任何其他 Component 实例上调用 GetComponent<T>()，它会在该组件附加的 GameObject 上进行搜索。此问题也存在于第 139 和 202 行。
这可以提高代码的简洁性，是 Unity 开发中的常见做法。
var winLayout = window.transform.Find("IMG_Window").GetComponent<HorizontalLayoutGroup>();| { | ||
| if (CheckNextTrackProcess(____mode) && !_timeCounterChanged) | ||
| { | ||
| ____timeCounter = 5f; |
There was a problem hiding this comment.
数值 5f 是一个“魔术数字”。它代表提示图片显示的时长。最好在类的顶部将其定义为一个命名的常量，以提高可读性并使其更易于修改。
建议在类顶部添加一个常量：
private const float TipDisplayDuration = 5f;
然后在这里使用它。
____timeCounter = TipDisplayDuration;| } | ||
| #endregion | ||
|
|
||
| #region KaleidxScopeFadeProcess Patch |
There was a problem hiding this comment.
用于 patch NextTrackProcess (lines 115-191) 和 KaleidxScopeFadeProcess (lines 193-257) 的逻辑非常相似，导致了大量代码重复。OnStart, OnLateUpdate, StartFadeIn, 和 OnRelease 这些 patch 方法的结构几乎完全相同。
可以将其重构为共享的辅助方法以提高可维护性。例如，你可以创建如下的辅助方法：
ShowTipsWindow(int monitorIndex, Transform parent)UpdateTipsWindows()CloseTipsWindows()ReleaseTipsWindows()
然后 patch 方法只需调用这些辅助方法。主要的区别在于监视器/控制器列表的来源，可以将其作为参数传递。
| [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??? |
There was a problem hiding this comment.
像 WTF SBGA??? 这样的注释是不专业的，对未来的维护者没有帮助。最好解释一下为什么这个检查是必要的。例如，描述这段代码是为了解决哪个特定的游戏行为或问题。这个问题在第 204 和 217 行也存在。
|
bugbot run |
|
Skipping Bugbot: Unable to authenticate your request. Please make sure Bugbot is properly installed and configured for this repository. |
1 participant
No description provided.