-
Notifications
You must be signed in to change notification settings - Fork 52
fix: PracticeMode在1.55以上,部分特性不工作。 #114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
具体而言: 1. 1.55以上,MovieController._moviePlayers的定义发生改变,从对象变为List。 2. 先前的gameCtrl只是一个对象,但实际上对于1、2P会分别有两个GameCtrl对象。 3. 給SBGA擦屁股,GameCtrl.ResetOptionSpeed里都不对是否开启2P做一下校验的,1P运行时直接一个NPE抛出来了。
Summary of ChangesHello @Starrah, 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! 此拉取请求旨在解决AquaMai练习模式在游戏版本1.55及以上运行时遇到的兼容性问题。通过调整对游戏内部API(如 Highlights
Changelog
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
本次 PR 修复了练习模式(PracticeMode)在 1.55 以上版本中因游戏内部数据结构变更而导致的部分功能失效问题。主要改动包括:正确处理了 MovieController._moviePlayers 从对象到列表的类型变化,并为 GameCtrl 增加了对双玩家的支持和空引用的安全处理。
代码改动方向正确,有效解决了描述中的问题。我提出了一些建议以进一步提高代码的健壮性和可读性:
- 恢复一个被误删的
using声明,以避免在DEBUG模式下编译失败。 - 通过条件检查替代
try-catch块,使代码逻辑更清晰,避免使用异常来控制流程。 - 增加一个空值检查,以防止在
movie列表未初始化时发生崩溃。
整体而言,这是一次不错的修复,解决了兼容性问题。
| foreach (var g in gameCtrl) | ||
| { | ||
| try | ||
| { | ||
| g?.ResetOptionSpeed(); | ||
| } | ||
| catch (NullReferenceException) {} // 忽略即可,因为SBGA的代码在ResetOptionSpeed内部没有做null检查,单刷的时候对2P那侧这个函数必定会抛NPE | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我懒了,反正我的注释清楚且明确,而且这个函数的调用频率很低(只在SetSpeed的一瞬间调用,而不是定期调用)。所以我不想改了。
|
这就属于那种,写代码很快,但知道怎么写很慢的,事情。花了几个小时才全部查清楚。 |
此前在#113 (comment) 里有部分讨论
问题分析
AquaMai/AquaMai.Mods/UX/PracticeMode/PracticeMode.cs
Lines 70 to 78 in 8c69d1b
函数里面除去注释掉的,共有四行,其中前两行是正常执行没问题的;
第三行会抛出一个NPE,这里的情况是,
movie变量的来源是通过钩子捕获MovieController._moviePlayers,而它在1.55以上,类型被修改为了List<MovieMaterialMai2>。所以原来的代码movie.player.SetSpeed(speed)就是试图在一个List上访问player成员,thanks to C#,这居然不是一个更严重的异常而仅仅只是返回null;再对null调用SetSpeed(speed)就是NPE的来源。除此之外,如果你修好了这里,紧接着你发现第四行
gameCtrl?.ResetOptionSpeed()马上又是一个NPE。这是因为,AquaMai/AquaMai.Mods/UX/PracticeMode/PracticeMode.cs
Lines 194 to 199 in 8c69d1b
GameCtrl对象(其中的monitorIndex分别为0或1)。而我们亲爱的SBGA做了什么呢?他在ResetOptionSpeed()的实现里面,居然完全不会检查当前Monitor有没有人在玩游戏,而是直接调了某个“返回当前P位的玩家的配置”的函数,该函数返回了null,于是令人惊讶但是理所当然的,在ResetOptionSpeed()内部NPE了。啊,这就是我们亲爱的SBGA啊!进一步的问题:
SetSpeed的情况,就说AquaMai里的逻辑,SetSpeed是通过SpeedUp、SpeedDown等函数调用的,而它们的调用点呢?在:AquaMai/AquaMai.Mods/UX/PracticeMode/Libs/PractiseModeUI.cs
Lines 119 to 130 in 8c69d1b
public class PracticeModeUI : MonoBehaviour的Update()函数里面。也就是说,它是Unity负责调用的而不是Melonloader的钩子调用的,所以出了异常自然也不会被MelonLoader捕获,而是被Unity捕获打到Unity的log里去了。我修了什么(抄自commit message):
具体而言: