refactor: 重构 ModSetup.cs#2877
Open
LuLu-ling wants to merge 4 commits into
Open
Conversation
Reviewer's Guide将 ModSetup 从基于反射的实例级配置观察器重构为显式的、静态的 config-to-UI 绑定,并通过单一的 ApplyAll 初始化器完成启动初始化,从而简化配置变更处理与启动流程。 静态 config-to-UI 绑定与 ApplyAll 初始化的序列图sequenceDiagram
actor User
participant FormMain
participant PageSetupUI
participant Config
participant ConfigObserver
participant ModSetup
rect rgb(230,230,250)
note over FormMain,ModSetup: 启动初始化
FormMain->>ModSetup: ModSetup() constructor
ModSetup->>Config: Config.Preference.*.Observe(new ConfigObserver(...))
Config-->>ConfigObserver: register observer callbacks
FormMain->>ModSetup: ApplyAll()
ModSetup->>ModSetup: LaunchRamType(Config.Launch.MemoryAllocationMode)
ModSetup->>ModSetup: UiLauncherTheme(Config.Preference.Theme.ThemeSelected)
ModSetup->>ModSetup: UiFont(Config.Preference.Font)
ModSetup->>ModSetup: SystemHttpProxy(Config.Network.HttpProxy.CustomAddress)
ModSetup->>PageSetupUI: HiddenRefresh()
end
rect rgb(230,255,230)
note over User,ConfigObserver: 运行时配置变更传播
User->>PageSetupUI: interact controls
PageSetupUI->>PageSetupUI: SetByTag(tag, value)
PageSetupUI->>Config: update Config.Preference.*
Config-->>ConfigObserver: ConfigEvent.Changed
ConfigObserver->>ModSetup: UiFont((string)(e.Value ?? ""))
ModSetup->>PageSetupUI: HiddenRefresh() (via ApplyAll or Hide observer)
end
rect rgb(255,250,230)
note over PageSetupUI,ModSetup: UI 变更后的 ApplyAll
PageSetupUI->>ModSetup: ApplyAll()
ModSetup->>ModSetup: UiBlur(Config.Preference.Blur.IsEnabled)
ModSetup->>ModSetup: UiBlurValue(Config.Preference.Blur.Radius)
ModSetup->>ModSetup: UiLogoType((int)Config.Preference.WindowTitleType)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience打开你的 dashboard 以:
Getting HelpOriginal review guide in EnglishReviewer's GuideRefactors ModSetup from a reflection-based, instance-level config observer into an explicit, static config-to-UI wiring with a single ApplyAll initializer, simplifying configuration change handling and startup initialization. Sequence diagram for static config-to-UI wiring and ApplyAll initializationsequenceDiagram
actor User
participant FormMain
participant PageSetupUI
participant Config
participant ConfigObserver
participant ModSetup
rect rgb(230,230,250)
note over FormMain,ModSetup: Startup initialization
FormMain->>ModSetup: ModSetup() constructor
ModSetup->>Config: Config.Preference.*.Observe(new ConfigObserver(...))
Config-->>ConfigObserver: register observer callbacks
FormMain->>ModSetup: ApplyAll()
ModSetup->>ModSetup: LaunchRamType(Config.Launch.MemoryAllocationMode)
ModSetup->>ModSetup: UiLauncherTheme(Config.Preference.Theme.ThemeSelected)
ModSetup->>ModSetup: UiFont(Config.Preference.Font)
ModSetup->>ModSetup: SystemHttpProxy(Config.Network.HttpProxy.CustomAddress)
ModSetup->>PageSetupUI: HiddenRefresh()
end
rect rgb(230,255,230)
note over User,ConfigObserver: Runtime config change propagation
User->>PageSetupUI: interact controls
PageSetupUI->>PageSetupUI: SetByTag(tag, value)
PageSetupUI->>Config: update Config.Preference.*
Config-->>ConfigObserver: ConfigEvent.Changed
ConfigObserver->>ModSetup: UiFont((string)(e.Value ?? ""))
ModSetup->>PageSetupUI: HiddenRefresh() (via ApplyAll or Hide observer)
end
rect rgb(255,250,230)
note over PageSetupUI,ModSetup: ApplyAll after UI changes
PageSetupUI->>ModSetup: ApplyAll()
ModSetup->>ModSetup: UiBlur(Config.Preference.Blur.IsEnabled)
ModSetup->>ModSetup: UiBlurValue(Config.Preference.Blur.Radius)
ModSetup->>ModSetup: UiLogoType((int)Config.Preference.WindowTitleType)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - 我在这里给出一些高层面的反馈:
- 在移除了
ModBase.Setup之后,ModSetup构造函数看起来不再在任何地方被显式实例化,因此新的Config.*.Observe(...)注册可能永远不会被执行;建议将这些注册显式化(例如提供一个从启动代码中调用的静态初始化方法),或者重新引入一个明确会被使用的实例。 - 现在在
PageSetupUI中,每次调用SetByTag都会调用一次ModSetup.ApplyAll(),这会为了单个变更重新应用所有由配置驱动的 UI 更新;建议只更新受影响的设置,以避免重复工作以及可能的 UI 闪烁/性能问题。 - 大多数
ModSetup方法已经被改为静态,但仍然依赖全局 UI 状态,比如ModMain.FrmMain/ModMain.FrmSetupUI;更干净、更安全的做法是要么在使用前始终进行空值检查,要么将这部分逻辑从静态/全局状态中解耦,以提升健壮性和可测试性。
AI 代理提示
Please address the comments from this code review:
## Overall Comments
- After removing `ModBase.Setup`, the `ModSetup` constructor is no longer obviously instantiated anywhere, so the new `Config.*.Observe(...)` registrations may never run; consider making the registration explicit (e.g., a static initialization method that is called from startup) or re-introducing a clearly used instance.
- `ModSetup.ApplyAll()` is now called on every `SetByTag` in `PageSetupUI`, which re-applies all configuration-driven UI updates for a single change; consider updating only the affected setting to avoid redundant work and potential UI flicker/performance issues.
- Most `ModSetup` methods have been made static but still depend on global UI state like `ModMain.FrmMain`/`ModMain.FrmSetupUI`; it would be cleaner and safer to either consistently guard against null before use or decouple this logic from static/global state to improve robustness and testability.帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据这些反馈改进为你提供的评审。
Original comment in English
Hey - I've left some high level feedback:
- After removing
ModBase.Setup, theModSetupconstructor is no longer obviously instantiated anywhere, so the newConfig.*.Observe(...)registrations may never run; consider making the registration explicit (e.g., a static initialization method that is called from startup) or re-introducing a clearly used instance. ModSetup.ApplyAll()is now called on everySetByTaginPageSetupUI, which re-applies all configuration-driven UI updates for a single change; consider updating only the affected setting to avoid redundant work and potential UI flicker/performance issues.- Most
ModSetupmethods have been made static but still depend on global UI state likeModMain.FrmMain/ModMain.FrmSetupUI; it would be cleaner and safer to either consistently guard against null before use or decouple this logic from static/global state to improve robustness and testability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- After removing `ModBase.Setup`, the `ModSetup` constructor is no longer obviously instantiated anywhere, so the new `Config.*.Observe(...)` registrations may never run; consider making the registration explicit (e.g., a static initialization method that is called from startup) or re-introducing a clearly used instance.
- `ModSetup.ApplyAll()` is now called on every `SetByTag` in `PageSetupUI`, which re-applies all configuration-driven UI updates for a single change; consider updating only the affected setting to avoid redundant work and potential UI flicker/performance issues.
- Most `ModSetup` methods have been made static but still depend on global UI state like `ModMain.FrmMain`/`ModMain.FrmSetupUI`; it would be cleaner and safer to either consistently guard against null before use or decouple this logic from static/global state to improve robustness and testability.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
用显式的观察者和集中式的配置应用逻辑替代基于反射的 ModSetup 配置处理方式,从而将配置值应用到 UI。
新功能:
ApplyAll,在启动期间以及设置变更时,将当前所有配置值应用到启动器 UI。增强与改进:
ModSetup.ApplyAll,实现 UI 初始化逻辑的集中管理。Original summary in English
Summary by Sourcery
Replace reflection-based ModSetup config handling with explicit observers and centralized application of config values to the UI.
New Features:
Enhancements: