Conversation
Cap window scale to available screen
Handle malformed Mii data without crashing
Fix Mii editor color selection guard
📝 WalkthroughWalkthroughWheelWizard v2.4.8 adds a window scale validation and clamping system to prevent invalid UI scaling factors, enhances Mii deserialization with graceful error handling, fixes eye/eyebrow color comparison logic in the Mii editor, and refactors ZIP extraction to use stream-based file abstraction. ChangesVersion 2.4.8 Release
Window Scale Validation and Clamping System
Mii Deserialization Robustness
Bug Fixes
Sequence Diagram(s)sequenceDiagram
participant User
participant WhWzSettings
participant ViewUtils
participant SettingsService
participant Layout
User->>WhWzSettings: Select new window scale
WhWzSettings->>ViewUtils: GetUsableWindowScale(requestedScale, size, window)
ViewUtils-->>WhWzSettings: normalizedScale
WhWzSettings->>WhWzSettings: Update dropdown display to normalized scale
WhWzSettings->>SettingsService: Set(WINDOW_SCALE, normalizedScale)
alt Set succeeds
SettingsService-->>WhWzSettings: true
WhWzSettings->>Layout: OnSettingChanged(WINDOW_SCALE)
Layout->>Layout: Recalculate dimensions with new scale
Layout-->>User: Window resized
else Set fails (out of bounds)
SettingsService-->>WhWzSettings: false
WhWzSettings->>WhWzSettings: Revert dropdown to current WINDOW_SCALE
WhWzSettings-->>User: No change applied
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
WheelWizard/Features/CustomDistributions/RetroRewindBeta.cs (1)
224-224:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the injected file system abstraction for consistency.
Line 224 uses
File.Createdirectly instead of_fileSystem.File.Create, breaking the abstraction pattern used throughout the class. This makes the extraction logic harder to test with mocked file systems.🔧 Proposed fix
- using var outputStream = File.Create(destinationPath); + using var outputStream = _fileSystem.File.Create(destinationPath);🤖 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 `@WheelWizard/Features/CustomDistributions/RetroRewindBeta.cs` at line 224, Replace the direct System.IO call with the injected file system abstraction: locate the line using File.Create(destinationPath) inside the RetroRewindBeta class and change it to use the injected _fileSystem.File.Create(destinationPath) so the extraction logic consistently uses the mocked/testable IFileSystem; keep the existing using/disposal semantics (using var outputStream ...) and ensure any tests/mock setups reference _fileSystem.
🤖 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 `@WheelWizard/Features/WiiManagement/MiiManagement/MiiSerializer.cs`:
- Around line 158-171: The catch in Deserialize currently swallows all
exceptions from DeserializeCore and returns InvalidDataExc(ex); add a
debug-level log of the caught exception before returning so programming errors
are visible in diagnostics without changing the returned OperationResult. Update
the catch (Exception ex) block in Deserialize to call the project's logging
facility (e.g., Trace/ILogger/Serilog) to log a short contextual message like
"Deserialize failed" along with ex (stack and message) and then return
InvalidDataExc(ex); reference Deserialize, DeserializeCore, and InvalidDataExc
when making the change.
In `@WheelWizard/Views/Layout.axaml.cs`:
- Around line 167-176: The clamp currently runs in the Layout() constructor and
may use incorrect screen info; remove the call to
ClampSavedWindowScaleToCurrentScreen() from the Layout() constructor and instead
invoke it once the window is shown by overriding OnOpened or handling the
Opened/Loaded event (e.g., override OnOpened or subscribe to Opened and call
ClampSavedWindowScaleToCurrentScreen there). Keep the existing
ClampSavedWindowScaleToCurrentScreen() and GetUsableWindowScale() (which calls
ViewUtils.GetUsableWindowScale) and ensure it updates
SettingsService.SAVED_WINDOW_SCALE only after the window is opened so
ScreenFromWindow/Primary use correct geometry.
---
Outside diff comments:
In `@WheelWizard/Features/CustomDistributions/RetroRewindBeta.cs`:
- Line 224: Replace the direct System.IO call with the injected file system
abstraction: locate the line using File.Create(destinationPath) inside the
RetroRewindBeta class and change it to use the injected
_fileSystem.File.Create(destinationPath) so the extraction logic consistently
uses the mocked/testable IFileSystem; keep the existing using/disposal semantics
(using var outputStream ...) and ensure any tests/mock setups reference
_fileSystem.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e785248b-facb-429c-bb66-37be4627140d
📒 Files selected for processing (14)
Flatpak/io.github.TeamWheelWizard.WheelWizard.metainfo.xmlWheelWizard.Test/Features/MiiSerializerTests.csWheelWizard.Test/Features/Settings/SettingsTests.csWheelWizard/Features/CustomDistributions/RetroRewindBeta.csWheelWizard/Features/Settings/SettingsManager.csWheelWizard/Features/Settings/Types/SettingConstants.csWheelWizard/Features/WiiManagement/MiiManagement/MiiSerializer.csWheelWizard/Views/Layout.axaml.csWheelWizard/Views/Pages/Settings/WhWzSettings.axaml.csWheelWizard/Views/Popups/Base/PopupWindow.axaml.csWheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorEyebrows.axaml.csWheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorEyes.axaml.csWheelWizard/Views/ViewUtils.csWheelWizard/WheelWizard.csproj
| public static OperationResult<Mii> Deserialize(byte[]? data) | ||
| { | ||
| try | ||
| { | ||
| return DeserializeCore(data); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| return InvalidDataExc(ex); | ||
| } | ||
| } | ||
|
|
||
| private static OperationResult<Mii> DeserializeCore(byte[]? data) | ||
| { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Robustness refactor looks correct; consider logging the swallowed exception.
Routing DeserializeCore exceptions (including the impossible-date case from line 187) into a failure OperationResult is the right call for parsing untrusted Mii data. One caveat: catch (Exception ex) also absorbs genuine programming errors (e.g. IndexOutOfRangeException) and reports them as generic "invalid data". The message is preserved in the result, but a debug-level log here would aid diagnosis without changing behavior.
🤖 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 `@WheelWizard/Features/WiiManagement/MiiManagement/MiiSerializer.cs` around
lines 158 - 171, The catch in Deserialize currently swallows all exceptions from
DeserializeCore and returns InvalidDataExc(ex); add a debug-level log of the
caught exception before returning so programming errors are visible in
diagnostics without changing the returned OperationResult. Update the catch
(Exception ex) block in Deserialize to call the project's logging facility
(e.g., Trace/ILogger/Serilog) to log a short contextual message like
"Deserialize failed" along with ex (stack and message) and then return
InvalidDataExc(ex); reference Deserialize, DeserializeCore, and InvalidDataExc
when making the change.
| private void ClampSavedWindowScaleToCurrentScreen() | ||
| { | ||
| var savedScale = SettingsService.Get<double>(SettingsService.SAVED_WINDOW_SCALE); | ||
| var usableScale = GetUsableWindowScale(savedScale); | ||
| if (!savedScale.Equals(usableScale)) | ||
| SettingsService.Set(SettingsService.SAVED_WINDOW_SCALE, usableScale); | ||
| } | ||
|
|
||
| private double GetUsableWindowScale(double requestedScale) => | ||
| ViewUtils.GetUsableWindowScale(requestedScale, new Size(WindowWidth, WindowHeight), this); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Avalonia 11.2.5 Window.Screens ScreenFromWindow availability before window shown
💡 Result:
In Avalonia 11.2.5, the Window.Screens.ScreenFromWindow(window) method is not reliable or may not return valid screen information before a window has been shown (i.e., before it has a valid platform implementation) [1][2]. The ScreenFromWindow method requires a TopLevel (or WindowBase) instance that has been initialized with a platform-specific implementation [2][3]. Before the window is shown, the PlatformImpl property is often null or uninitialized, which can lead to unexpected results or exceptions depending on the specific platform and state [2]. If you need to access screen information before a window is shown, consider these alternatives: 1. Use the static Screens API: You can access the general Screens collection via TopLevel.Screens or by using the global screen management services available in Avalonia to query primary or available displays without needing a specific window instance [4]. 2. Defer logic: If you need to position or size a window based on a specific screen, perform this logic in the Window's Opened event or an equivalent lifecycle hook where the PlatformImpl is guaranteed to be available [1][4]. 3. Platform-specific APIs: If you strictly require display information before any Avalonia UI elements are initialized, you may need to use platform-native APIs (such as Win32 APIs on Windows) or cross-platform libraries that operate independently of the Avalonia framework [5].
Citations:
- 1: Issues with screens API AvaloniaUI/Avalonia#3795
- 2: https://github.com/AvaloniaUI/Avalonia/blob/master/src/Avalonia.Controls/Screens.cs
- 3: https://api-docs.avaloniaui.net/docs/M_Avalonia_Controls_Screens_ScreenFromWindow
- 4: https://docs.avaloniaui.net/docs/app-development/window-management
- 5: How to get Avalonia.Controls.Screens instance without access to an instance of WindowBase? AvaloniaUI/Avalonia#14067
🏁 Script executed:
rg -n "GetUsableWindowScale" -S .Repository: TeamWheelWizard/WheelWizard
Length of output: 1055
🏁 Script executed:
rg -n "GetUsableWindowScale" -S .Repository: TeamWheelWizard/WheelWizard
Length of output: 1055
🏁 Script executed:
rg -n "GetUsableWindowScale" -S .Repository: TeamWheelWizard/WheelWizard
Length of output: 1055
🏁 Script executed:
sed -n '1,120p' WheelWizard/Views/ViewUtils.csRepository: TeamWheelWizard/WheelWizard
Length of output: 4997
🏁 Script executed:
sed -n '120,220p' WheelWizard/Views/Layout.axaml.csRepository: TeamWheelWizard/WheelWizard
Length of output: 4105
🏁 Script executed:
rg -n "ClampSavedWindowScaleToCurrentScreen\\(" WheelWizard/Views/Layout.axaml.csRepository: TeamWheelWizard/WheelWizard
Length of output: 182
🏁 Script executed:
rg -n "ClampSavedWindowScaleToCurrentScreen\\(" WheelWizard/Views/Layout.axaml.csRepository: TeamWheelWizard/WheelWizard
Length of output: 182
🏁 Script executed:
sed -n '50,110p' WheelWizard/Views/Layout.axaml.csRepository: TeamWheelWizard/WheelWizard
Length of output: 2197
🏁 Script executed:
sed -n '50,110p' WheelWizard/Views/Layout.axaml.csRepository: TeamWheelWizard/WheelWizard
Length of output: 2197
🏁 Script executed:
head -n 40 WheelWizard/Views/Layout.axaml.csRepository: TeamWheelWizard/WheelWizard
Length of output: 1461
🏁 Script executed:
head -n 40 WheelWizard/Views/Layout.axaml.csRepository: TeamWheelWizard/WheelWizard
Length of output: 1461
🏁 Script executed:
rg -n "MinWindowScale|MaxWindowScale" WheelWizardRepository: TeamWheelWizard/WheelWizard
Length of output: 739
🏁 Script executed:
sed -n '1,80p' WheelWizard/Features/Settings/Types/SettingConstants.csRepository: TeamWheelWizard/WheelWizard
Length of output: 2367
🏁 Script executed:
rg -n "class BaseWindow" -S WheelWizardRepository: TeamWheelWizard/WheelWizard
Length of output: 147
🏁 Script executed:
sed -n '1,80p' WheelWizard/Features/Settings/Types/SettingConstants.csRepository: TeamWheelWizard/WheelWizard
Length of output: 2367
🏁 Script executed:
rg -n "Avalonia\\.?|PackageReference.*Avalonia" -S . --glob '*.csproj' --glob '*.props' --glob '*.targets'Repository: TeamWheelWizard/WheelWizard
Length of output: 1199
🏁 Script executed:
rg -n "Avalonia\\.?|PackageReference.*Avalonia" -S . --glob '*.csproj' --glob '*.props' --glob '*.targets'Repository: TeamWheelWizard/WheelWizard
Length of output: 1199
Defer screen-based clamping until the window is opened
WheelWizard/Views/Layout.axaml.cscallsClampSavedWindowScaleToCurrentScreen()insidepublic Layout()(constructor), before the window is shown.ViewUtils.GetUsableWindowScale(...)useswindow.Screens.ScreenFromWindow(window) ?? window.Screens.Primary; in Avalonia 11.2.5ScreenFromWindowmay not provide reliable screen geometry until after show/open, so multi-monitor setups can clamp against the wrong screen.- Move the clamp to
Opened/OnOpened(orLoaded) so the correctWorkingArea/Scalingare available before updatingSAVED_WINDOW_SCALE.
🤖 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 `@WheelWizard/Views/Layout.axaml.cs` around lines 167 - 176, The clamp
currently runs in the Layout() constructor and may use incorrect screen info;
remove the call to ClampSavedWindowScaleToCurrentScreen() from the Layout()
constructor and instead invoke it once the window is shown by overriding
OnOpened or handling the Opened/Loaded event (e.g., override OnOpened or
subscribe to Opened and call ClampSavedWindowScaleToCurrentScreen there). Keep
the existing ClampSavedWindowScaleToCurrentScreen() and GetUsableWindowScale()
(which calls ViewUtils.GetUsableWindowScale) and ensure it updates
SettingsService.SAVED_WINDOW_SCALE only after the window is opened so
ScreenFromWindow/Primary use correct geometry.
1 participant
Release Wheel Wizard 2.4.8.\n\nValidation:\n- dotnet restore WheelWizard.sln\n- dotnet test WheelWizard.sln --configuration Release --verbosity normal\n- dotnet test WheelWizard.sln --configuration Release --verbosity minimal
Summary by CodeRabbit
Bug Fixes
New Features
Tests
Chores