Release 1.1.1.0#26
Conversation
refactor: migrate UI to Material3.WinForms library
WalkthroughThe release replaces legacy custom WinForms infrastructure with Material3.WinForms, adds automated Steam WARNO version switching with rollback, removes local executable/catalog persistence, cleans legacy settings on startup, and marks YSM Installer as superseded by Yuri’s WARNO Toolkit. ChangesMaterial3 UI shell and project wiring
Build selection and Steam branch switching
Final-release cleanup and documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Forms/Form1.Installation.cs (1)
364-418: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCap the action-card subtitle width
src/Forms/Form1.Installation.cs:364-418—gridputstextStackin aPercent(100)column with no wrap cap. MirrorBuildMessageCardand setMaximumSizeon the subtitle label so long switch descriptions don’t blow out the card.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Forms/Form1.Installation.cs` around lines 364 - 418, Update BuildActionCard so the subtitle Label added to textStack has a MaximumSize matching the width cap used by BuildMessageCard. Preserve the existing title and button layout while ensuring long switch descriptions wrap within the action card instead of expanding the card.Source: Path instructions
src/Utilities/UpdateService.cs (1)
94-118: 🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy liftAuto-update path executes an unverified downloaded binary.
FetchLatestReleaseAsync/IsSafeDownloadUrlonly enforce HTTPS transport;DownloadAndRestartAsyncthen copiestempExePathover the running installer and relaunches it with no hash or signature check against the downloaded payload. A compromised release asset or GitHub account takeover gets silent code execution on every user who accepts the update prompt.Pin a checksum/signature (e.g. publish a SHA-256 or code-signing cert alongside the release asset) and verify it before
Process.Startlaunches the updater script.As per path instructions for
src/Utilities/UpdateService.cs: "the downloaded payload is hash/signature-verified before execution."Also applies to: 162-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Utilities/UpdateService.cs` around lines 94 - 118, Require downloaded update payloads to be cryptographically verified before execution: extend FetchLatestReleaseAsync to obtain the expected SHA-256 checksum or trusted code-signing certificate associated with the release, then have DownloadAndRestartAsync validate tempExePath against it before copying or launching the updater. Reject and clean up any payload that fails verification, and preserve Process.Start only for successfully verified binaries.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Forms/Form1.BuildSelect.cs`:
- Around line 43-44: Update the identifier comparisons in the release filtering
logic around _supportedVersions and the code at the additional referenced
location to use string.Equals with StringComparison.Ordinal instead of ==. Apply
this consistently to identifier, path, and key comparisons in
Form1.BuildSelect.cs while preserving the existing selection behavior.
In `@src/Forms/Form1.Dev.cs`:
- Around line 382-390: Update DevRunInstall so the StartInstallAsync invocation
is executed through the existing SafeFireDev wrapper, preserving the selected
target and version arguments while ensuring asynchronous failures are caught and
reported via the established AppLogger.Critical path.
In `@src/Forms/Form1.Layout.cs`:
- Around line 180-188: Remove the icon.Disposed handler that disposes icon.Image
in the icon construction block, leaving the cached bitmap from
MaterialIconRenderer.Get untouched. Follow the existing ownership pattern used
by badgeIcon.Image, while retaining disposal only for freshly allocated images
such as those returned by BuildSuccessBadge.
In `@src/Utilities/AcfKeyValues.cs`:
- Around line 110-120: Update the key comparison in AcfKeyValues.FindChild to
use StringComparison.Ordinal instead of StringComparison.OrdinalIgnoreCase,
ensuring only an exact-case manifest key is matched while preserving the
existing child traversal and null behavior.
- Around line 95-100: Update the escape-handling branch in the parser around the
backslash logic to preserve unrecognized escape sequences, including the leading
backslash, so Serialize() can reproduce them unchanged. Continue decoding the
recognized \n and \t sequences, while retaining the existing handling for
supported escapes such as \\ and \".
In `@src/Utilities/IniFile.cs`:
- Around line 81-82: The comment near the atomic write logic in IniFile should
accurately describe that WriteAllLines writes to path + ".tmp" before
replacement. Remove the claim that File.Replace is NTFS-journal-atomic and use a
concise plain explanation of the temporary-file-then-replace flow.
In `@src/Warno/SteamVersionSwitcher.cs`:
- Around line 64-71: Update ReadCurrentBranch and ReadMountedBranch to return an
explicit success/failure result instead of using an empty branch string as the
error sentinel. In the initial switch flow, abort when reading the original
branch fails; during rollback or mounted-branch handling, treat an unknown
mounted branch as requiring StateFlagsNeedsUpdate so it cannot be mistaken for
the valid public branch.
- Around line 205-250: Update the manifest polling logic around
AcfKeyValues.Parse and the snapshot stall guard so failed reads do not replace
the last valid snapshot with an empty value or reset lastChange. Track whether
the current read produced a valid snapshot, preserving the prior snapshot on
transient IOException/FormatException retries; ensure FileNotFoundException is
not swallowed and propagates to the caller.
- Around line 183-195: Update LaunchSteam to propagate failures after logging
the exception instead of swallowing them, so callers can stop the normal switch
wait and report rollback failure when Steam does not launch. Preserve the
existing Steam executable selection and AppLogger.Error call, and ensure the
exception reaches the methods that invoke LaunchSteam.
---
Outside diff comments:
In `@src/Forms/Form1.Installation.cs`:
- Around line 364-418: Update BuildActionCard so the subtitle Label added to
textStack has a MaximumSize matching the width cap used by BuildMessageCard.
Preserve the existing title and button layout while ensuring long switch
descriptions wrap within the action card instead of expanding the card.
In `@src/Utilities/UpdateService.cs`:
- Around line 94-118: Require downloaded update payloads to be cryptographically
verified before execution: extend FetchLatestReleaseAsync to obtain the expected
SHA-256 checksum or trusted code-signing certificate associated with the
release, then have DownloadAndRestartAsync validate tempExePath against it
before copying or launching the updater. Reject and clean up any payload that
fails verification, and preserve Process.Start only for successfully verified
binaries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 813b29fe-b2fa-4bda-be5f-d32de91eb751
⛔ Files ignored due to path filters (45)
src/Forms/Form1.Designer.csis excluded by!**/*.designer.cs,!**/*.Designer.cssrc/Forms/StepsForm.Designer.csis excluded by!**/*.designer.cs,!**/*.Designer.cssrc/Forms/StepsForm.resxis excluded by!**/*.resxsrc/Properties/Resources.Designer.csis excluded by!**/*.designer.cs,!**/*.Designer.cssrc/Properties/Resources.resxis excluded by!**/*.resxsrc/Properties/Settings.Designer.csis excluded by!**/*.designer.cs,!**/*.Designer.cssrc/Resources/Screenshot 2025-03-02 182641.pngis excluded by!**/*.png,!**/Resources/**src/Resources/Screenshot 2025-03-02 182659.pngis excluded by!**/*.png,!**/Resources/**src/Resources/Screenshot 2025-03-02 182718.pngis excluded by!**/*.png,!**/Resources/**src/Resources/Screenshot 2025-03-02 182724.pngis excluded by!**/*.png,!**/Resources/**src/Resources/icons/arrow_drop_down.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/arrow_forward.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/check.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/check_circle.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/check_circle_filled.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/check_filled.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/close.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/cloud.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/deployed_code.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/download.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/error.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/error_filled.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/expand_more.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/folder.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/folder_open.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/help.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/help_outline.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/info.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/info_filled.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/layers.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/minimize.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/open_in_new.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/play_arrow.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/play_arrow_filled.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/radio_button_checked.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/radio_button_unchecked.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/refresh.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/search_off.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/settings.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/tips_and_updates.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/travel_explore.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/videogame_asset.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/warning.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/warning_filled.svgis excluded by!**/*.svg,!**/Resources/**src/Resources/icons/ysm.pngis excluded by!**/*.png,!**/Resources/**
📒 Files selected for processing (62)
README.mdsrc/Controls/Material/MaterialButton.cssrc/Controls/Material/MaterialCard.cssrc/Controls/Material/MaterialOptionCard.cssrc/Controls/Material/MaterialProgressBar.cssrc/Controls/Material/MaterialRadioCard.cssrc/Controls/Material/MaterialScrollPanel.cssrc/Controls/Material/MaterialTitleBar.cssrc/Controls/Material/SkeletonCard.cssrc/Controls/Material/SoftLabel.cssrc/Controls/Material/StepChecklist.cssrc/Controls/Rounded/RoundedButton.cssrc/Controls/Rounded/RoundedControls.cssrc/Controls/Rounded/RoundedPanel.cssrc/Controls/Ui/DropdownSelect.cssrc/Forms/BorderlessForm.cssrc/Forms/Form1.BuildSelect.cssrc/Forms/Form1.Dev.cssrc/Forms/Form1.Installation.cssrc/Forms/Form1.Layout.cssrc/Forms/Form1.ManualInstall.cssrc/Forms/Form1.Scan.cssrc/Forms/Form1.cssrc/Forms/MaterialDialog.cssrc/Forms/SettingsForm.cssrc/Forms/StepsForm.cssrc/Models/ModMetadata.cssrc/Properties/AssemblyInfo.cssrc/Properties/Program.cssrc/Properties/Settings.settingssrc/UI/AppLinks.cssrc/UI/BuildIcons.cssrc/UI/FormAnimation.cssrc/UI/FormDragAnywhere.cssrc/UI/MaterialIconRenderer.cssrc/UI/MaterialIcons.cssrc/UI/MaterialPalette.cssrc/UI/MaterialType.cssrc/UI/Sizes.cssrc/UI/TaskbarProgress.cssrc/UI/Tokens.cssrc/UI/UserMessages.cssrc/UI/WindowChrome.cssrc/Utilities/AcfKeyValues.cssrc/Utilities/Connectivity.cssrc/Utilities/DevService.cssrc/Utilities/IniFile.cssrc/Utilities/LegacySettingsCleanup.cssrc/Utilities/PathFormatting.cssrc/Utilities/RemoteHtmlResponseException.cssrc/Utilities/SafeArchiveExtractor.cssrc/Utilities/ShellOpen.cssrc/Utilities/SystemCursors.cssrc/Utilities/UpdateService.cssrc/Warno/DevWarnoMocks.cssrc/Warno/InstallWorkflow.cssrc/Warno/ModArchiveFormats.cssrc/Warno/ModCatalogSettings.cssrc/Warno/SteamVersionSwitcher.cssrc/Warno/WarnoFinder.cssrc/Warno/WarnoPaths.cssrc/YSMInstaller.csproj
💤 Files with no reviewable changes (32)
- src/Utilities/PathFormatting.cs
- src/Controls/Rounded/RoundedPanel.cs
- src/Utilities/Connectivity.cs
- src/UI/Sizes.cs
- src/UI/TaskbarProgress.cs
- src/UI/FormAnimation.cs
- src/Properties/Settings.settings
- src/Controls/Material/MaterialButton.cs
- src/Controls/Rounded/RoundedButton.cs
- src/Controls/Material/SoftLabel.cs
- src/Controls/Ui/DropdownSelect.cs
- src/Controls/Material/MaterialTitleBar.cs
- src/Warno/InstallWorkflow.cs
- src/Controls/Material/SkeletonCard.cs
- src/UI/MaterialPalette.cs
- src/Forms/BorderlessForm.cs
- src/UI/MaterialType.cs
- src/Controls/Material/MaterialCard.cs
- src/Controls/Material/StepChecklist.cs
- src/UI/WindowChrome.cs
- src/Utilities/ShellOpen.cs
- src/UI/MaterialIcons.cs
- src/Utilities/SystemCursors.cs
- src/Controls/Material/MaterialProgressBar.cs
- src/UI/MaterialIconRenderer.cs
- src/Controls/Material/MaterialOptionCard.cs
- src/Forms/StepsForm.cs
- src/Forms/MaterialDialog.cs
- src/Controls/Rounded/RoundedControls.cs
- src/Controls/Material/MaterialScrollPanel.cs
- src/UI/FormDragAnywhere.cs
- src/Warno/ModArchiveFormats.cs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Warno/SteamVersionSwitcher.cs (1)
208-218: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
DirectoryNotFoundExceptionslips through the "transient" filter.
DirectoryNotFoundExceptionderives fromIOExceptionand isn't excluded likeFileNotFoundExceptionis (line 213). If the steamapps folder itself vanishes mid-switch, this gets retried as a transient glitch instead of propagating immediately — contradicting the comment's stated intent ("a vanished manifest ... propagates to the caller"). It'll still eventually fail via the 180s stall timeout, just with a much slower, mislabeled failure.🐛 Proposed fix
catch (Exception exception) when ( - (exception is IOException && !(exception is FileNotFoundException)) || exception is FormatException) { + (exception is IOException && !(exception is FileNotFoundException) && !(exception is DirectoryNotFoundException)) + || exception is FormatException) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Warno/SteamVersionSwitcher.cs` around lines 208 - 218, Update the exception filter around AcfKeyValues.Parse and File.ReadAllText to exclude DirectoryNotFoundException alongside FileNotFoundException, so a missing steamapps directory propagates immediately while other transient IOExceptions and FormatException remain retriable.src/Forms/Form1.Installation.cs (1)
364-419: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
BuildActionCard's click delegate runs unguarded — inconsistent with every other Click handler here.
actionBtn.Click += (s, e) => captured();(line 413) invokesonClickwith no try/catch. Its only caller passes() => RenderVersionSwitchPlan(switchOption)(RenderVersionMismatch, lines 331-337) — the sameRenderVersionSwitchPlancall is wrapped in try/catch +AppLogger.Criticalat its other two call sites (Form1.BuildSelect.cs:288-295, 351-358). An exception here (UI construction/layout) escapes uncaught instead of being logged, breaking the established convention.As per path instructions, "every async lambda passed to Click/Activated must have a try/catch around its body OR funnel through a helper that logs via AppLogger" — the same convention this codebase already applies to every other
RenderVersionSwitchPlancall site.🐛 Proposed fix
MaterialButton switchCard's action button, or fix at call site: - () => RenderVersionSwitchPlan(switchOption) + () => { + try { + RenderVersionSwitchPlan(switchOption); + } + catch (Exception ex) { + AppLogger.Critical("Render version switch plan failed (mismatch card).", ex); + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Forms/Form1.Installation.cs` around lines 364 - 419, Update the click handler created in BuildActionCard so invoking the captured onClick delegate is guarded by try/catch, matching the existing Click-handler convention. Log any exception through AppLogger.Critical while preserving the current delegate invocation behavior.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/Forms/Form1.Installation.cs`:
- Around line 364-419: Update the click handler created in BuildActionCard so
invoking the captured onClick delegate is guarded by try/catch, matching the
existing Click-handler convention. Log any exception through AppLogger.Critical
while preserving the current delegate invocation behavior.
In `@src/Warno/SteamVersionSwitcher.cs`:
- Around line 208-218: Update the exception filter around AcfKeyValues.Parse and
File.ReadAllText to exclude DirectoryNotFoundException alongside
FileNotFoundException, so a missing steamapps directory propagates immediately
while other transient IOExceptions and FormatException remain retriable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 127a3dc4-5b32-454f-9668-75b8f5c40d5c
📒 Files selected for processing (7)
src/Forms/Form1.BuildSelect.cssrc/Forms/Form1.Dev.cssrc/Forms/Form1.Installation.cssrc/Forms/Form1.Layout.cssrc/Utilities/AcfKeyValues.cssrc/Utilities/IniFile.cssrc/Warno/SteamVersionSwitcher.cs
💤 Files with no reviewable changes (1)
- src/Forms/Form1.Layout.cs
Summary by CodeRabbit