-
Notifications
You must be signed in to change notification settings - Fork 48
又力大砖飞了一个模组,但是直接在fade模组上修改的。经过1.55+版本测试正常。 #107
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
base: main
Are you sure you want to change the base?
Conversation
Updated warning message for potential performance impact and conflicts.
Co-authored-by: 凌莞~(=^▽^=) <[email protected]>
Co-authored-by: 凌莞~(=^▽^=) <[email protected]>
Co-authored-by: 凌莞~(=^▽^=) <[email protected]>
Co-authored-by: 凌莞~(=^▽^=) <[email protected]>
Refactor SetFade class for better organization and functionality, including JSON loading and resource management.
Summary of ChangesHello @Haisairova-Official, 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! 此拉取请求通过引入KLD特殊转场功能,显著增强了现有转场动画模块。该功能允许开发者为特定歌曲配置独特的转场效果,通过外部JSON文件进行灵活管理。它采用了一种创新的“充能”机制,确保KLD转场在歌曲选择后有限次触发,为游戏体验增添了更多动态和个性化的视觉元素,尤其适用于1.55+版本。 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. 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通过为特定歌曲增加KLD转场动画,极大地增强了模组的功能,非常棒。代码重构将原有逻辑与新功能整合得很好。
不过,我发现了一些可以改进的地方,主要集中在以下几点:
- JSON解析: 目前使用正则表达式来解析JSON文件,这种方式非常脆弱且容易出错。我强烈建议改用成熟的JSON库(如
Newtonsoft.Json)来处理,这样代码会更健壮、更易于维护。 - 异常处理: 代码中多处使用了空的
catch {}块。这会静默地吞噬所有异常,导致出现问题时难以排查。建议至少在catch块中记录异常信息。 - 代码健壮性与性能: 部分代码在健壮性(如资源加载检查)和性能(如高频调用的Hook点)方面还有提升空间。
我已经针对这些问题提出了具体的修改建议,希望能帮助你进一步完善代码。
AquaMai.Mods/Fancy/SetFade.cs
Outdated
| private static void LoadJsonManual() | ||
| { | ||
| try { | ||
| string path = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, JSONDir, JSONFileName); | ||
| if (!File.Exists(path)) return; | ||
| string content = File.ReadAllText(path); | ||
| cachedEntries.Clear(); | ||
| var matches = Regex.Matches(content, @"\{[^{}]+\}"); | ||
| foreach (Match m in matches) { | ||
| string raw = m.Value; | ||
| var e = new CommonFadeEntry { | ||
| ID = ExtractInt(raw, "ID"), | ||
| isBlack = ExtractInt(raw, "isBlack"), | ||
| Type = ExtractInt(raw, "Type"), | ||
| FadeType = ExtractInt(raw, "FadeType") | ||
| }; | ||
| if (e.ID > 0) cachedEntries.Add(e); | ||
| } | ||
| MelonLogger.Msg($"[SetFade] 共载入 {cachedEntries.Count} 条 KLD 特殊配置。"); | ||
| } catch (Exception e) { MelonLogger.Error($"[SetFade] JSON加载出错: {e.Message}"); } | ||
| } |
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.
使用正则表达式来手动解析JSON文件非常脆弱且容易出错。它强依赖于JSON字符串的格式(如空格),并且无法处理嵌套对象或数组等复杂情况。当 CommonFadeList.json 文件格式稍微改变,解析就可能失败。
强烈建议使用一个标准的JSON库(例如 Newtonsoft.Json)来反序列化JSON。这样代码会更简洁、稳健且易于维护。这也使得 ExtractInt 方法变得多余,可以将其删除。
你需要将 Newtonsoft.Json.dll 添加到你的项目引用中,并在文件顶部添加 using Newtonsoft.Json;。
private static void LoadJsonManual()
{
try {
string path = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, JSONDir, JSONFileName);
if (!File.Exists(path)) return;
string content = File.ReadAllText(path);
var entries = Newtonsoft.Json.JsonConvert.DeserializeObject<List<CommonFadeEntry>>(content);
cachedEntries = entries?.FindAll(e => e.ID > 0) ?? new List<CommonFadeEntry>();
MelonLogger.Msg($"[SetFade] 共载入 {cachedEntries.Count} 条 KLD 特殊配置。");
} catch (Exception e) {
MelonLogger.Error($"[SetFade] JSON加载或解析出错: {e}");
}
}
AquaMai.Mods/Fancy/SetFade.cs
Outdated
| _kldRemainingCharges = 0; | ||
| } | ||
| } | ||
| } catch { } |
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.
AquaMai.Mods/Fancy/SetFade.cs
Outdated
| try { | ||
| var subBG = monitor.transform.Find("Canvas/Sub/Sub_ChangeScreen(Clone)/Sub_BG")?.GetComponent<Image>(); | ||
| if (subBG != null) subBG.sprite = subBGs[FadeType]; | ||
| } catch { } |
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.
AquaMai.Mods/Fancy/SetFade.cs
Outdated
| sCtrl.PlayAnimation(KaleidxScopeSubFadeController.AnimState.In); | ||
| } | ||
| } | ||
| } catch { } |
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.
| subBGs[0] = Resources.Load<Sprite>("Process/ChangeScreen/Sprites/Sub_01"); | ||
| subBGs[1] = Resources.Load<Sprite>("Process/ChangeScreen/Sprites/Sub_02"); | ||
| subBGs[2] = (GameInfo.GameVersion >= 26000) ? Resources.Load<Sprite>("Process/ChangeScreen/Sprites/Sub_03") : subBGs[0]; |
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.
新的 Prepare 方法直接加载了 SubBG 精灵,但没有像旧的 SetFade_Initialize 方法那样检查加载结果是否为 null。如果 Resources.Load 由于某些原因(例如资源路径错误或资源损坏)失败并返回 null,那么在后续调用 ReplaceSubBG 时可能会导致 NullReferenceException。
建议恢复对加载资源的空值检查,并在加载失败时记录错误日志,以提高模组的健壮性。
subBGs[0] = Resources.Load<Sprite>("Process/ChangeScreen/Sprites/Sub_01");
subBGs[1] = Resources.Load<Sprite>("Process/ChangeScreen/Sprites/Sub_02");
subBGs[2] = (GameInfo.GameVersion >= 26000) ? Resources.Load<Sprite>("Process/ChangeScreen/Sprites/Sub_03") : subBGs[0];
if (subBGs[0] == null || subBGs[1] == null || (GameInfo.GameVersion >= 26000 && subBGs[2] == null))
{
MelonLogger.Error("[SetFade] Failed to load one or more SubBG sprites. The mod may not function correctly.");
}
AquaMai.Mods/Fancy/SetFade.cs
Outdated
| if (_activeKldConfig != matched) | ||
| { | ||
| _activeKldConfig = matched; | ||
| _kldRemainingCharges = 3; // 锁定 3 次机会 |
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.
|
正则是不能改的。一改全都要出问题。我本来是正儿八经的json后来改成现在这样 |
AquaMai.Mods/Fancy/SetFade.cs
Outdated
| name: "转场类型", | ||
| en: "Type: Non-Plus 0, Plus 1. (If SDEZ 1.60 can choose Festa 2)", | ||
| zh: "类型: Non-Plus 0, Plus 1. (SDEZ 1.60 限定可选 Festa 2)")] | ||
| [ConfigEntry(name: "转场类型", zh: "0:Normal, 1:Plus, 2:Festa(仅限1.60+)")] |
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.
其实我在想写的时候预留过一个5)
AquaMai.Mods/Fancy/SetFade.cs
Outdated
| } | ||
|
|
||
| private static int ExtractInt(string text, string key) { | ||
| var m = Regex.Match(text, $"\"{key}\"\\s*:\\s*\"?(\\d+)\"?"); |
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.
真力大砖飞啊
有一个 TinyJson,要合并到主线里肯定是不可以手写 JSON parser 的
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.
想办法解析到 dictionary 吧
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.
真力大砖飞啊 有一个 TinyJson,要合并到主线里肯定是不可以手写 JSON parser 的
那我这个又想要点自定义的配置)) 写死也可以就是有点太长了吧
想办法解析到 dictionary 吧
没搞过 我理解一下
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.
好吧,那我试试给你改一下
其实可以有一个默认的 json,没有的时候自动启用,内置在里面,内容就是门对应的歌
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.
好吧,那我试试给你改一下 其实可以有一个默认的 json,没有的时候自动启用,内置在里面,内容就是门对应的歌
我其实也有这个想法)
Co-authored-by: 凌莞~(=^▽^=) <[email protected]>
Co-authored-by: 凌莞~(=^▽^=) <[email protected]>
|
宝宝我修好了你看看 |
|
又加了新模组) |
No description provided.