refactor: 使用配置系统全面取代 ModBase.Setup 并移除部分 VB.CompilerServices#2866
Open
LuLu-ling wants to merge 6 commits into
Open
refactor: 使用配置系统全面取代 ModBase.Setup 并移除部分 VB.CompilerServices#2866LuLu-ling wants to merge 6 commits into
LuLu-ling wants to merge 6 commits into
Conversation
审阅者指南重构了设置处理逻辑,完全使用新的强类型配置系统来替代 ModBase.Setup;将大量 VB.CompilerServices 的辅助方法替换为惯用的 C# 实现;并将 UI 页面、启动器启动流程、Minecraft 实例/启动逻辑、下载以及事件系统接入新的 Config/ConfigService API,并支持按实例配置。 使用 ConfigService 的 EventType.写入设置 时序图sequenceDiagram
participant User as User
participant CustomEvent as CustomEvent
participant ModEvent as ModEvent
participant ConfigService as ConfigService
participant ConfigItem as ConfigItem
User->>CustomEvent: trigger event (type=写入设置, arg="key|value")
CustomEvent->>ModEvent: Raise(type, arg)
ModEvent->>ModEvent: parse args[0]=key, args[1]=value
ModEvent->>ConfigService: TryGetConfigItemNoType(key)
alt [config item found and not SharedEncrypt]
ConfigService-->>ModEvent: item
ModEvent->>ConfigItem: SetValueNoType(value, McInstanceSelected.PathInstance)
ModEvent-->>User: optional hint "已写入设置"
else [missing or encrypted]
ModEvent-->>User: no write performed
end
文件级变更
可能关联的 Issue
技巧与指令与 Sourcery 交互
自定义你的体验打开你的 控制面板 以:
获取帮助Original review guide in EnglishReviewer's GuideRefactors settings handling to fully use the new typed configuration system instead of ModBase.Setup, replaces many VB.CompilerServices helpers with idiomatic C#, and wires UI pages, launcher startup, Minecraft instance/launch logic, downloads, and events to the new Config/ConfigService APIs with per-instance support. Sequence diagram for EventType.写入设置 using ConfigServicesequenceDiagram
participant User as User
participant CustomEvent as CustomEvent
participant ModEvent as ModEvent
participant ConfigService as ConfigService
participant ConfigItem as ConfigItem
User->>CustomEvent: trigger event (type=写入设置, arg="key|value")
CustomEvent->>ModEvent: Raise(type, arg)
ModEvent->>ModEvent: parse args[0]=key, args[1]=value
ModEvent->>ConfigService: TryGetConfigItemNoType(key)
alt [config item found and not SharedEncrypt]
ConfigService-->>ModEvent: item
ModEvent->>ConfigItem: SetValueNoType(value, McInstanceSelected.PathInstance)
ModEvent-->>User: optional hint "已写入设置"
else [missing or encrypted]
ModEvent-->>User: no write performed
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
8b9c4d3 to
8af4c2a
Compare
There was a problem hiding this comment.
Hey,我在这里给出了一些整体性的反馈:
- 在各个
Set*ByTag辅助方法中(例如SetByTag、SetLaunchByTag、SetGameManageByTag、SetInstanceByTag),很多分支会把object value直接强制转换为int/double/bool,但调用方(滑块等)往往会传入double或可空类型,这可能会抛出InvalidCastException。建议使用Convert.ToInt32/Convert.ToDouble并结合空值安全处理,而不是直接强转。 - 新的直接配置 setter 绕过了旧的
ModBase.Setup事件/副作用机制;对于之前依赖变更回调的选项(例如 logo 类型/标题的更新、一些 UI 刷新操作),请仔细检查:确保在用户修改设置时仍然会调用等价的方法(如ModSetup.UiLogoType/UiLogoText/UiLogoLeft),而不是只在启动时调用一次。 - 在
MyHint中,加载/关闭逻辑现在使用ConfigService.TryGetConfigItemNoType(...).GetValueNoType()来决定可见性并持久化关闭状态;请确认对应的配置项在未设置时的状态是null(或使用IsDefault),以避免因为非空默认值导致提示在一开始就被隐藏。
给 AI 智能体的提示
Please address the comments from this code review:
## Overall Comments
- In the various `Set*ByTag` helpers (e.g. `SetByTag`, `SetLaunchByTag`, `SetGameManageByTag`, `SetInstanceByTag`), many cases cast `object value` directly to `int`/`double`/`bool`, but the callers (sliders, etc.) often pass a `double` or nullable type, which can throw `InvalidCastException`; consider using `Convert.ToInt32`/`Convert.ToDouble` and null-safe handling instead of direct casts.
- The new direct-config setters bypass the old `ModBase.Setup` event/side-effect mechanism; for options that previously relied on change callbacks (e.g. logo type/title updates, some UI refresh actions), double-check that equivalent calls (like `ModSetup.UiLogoType/UiLogoText/UiLogoLeft`) are still invoked on user changes rather than only at startup.
- In `MyHint`, the loaded/close logic now uses `ConfigService.TryGetConfigItemNoType(...).GetValueNoType()` to decide visibility and persist dismissal; verify that the corresponding config items have `null` as their unset state (or use `IsDefault`) so that hints are not hidden immediately due to a non-null default value.帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进之后的代码评审。
Original comment in English
Hey - I've left some high level feedback:
- In the various
Set*ByTaghelpers (e.g.SetByTag,SetLaunchByTag,SetGameManageByTag,SetInstanceByTag), many cases castobject valuedirectly toint/double/bool, but the callers (sliders, etc.) often pass adoubleor nullable type, which can throwInvalidCastException; consider usingConvert.ToInt32/Convert.ToDoubleand null-safe handling instead of direct casts. - The new direct-config setters bypass the old
ModBase.Setupevent/side-effect mechanism; for options that previously relied on change callbacks (e.g. logo type/title updates, some UI refresh actions), double-check that equivalent calls (likeModSetup.UiLogoType/UiLogoText/UiLogoLeft) are still invoked on user changes rather than only at startup. - In
MyHint, the loaded/close logic now usesConfigService.TryGetConfigItemNoType(...).GetValueNoType()to decide visibility and persist dismissal; verify that the corresponding config items havenullas their unset state (or useIsDefault) so that hints are not hidden immediately due to a non-null default value.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the various `Set*ByTag` helpers (e.g. `SetByTag`, `SetLaunchByTag`, `SetGameManageByTag`, `SetInstanceByTag`), many cases cast `object value` directly to `int`/`double`/`bool`, but the callers (sliders, etc.) often pass a `double` or nullable type, which can throw `InvalidCastException`; consider using `Convert.ToInt32`/`Convert.ToDouble` and null-safe handling instead of direct casts.
- The new direct-config setters bypass the old `ModBase.Setup` event/side-effect mechanism; for options that previously relied on change callbacks (e.g. logo type/title updates, some UI refresh actions), double-check that equivalent calls (like `ModSetup.UiLogoType/UiLogoText/UiLogoLeft`) are still invoked on user changes rather than only at startup.
- In `MyHint`, the loaded/close logic now uses `ConfigService.TryGetConfigItemNoType(...).GetValueNoType()` to decide visibility and persist dismissal; verify that the corresponding config items have `null` as their unset state (or use `IsDefault`) so that hints are not hidden immediately due to a non-null default value.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
重构启动器、UI 和 Minecraft 模块代码,使其使用新的强类型配置系统来替代
ModBase.Setup和VB.CompilerServices,引入基于标签的映射辅助方法用于保存设置,并将旧版迁移、校验和下载逻辑与更新后的配置和状态结构对齐。Enhancements:
Config/ConfigService访问方式替换ModBase.Setup以及 Conversions/Operators 的使用。string.CompareOrdinal、int.Parse、double.Parse等)现代化版本号、UUID 和数值处理逻辑,并简化布尔值判断。States字段,而不是通用的 setup 键,从而在处理加密设置时提高安全性。Original summary in English
Summary by Sourcery
Refactor launcher, UI, and Minecraft module code to use the new strongly-typed configuration system instead of ModBase.Setup and VB.CompilerServices, introducing tag-based mapping helpers for saving settings and aligning legacy migrations, validation, and download logic with the updated config and state structures.
Enhancements:
增强内容:
Config/ConfigService进行的强类型配置访问(包括对实例级设置的支持),替换对ModBase.Setup的直接 get/set/reset 调用。IsDefault/Reset/DefaultValue)。Original summary in English
Summary by Sourcery
重构启动器、UI 和 Minecraft 模块代码,使其使用新的强类型配置系统来替代
ModBase.Setup和VB.CompilerServices,引入基于标签的映射辅助方法用于保存设置,并将旧版迁移、校验和下载逻辑与更新后的配置和状态结构对齐。Enhancements:
Config/ConfigService访问方式替换ModBase.Setup以及 Conversions/Operators 的使用。string.CompareOrdinal、int.Parse、double.Parse等)现代化版本号、UUID 和数值处理逻辑,并简化布尔值判断。States字段,而不是通用的 setup 键,从而在处理加密设置时提高安全性。Original summary in English
Summary by Sourcery
Refactor launcher, UI, and Minecraft module code to use the new strongly-typed configuration system instead of ModBase.Setup and VB.CompilerServices, introducing tag-based mapping helpers for saving settings and aligning legacy migrations, validation, and download logic with the updated config and state structures.
Enhancements: