refactor: 使用配置系统全面取代 ModBase.Setup 并移除部分 VB.CompilerServices#2866
Conversation
审阅者指南将启动器、Minecraft 和 UI 代码重构为使用新的强类型 Config/ConfigService 和 States 结构,替代 ModBase.Setup 和 VB.CompilerServices;为从 UI 控件保存全局和按实例设置引入基于 Tag 的映射辅助方法;并在更新配置模型的同时,对遗留迁移、校验、下载和事件逻辑进行对齐以及现代化各种字符串/数字处理路径。 基于 Tag 的 UI 设置更新时序图sequenceDiagram
actor User
participant PageSetupUI
participant SetByTag
participant Config
User->>PageSetupUI: Drag SliderLauncherOpacity
PageSetupUI->>PageSetupUI: SliderChange(sender, user)
PageSetupUI->>SetByTag: SetByTag(sender.Tag, sender.Value)
SetByTag->>Config: Preference.Theme.WindowOpacity = value
User->>PageSetupUI: Toggle CheckMusicStop
PageSetupUI->>PageSetupUI: CheckBoxChange(sender, user)
PageSetupUI->>SetByTag: SetByTag(sender.Tag, sender.Checked)
SetByTag->>Config: Preference.Music.StopInGame = value
文件级改动
可能关联的 Issue
提示与命令与 Sourcery 交互
自定义使用体验访问你的 dashboard 以:
获取帮助Original review guide in EnglishReviewer's GuideRefactors launcher, Minecraft, and UI code to use the new strongly-typed Config/ConfigService and States structures instead of ModBase.Setup and VB.CompilerServices, introduces tag-based mapping helpers for saving global and per-instance settings from UI controls, and aligns legacy migration, validation, download, and event logic with the updated configuration model while modernizing various string/number handling paths. Sequence diagram for tag-based UI setting updatessequenceDiagram
actor User
participant PageSetupUI
participant SetByTag
participant Config
User->>PageSetupUI: Drag SliderLauncherOpacity
PageSetupUI->>PageSetupUI: SliderChange(sender, user)
PageSetupUI->>SetByTag: SetByTag(sender.Tag, sender.Value)
SetByTag->>Config: Preference.Theme.WindowOpacity = value
User->>PageSetupUI: Toggle CheckMusicStop
PageSetupUI->>PageSetupUI: CheckBoxChange(sender, user)
PageSetupUI->>SetByTag: SetByTag(sender.Tag, sender.Checked)
SetByTag->>Config: Preference.Music.StopInGame = value
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.
whitecat346
left a comment
There was a problem hiding this comment.
i think this pr is not have problem. but you can try to improve the quality as the AI said
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体性的反馈:
- 新增的基于 tag 的 setter(例如 SetByTag/SetLaunchByTag/SetGameManageByTag/SetInstanceByTag)直接进行了类型转换,如 (int)value 和 (bool)value,但调用方经常传入的是 Slider.Value(double)和 CheckBox.Checked(bool?),这样在运行时会抛出 InvalidCastException。建议改为使用 Convert.ToInt32 / Convert.ToBoolean,或者根据运行时类型做模式匹配。
- 这些基于 tag 的 switch 帮助方法现在把配置写入逻辑集中到了一个地方,但它们依赖字符串 tag 的取值与 XAML 保持完全一致;为了在后续重构时更容易发现不匹配问题,可能更安全的做法是在 default 分支中对未知/空的 tag 做防护,并记录日志或抛出异常。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- The new tag-based setters (e.g. SetByTag/SetLaunchByTag/SetGameManageByTag/SetInstanceByTag) are performing direct casts like (int)value and (bool)value, but the callers often pass Slider.Value (double) and CheckBox.Checked (bool?) so this will throw InvalidCastException at runtime; consider using Convert.ToInt32 / Convert.ToBoolean or pattern matching on the runtime type instead.
- The tag-switch helper methods now centralize config writes, but they depend on string tag values staying perfectly in sync with XAML; it may be safer to guard unknown/empty tags with a default case that logs or throws, to make mismatches easier to detect during future refactors.帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈来改进之后的评审。
Original comment in English
Hey - I've left some high level feedback:
- The new tag-based setters (e.g. SetByTag/SetLaunchByTag/SetGameManageByTag/SetInstanceByTag) are performing direct casts like (int)value and (bool)value, but the callers often pass Slider.Value (double) and CheckBox.Checked (bool?) so this will throw InvalidCastException at runtime; consider using Convert.ToInt32 / Convert.ToBoolean or pattern matching on the runtime type instead.
- The tag-switch helper methods now centralize config writes, but they depend on string tag values staying perfectly in sync with XAML; it may be safer to guard unknown/empty tags with a default case that logs or throws, to make mismatches easier to detect during future refactors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new tag-based setters (e.g. SetByTag/SetLaunchByTag/SetGameManageByTag/SetInstanceByTag) are performing direct casts like (int)value and (bool)value, but the callers often pass Slider.Value (double) and CheckBox.Checked (bool?) so this will throw InvalidCastException at runtime; consider using Convert.ToInt32 / Convert.ToBoolean or pattern matching on the runtime type instead.
- The tag-switch helper methods now centralize config writes, but they depend on string tag values staying perfectly in sync with XAML; it may be safer to guard unknown/empty tags with a default case that logs or throws, to make mismatches easier to detect during future refactors.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Sourcery 总结
在启动器 UI、实例设置和 Minecraft 模块中,用强类型配置 API 替换遗留的
ModBase.Setup用法,同时简化 VB 兼容性辅助方法,并使迁移逻辑和提示逻辑与新的配置/状态布局保持一致。Bug 修复:
增强改进:
Config/ConfigService属性,而不是使用ModBase.Setup。Microsoft.VisualBasic.CompilerServices中的运算符和Conversions,改用原生 C# API 来简化版本和字符串比较、数值解析以及条件检查。Original summary in English
Summary by Sourcery
Replace legacy ModBase.Setup usage with strongly-typed configuration APIs across launcher UI, instance settings, and Minecraft modules, while simplifying VB compatibility helpers and aligning migration and hint logic with the new config/state layout.
Bug Fixes:
Enhancements:
Original summary in English
Sourcery 总结
在启动器 UI、实例设置和 Minecraft 模块中,用强类型配置 API 替换遗留的
ModBase.Setup用法,同时简化 VB 兼容性辅助方法,并使迁移逻辑和提示逻辑与新的配置/状态布局保持一致。Bug 修复:
增强改进:
Config/ConfigService属性,而不是使用ModBase.Setup。Microsoft.VisualBasic.CompilerServices中的运算符和Conversions,改用原生 C# API 来简化版本和字符串比较、数值解析以及条件检查。Original summary in English
Summary by Sourcery
Replace legacy ModBase.Setup usage with strongly-typed configuration APIs across launcher UI, instance settings, and Minecraft modules, while simplifying VB compatibility helpers and aligning migration and hint logic with the new config/state layout.
Bug Fixes:
Enhancements:
增强内容:
Config/ConfigService进行的强类型配置访问(包括对实例级设置的支持),替换对ModBase.Setup的直接 get/set/reset 调用。IsDefault/Reset/DefaultValue)。Original summary in English
Sourcery 总结
在启动器 UI、实例设置和 Minecraft 模块中,用强类型配置 API 替换遗留的
ModBase.Setup用法,同时简化 VB 兼容性辅助方法,并使迁移逻辑和提示逻辑与新的配置/状态布局保持一致。Bug 修复:
增强改进:
Config/ConfigService属性,而不是使用ModBase.Setup。Microsoft.VisualBasic.CompilerServices中的运算符和Conversions,改用原生 C# API 来简化版本和字符串比较、数值解析以及条件检查。Original summary in English
Summary by Sourcery
Replace legacy ModBase.Setup usage with strongly-typed configuration APIs across launcher UI, instance settings, and Minecraft modules, while simplifying VB compatibility helpers and aligning migration and hint logic with the new config/state layout.
Bug Fixes:
Enhancements:
Original summary in English
Sourcery 总结
在启动器 UI、实例设置和 Minecraft 模块中,用强类型配置 API 替换遗留的
ModBase.Setup用法,同时简化 VB 兼容性辅助方法,并使迁移逻辑和提示逻辑与新的配置/状态布局保持一致。Bug 修复:
增强改进:
Config/ConfigService属性,而不是使用ModBase.Setup。Microsoft.VisualBasic.CompilerServices中的运算符和Conversions,改用原生 C# API 来简化版本和字符串比较、数值解析以及条件检查。Original summary in English
Summary by Sourcery
Replace legacy ModBase.Setup usage with strongly-typed configuration APIs across launcher UI, instance settings, and Minecraft modules, while simplifying VB compatibility helpers and aligning migration and hint logic with the new config/state layout.
Bug Fixes:
Enhancements: