Skip to content

Offline mii rendering#240

Open
patchzyy wants to merge 15 commits intodevfrom
Offline-mii-rendering
Open

Offline mii rendering#240
patchzyy wants to merge 15 commits intodevfrom
Offline-mii-rendering

Conversation

@patchzyy
Copy link
Member

@patchzyy patchzyy commented Mar 8, 2026

Render Mii's offline + automatic download path for mii file

Summary by CodeRabbit

Release Notes

  • New Features

    • Added native Mii rendering with interactive 3D view featuring camera zoom and rotation controls
    • Added automated Mii rendering resource installation system
    • Redesigned Mii list interface with improved grid organization
  • Changed

    • Updated Mii editor and viewer to display interactive 3D models instead of carousel previews
    • Enhanced image display specifications with improved rendering options

@coderabbitai
Copy link

coderabbitai bot commented Mar 8, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b4967381-0fb4-4a2e-b898-b18e27ca24a7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR refactors Mii image loading from API-based to native rendering. It introduces a MiiRendering subsystem with resource installation, native rendering interfaces, and new 3D UI components (Mii3DRender replacing MiiCarousel). The system manages cached resources locally, removes Refit API dependency, and updates image specifications to support new rendering parameters like CameraZoom and RenderScale.

Changes

Cohort / File(s) Summary
Package & Resource Configuration
WheelWizard.Test/WheelWizard.Test.csproj, WheelWizard.csproj, WheelWizard/Services/PathManager.cs, WheelWizard/Services/Endpoints.cs
Added SixLabors.ImageSharp NuGet reference, embedded MiiRendering resources, added MiiRendering folder paths, replaced Mii image endpoints with Internet Archive base address and rendering archive path.
Mii Rendering Core Infrastructure
WheelWizard/Features/MiiRendering/Configuration/MiiRenderingConfiguration.cs, WheelWizard/Features/MiiRendering/Configuration/MiiViewMode.cs, WheelWizard/Features/MiiRendering/Domain/IMiiRenderingAssetApi.cs, WheelWizard/Features/MiiRendering/MiiRenderingExtensions.cs
New configuration classes for rendering, lighting profiles, asset API interface for downloading FFL resources, and service extension method to register rendering dependencies with Refit client and HTTP configuration.
Native Interop & Resource Management
WheelWizard/Features/MiiRendering/Services/FflNativeInterop.cs, WheelWizard/Features/MiiRendering/Services/ManagedFflResourceArchive.cs, WheelWizard/Features/MiiRendering/Services/IMiiRenderingResourceLocator.cs, WheelWizard/Features/MiiRendering/Services/MiiRenderingResourceLocator.cs
Dense native interop marshalling structures for FFL rendering, resource archive loader with decompression support, resource locator interface and implementation for validating installed resources.
Rendering Service Interfaces & Installation
WheelWizard/Features/MiiRendering/Services/IMiiNativeRenderer.cs, WheelWizard/Features/MiiRendering/Services/IMiiRenderingResourceInstaller.cs, WheelWizard/Features/MiiRendering/Services/MiiRenderingResourceInstaller.cs
New native renderer interface returning bitmap or pixel buffer, installer interface and concrete implementation with ZIP extraction, validation, progress reporting, and extensive error handling.
Image Specifications & Service Refactor
WheelWizard/Features/MiiImages/Domain/MiiImageSpecifications.cs, WheelWizard/Features/MiiImages/Domain/MiiImageVariants.cs, WheelWizard/Features/MiiImages/MiiImagesExtensions.cs, WheelWizard/Features/MiiImages/MiiImagesSingletonService.cs
Removed IMiiIMagesApi interface; added CameraZoom and RenderScale properties to specifications; updated variants with new MiiListTile and modified expiration settings; refactored singleton service to use native renderer with memory caching, task coalescing, and in-flight request management instead of API calls.
3D Rendering UI Component
WheelWizard/Views/Patterns/MiiImages/Mii3DRender.axaml, WheelWizard/Views/Patterns/MiiImages/Mii3DRender.axaml.cs
New Avalonia control for 3D Mii rendering with interactive rotation/zoom, preview scaling, high-quality settle delay, background render loop, WriteableBitmap surface management, and buffer synchronization.
Legacy Carousel Removal & Replacement
WheelWizard/Views/Patterns/MiiImages/MiiCarousel.axaml, WheelWizard/Views/Patterns/MiiImages/MiiCarousel.axaml.cs, WheelWizard/Views/Patterns/MiiImages/MiiImageLoader.axaml.cs, WheelWizard/Views/Patterns/MiiImages/MiiImageLoaderWithHover.axaml.cs
Removed MiiCarousel component; updated MiiImageLoader to correct property registration and add refresh method; implemented lazy hover variant loading in MiiImageLoaderWithHover with separate primary and hover reload paths.
MiiBlock & Image Variant Updates
WheelWizard/Views/Patterns/MiiBlock.axaml, WheelWizard/Views/Patterns/MiiBlock.axaml.cs
Changed MiiBlock hover variant from MiiBlockProfile to MiiListTile, set expiration to zero, removed immediate reload triggers.
UI Pages & Windows Refactor
WheelWizard/Views/Pages/MiiListPage.axaml, WheelWizard/Views/Pages/MiiListPage.axaml.cs, WheelWizard/Views/Pages/UserProfilePage.axaml
Replaced ScrollViewer grid layout with ListBox-based virtualized list using nested ItemsControl for 4-column layout; introduced MiiListRow and MiiListEntry data model; updated UserProfilePage to use Mii3DRender instead of MiiCarousel.
Editor & Window Updates
WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml.cs, WheelWizard/Views/Popups/MiiManagement/MiiEditorWindow.axaml, WheelWizard/Views/Popups/MiiManagement/MiiEditorWindow.axaml.cs, WheelWizard/Views/Popups/MiiManagement/MiiCarouselWindow.axaml
Removed DispatcherTimer refresh mechanism from editor; replaced MiiCarousel with Mii3DRender in carousel windows; removed LowQualitySpeedup attribute; changed refresh logic to call RefreshCurrentMii() instead of setting Mii property.
Resource Setup & Initialization
WheelWizard/Views/Popups/Generic/MiiRenderingSetupPopup.axaml, WheelWizard/Views/Popups/Generic/MiiRenderingSetupPopup.axaml.cs, WheelWizard/Views/Popups/Base/PopupWindow.axaml.cs, WheelWizard/Views/App.axaml.cs
New popup for guiding resource download and installation with progress reporting; improved PopupWindow to use visible main window detection; added startup resource validation check in App.axaml.cs with error handling and conditional shutdown.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant Installer as ResourceInstaller
    participant API as Asset API
    participant FS as File System
    participant Cache as Resource Cache
    participant Renderer as NativeRenderer

    App->>Installer: DownloadAndInstallAsync()
    activate Installer
    
    Installer->>API: DownloadArchiveAsync()
    activate API
    API-->>Installer: HttpResponseMessage (ZIP)
    deactivate API
    
    Installer->>FS: Validate ZIP header & entries
    Installer->>FS: Extract resource to temp file
    Installer->>FS: Validate file size
    
    Installer->>FS: Move to managed path
    Installer-->>App: OperationResult<string>
    deactivate Installer
    
    App->>Cache: Load resource via Locator
    App->>Renderer: RenderAsync(Mii, specs)
    activate Renderer
    
    Renderer->>Cache: LoadShapePart/LoadTexturePart
    activate Cache
    Cache->>FS: Read & decompress part
    Cache-->>Renderer: byte[] (cached)
    deactivate Cache
    
    Renderer->>Renderer: Native FFLInterop rendering
    Renderer-->>App: Bitmap / PixelBuffer
    deactivate Renderer
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes


Poem

🐰 Whiskers twitching with delight,
No more network calls at night!
Native rendering takes the stage,
3D Miis now turn the page. 🎨
Resources cached, in folders deep,
While carousels take eternal sleep.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is minimal and lacks required sections from the template such as 'Purpose of this PR', 'How to Test', and 'What Has Been Changed'. The template specifies these as expected structure. Add the missing required sections to the description following the template: expand Purpose, provide testing steps, list detailed changes, link related issues, and verify the checklist.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.79% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Offline mii rendering' is concise and directly summarizes the main change: adding offline rendering capabilities for Mii characters.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch Offline-mii-rendering

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@patchzyy
Copy link
Member Author

patchzyy commented Mar 8, 2026

todo: maybe dont make mii files blocking and instead make settings Yellow if you dont have the mii file downloaded.

@patchzyy
Copy link
Member Author

patchzyy commented Mar 8, 2026

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Mar 8, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 17

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
WheelWizard/Views/Patterns/MiiBlock.axaml.cs (1)

50-79: 🧹 Nitpick | 🔵 Trivial

Eliminate duplicate hover variant setup logic.

The hover variant creation logic in OnApplyTemplate (lines 59-64) is duplicated in SetupHoverVariant (lines 72-77). Consider calling SetupHoverVariant() from OnApplyTemplate instead.

♻️ Proposed refactor to eliminate duplication
 protected override void OnApplyTemplate(TemplateAppliedEventArgs e)
 {
     base.OnApplyTemplate(e);

     _miiImageLoader = e.NameScope.Find<MiiImageLoaderWithHover>("PART_MiiImageLoader");

     // Set hover variant immediately
-    if (_miiImageLoader != null)
-    {
-        // Create hover variant with smile expression
-        var hoverVariant = MiiImageVariants.MiiListTile.Clone();
-        hoverVariant.Name = "MiiBlockProfileHover";
-        hoverVariant.Expression = MiiImageSpecifications.FaceExpression.smile;
-        hoverVariant.ExpirationSeconds = TimeSpan.Zero;
-        _miiImageLoader.HoverVariant = hoverVariant;
-    }
+    SetupHoverVariant();
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@WheelWizard/Views/Patterns/MiiBlock.axaml.cs` around lines 50 - 79, Move the
hover-variant creation out of OnApplyTemplate and call the existing
SetupHoverVariant() instead to remove duplicated logic: inside
OnApplyTemplate(TemplateAppliedEventArgs e) after assigning _miiImageLoader use
a single null-check and invoke SetupHoverVariant(); keep SetupHoverVariant()
body as the sole place that constructs the hoverVariant (using
MiiImageVariants.MiiListTile.Clone(), setting Name, Expression,
ExpirationSeconds and assigning to _miiImageLoader.HoverVariant) so no hover
setup remains duplicated in OnApplyTemplate, and ensure _miiImageLoader is the
same field referenced in both methods.
WheelWizard/Features/MiiImages/Domain/MiiImageSpecifications.cs (1)

25-30: ⚠️ Potential issue | 🟠 Major

Make the cache key culture-invariant.

Line 30 appends the new float values with default ToString(), and Vector3.ToString() is culture-sensitive too. The same MiiImageSpecifications will hash to different keys on different locales (1.5 vs 1,5), which breaks cache reuse and can duplicate renders. Serialize numeric components with InvariantCulture and explicit separators instead.

🧩 Suggested fix
+using System.Globalization;
 using System.Numerics;
 using Microsoft.Extensions.Caching.Memory;
@@
-        parts += $"{CharacterRotate}{CameraRotate}{CameraZoom}{RenderScale}";
+        parts += string.Join(
+            "|",
+            CharacterRotate.X.ToString(CultureInfo.InvariantCulture),
+            CharacterRotate.Y.ToString(CultureInfo.InvariantCulture),
+            CharacterRotate.Z.ToString(CultureInfo.InvariantCulture),
+            CameraRotate.X.ToString(CultureInfo.InvariantCulture),
+            CameraRotate.Y.ToString(CultureInfo.InvariantCulture),
+            CameraRotate.Z.ToString(CultureInfo.InvariantCulture),
+            CameraZoom.ToString(CultureInfo.InvariantCulture),
+            RenderScale.ToString(CultureInfo.InvariantCulture)
+        );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@WheelWizard/Features/MiiImages/Domain/MiiImageSpecifications.cs` around lines
25 - 30, The ToString override in MiiImageSpecifications builds a cache key
using default, culture-sensitive ToString() for floats and Vector3s (e.g.,
CharacterRotate, CameraRotate, CameraZoom, RenderScale, BackgroundColor),
causing locale-dependent keys; update the ToString() implementation to format
all numeric values and Vector3 components with CultureInfo.InvariantCulture and
explicit separators (e.g., use ToString("G", CultureInfo.InvariantCulture) for
floats and join Vector3 components with a known delimiter) so the key is
deterministic across locales.
WheelWizard/Views/Pages/MiiListPage.axaml.cs (2)

158-158: ⚠️ Potential issue | 🟡 Minor

Potential IndexOutOfRangeException if no Miis are selected.

GetSelectedMiis() can return an empty array, causing [0] access to throw. The EditMiisButton visibility is conditional (line 513), but a race or binding issue could still invoke this handler with no selection.

🛡️ Proposed defensive fix
-private void EditMii_OnClick(object? sender, RoutedEventArgs e) => EditMii(GetSelectedMiis()[0]);
+private void EditMii_OnClick(object? sender, RoutedEventArgs e)
+{
+    var selected = GetSelectedMiis();
+    if (selected.Length > 0)
+        EditMii(selected[0]);
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@WheelWizard/Views/Pages/MiiListPage.axaml.cs` at line 158, EditMii_OnClick
currently unconditionally calls EditMii(GetSelectedMiis()[0]), which can throw
if GetSelectedMiis() returns an empty array; change EditMii_OnClick to first get
the array from GetSelectedMiis(), check that its Length > 0 (or first element !=
null) and return early (or disable the action) when no selection exists,
otherwise call EditMii with the first selected Mii; reference the existing
methods EditMii_OnClick, GetSelectedMiis, and EditMii when implementing the
guard.

84-118: 🧹 Nitpick | 🔵 Trivial

Variable _isShiftPressed is misleadingly named.

This field is set to true when either Shift or Ctrl is pressed (lines 106, 114), but the name suggests only Shift. Consider renaming to _isMultiSelectModifierPressed or similar for clarity.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@WheelWizard/Views/Pages/MiiListPage.axaml.cs` around lines 84 - 118, The
field _isShiftPressed is misnamed because MiiListPage_KeyDown and
MiiListPage_KeyUp set it for both Shift and Ctrl; rename the field to
_isMultiSelectModifierPressed (update its declaration and any usages) and adjust
related comments in OnAttachedToVisualTree/OnDetachedFromVisualTree and the two
handlers (MiiListPage_KeyDown, MiiListPage_KeyUp) so the name reflects that it
tracks multi-select modifier keys (LeftShift, RightShift, LeftCtrl, RightCtrl);
ensure all references in the class are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@WheelWizard/Features/MiiImages/MiiImagesSingletonService.cs`:
- Around line 147-150: Dispose currently only disposes _imageCache and doesn't
cancel ongoing render tasks; add a CancellationTokenSource field (e.g.,
_disposeCts) to MiiImagesSingletonService, cancel it in Dispose and dispose the
CTS after cancelling, and update the code paths that start render tasks (the
methods that queue or run renders) to accept/observe a CancellationToken from
_disposeCts (or per-request CTS tracked in a concurrent collection) so in-flight
work aborts before accessing _imageCache or other disposed resources; ensure
Dispose waits or cooperatively cancels tasks as appropriate, then dispose
_imageCache and the CTS.

In `@WheelWizard/Features/MiiRendering/Configuration/MiiViewMode.cs`:
- Around line 1-24: The file name MiiViewMode.cs doesn't match the public types
it exports (MiiLightingProfile and MiiLightingProfiles); rename the file to
reflect the symbols (e.g., MiiLightingProfile.cs) or split the record and the
static class into separate files named after each type, and update any
project/CSProj entries or references if required so symbol discovery and
navigation match the filenames.

In `@WheelWizard/Features/MiiRendering/Services/ManagedFflResourceArchive.cs`:
- Around line 255-276: The bare catch in Decompress should be tightened to only
handle expected decompression failures: wrap the call to DecompressWithZlib
inside a try and catch specific exceptions (e.g., System.IO.InvalidDataException
and System.IO.IOException) and in the catch return
DecompressWithGzip(compressed); leave other exceptions to propagate; update the
catch block in Decompress to reference DecompressWithZlib and DecompressWithGzip
accordingly.

In `@WheelWizard/Features/MiiRendering/Services/MiiRenderingResourceInstaller.cs`:
- Around line 85-123: DownloadArchiveAsync currently calls
response.EnsureSuccessStatusCode() which throws HttpRequestException and escapes
the retry loop; modify the method to detect and handle non-success HTTP
responses inside the for loop (either by checking response.IsSuccessStatusCode
before calling EnsureSuccessStatusCode or by catching HttpRequestException
around EnsureSuccessStatusCode) so that HTTP failures increment the attempt and
retry up to MaxDownloadAttempts instead of failing immediately; when an HTTP
error occurs, log a warning including attempt number and the exception/response
status, then continue to the next attempt and only return a final failure after
exhausting attempts.

In `@WheelWizard/Features/MiiRendering/Services/MiiRenderingResourceLocator.cs`:
- Around line 11-29: The cached-path check in GetFflResourcePath should
revalidate the file size before returning _resolvedPath: when _resolvedPath is
non-empty and fileSystem.File.Exists(_resolvedPath) is true, also obtain
fileSystem.FileInfo.New(_resolvedPath).Length and compare against
configuration.MinimumExpectedSizeBytes; if the length is below the minimum
return Fail(...) (same message used for normalized), otherwise proceed to return
_resolvedPath. Keep the later logic for normalized as-is and ensure you still
set _resolvedPath = normalized after passing the same size check.

In `@WheelWizard/Views/App.axaml.cs`:
- Around line 108-113: The startup currently shuts down the entire app when the
MiiRenderingSetupPopup returns false; instead remove the desktop.Shutdown() path
so declining setup does not abort startup, and continue initializing the main
Layout while marking Mii rendering as disabled/degraded; specifically, change
the flow around MiiRenderingSetupPopup and its ShowAndAwaitCompletionAsync
result so that when it returns false you do not call desktop.Shutdown(), but
propagate the flag into Layout (or set a MiiRenderer.IsEnabled/IsAvailable flag)
so the rest of the app starts and Mii-dependent features can detect and operate
in a degraded mode.
- Around line 105-115: After the setup popup returns true you must re-check that
the resources were actually installed before proceeding to Layout; call
IMiiRenderingResourceInstaller.GetResolvedResourcePath() again after awaiting
MiiRenderingSetupPopup.ShowAndAwaitCompletionAsync() and if it still indicates
failure handle it the same way (e.g., show error/shutdown) instead of continuing
to Layout initialization. This adds an explicit defensive verification after
ShowAndAwaitCompletionAsync() and references GetResolvedResourcePath,
MiiRenderingSetupPopup.ShowAndAwaitCompletionAsync, and the Layout
initialization path.

In `@WheelWizard/Views/Pages/MiiListPage.axaml`:
- Line 117: Replace the hard-coded MenuItem Header="Duplicate" in the MenuItem
element with the localized resource used by the other context-menu entries (use
the same lang:Common resource pattern and the appropriate key, e.g., the
lang:Common key for "Duplicate"), so the label is pulled from localization
resources rather than being a literal string.

In `@WheelWizard/Views/Pages/MiiListPage.axaml.cs`:
- Around line 224-231: The current non-shift selection block preserves
entry.IsSelected so clicking an unselected item does nothing; change the logic
so when neither Shift nor Ctrl modifiers are active you clear all selections on
_miiEntries and then set the clicked entry's IsSelected to true (use
_isShiftPressed/_isCtrlPressed check as in the snippet and assign
entry.IsSelected = true after clearing) so a plain click selects the clicked
item.

In `@WheelWizard/Views/Patterns/MiiImages/Mii3DRender.axaml.cs`:
- Around line 229-274: RenderWorkerLoopAsync can exit via an exception and never
reset _renderWorkerRunning, preventing future workers from starting; modify the
method so that any exit path (normal return or exception) clears
_renderWorkerRunning under _renderLock — e.g., wrap the entire worker loop body
in a try/finally (or add an outer try/finally) and in the finally acquire
_renderLock and set _renderWorkerRunning = false (also ensure _inFlightRenderCts
is left consistent if needed); keep existing cleanup for render.Cancellation and
maintain use of Dispatcher.UIThread.InvokeAsync/PresentBuffer but guarantee the
flag is always cleared so QueueRenderCurrentView can start a new worker.

In `@WheelWizard/Views/Patterns/MiiImages/MiiImageLoader.axaml.cs`:
- Around line 87-95: The low-quality branch currently runs unconditionally when
LowQualitySpeedup is true, causing tiny/small variants to get inappropriate
placeholders; modify the logic around LowQualitySpeedup +
GeneratedImages/GetLowQualityClone to first check the specifications' size and
skip low-quality for tiny and small sizes (use a helper like
ShouldUseLowQualityPlaceholder(MiiImageSpecifications) that returns
specifications.Size != ImageSize.tiny && specifications.Size !=
ImageSize.small), and apply this guard in both places where GetLowQualityClone
is added (the block that nulls GeneratedImages and the similar block at lines
~105-113) so low-quality placeholders are only used for larger sizes.

In `@WheelWizard/Views/Patterns/MiiImages/MiiImageLoaderWithHover.axaml.cs`:
- Around line 193-194: Currently _hasLoadedHoverVariant is set to true before
calling ReloadImages(Mii, variants), so failures/cancellations leave the flag
true and prevent retries; change the flow so ReloadImages(Mii, variants) is
awaited and only set _hasLoadedHoverVariant = true after confirming the hover
variant loaded successfully (or return a success boolean from ReloadImages and
set the flag based on that), and ensure any exceptions/cancellation reset
_hasLoadedHoverVariant (or leave it false) so subsequent hover events can retry
loading HoverVariant for the given Mii/variants.
- Around line 118-125: CoerceHoverVariant currently resets
_hasLoadedHoverVariant and calls ReloadPrimaryVariant but misses loading the new
hover image when the control is currently hovered; update CoerceHoverVariant in
MiiImageLoaderWithHover to, after setting _hasLoadedHoverVariant = false and
calling ReloadPrimaryVariant(), check the IsHovered property and if true call
TryLoadHoverVariant() so the new HoverVariant is loaded immediately while
hovered.
- Around line 161-175: Both OnVariantChanged and OnMiiChanged reset
_hasLoadedHoverVariant then unconditionally call ReloadPrimaryVariant which
starts an async load that may immediately be cancelled if IsHovered is true;
change both methods to check IsHovered first: set _hasLoadedHoverVariant =
false, then if IsHovered call TryLoadHoverVariant (or ReloadBothVariants)
instead of ReloadPrimaryVariant, else call ReloadPrimaryVariant; keep the same
semantics for non-hovered cases and apply the identical change to both
OnVariantChanged and OnMiiChanged to avoid redundant async reloads.

In `@WheelWizard/Views/Popups/Base/PopupWindow.axaml.cs`:
- Around line 102-110: The popup is being pinned to the owner's top-left by
copying mainWindow.Position; instead, after calling TryGetVisibleMainWindow()
and setting Owner = mainWindow (in the same block where TryGetVisibleMainWindow
and Owner are used), remove the Position assignment and set
WindowStartupLocation = WindowStartupLocation.CenterOwner so Avalonia will
center the popup relative to the owner; adjust the block around
TryGetVisibleMainWindow/Owner/Position to use WindowStartupLocation.CenterOwner
instead of assigning Position.

In `@WheelWizard/Views/Popups/Generic/MiiRenderingSetupPopup.axaml.cs`:
- Around line 26-29: The popup currently sets PathTextBlock.Text from
PathManager.MiiRenderingResourceFilePath which can be wrong when the installer
overrides the path; change it to read from
MiiRenderingConfiguration.ManagedResourcePath (or from the installer/config
object that exposes ManagedResourcePath) instead. Update each place where
PathTextBlock.Text (and similar Status/Progress initialization lines around the
constructor or Init method) uses PathManager.MiiRenderingResourceFilePath to use
MiiRenderingConfiguration.ManagedResourcePath so the UI reflects the
installer-provided path.

In `@WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml.cs`:
- Line 114: The ValueChanged handlers Length_OnValueChanged and
Width_OnValueChanged are calling Editor.RefreshImage() on every slider event
causing repeated expensive renders; change this by debouncing or deferring the
refresh: stop calling Editor.RefreshImage() directly from those handlers and
instead either (a) implement a short debounce timer (e.g., reset/start a single
DispatcherTimer on each ValueChanged and call Editor.RefreshImage() only when it
elapses) or (b) remove the refresh from ValueChanged and wire the Slider
PointerReleased (or Thumb.DragCompleted) event to call Editor.RefreshImage()
once when the user finishes dragging; reference Editor.RefreshImage(),
Mii3DRenderControl.RefreshCurrentMii(), and MiiFaceImage.RefreshCurrentMii()
when updating render calls so only a single refresh runs after the drag ends.

---

Outside diff comments:
In `@WheelWizard/Features/MiiImages/Domain/MiiImageSpecifications.cs`:
- Around line 25-30: The ToString override in MiiImageSpecifications builds a
cache key using default, culture-sensitive ToString() for floats and Vector3s
(e.g., CharacterRotate, CameraRotate, CameraZoom, RenderScale, BackgroundColor),
causing locale-dependent keys; update the ToString() implementation to format
all numeric values and Vector3 components with CultureInfo.InvariantCulture and
explicit separators (e.g., use ToString("G", CultureInfo.InvariantCulture) for
floats and join Vector3 components with a known delimiter) so the key is
deterministic across locales.

In `@WheelWizard/Views/Pages/MiiListPage.axaml.cs`:
- Line 158: EditMii_OnClick currently unconditionally calls
EditMii(GetSelectedMiis()[0]), which can throw if GetSelectedMiis() returns an
empty array; change EditMii_OnClick to first get the array from
GetSelectedMiis(), check that its Length > 0 (or first element != null) and
return early (or disable the action) when no selection exists, otherwise call
EditMii with the first selected Mii; reference the existing methods
EditMii_OnClick, GetSelectedMiis, and EditMii when implementing the guard.
- Around line 84-118: The field _isShiftPressed is misnamed because
MiiListPage_KeyDown and MiiListPage_KeyUp set it for both Shift and Ctrl; rename
the field to _isMultiSelectModifierPressed (update its declaration and any
usages) and adjust related comments in
OnAttachedToVisualTree/OnDetachedFromVisualTree and the two handlers
(MiiListPage_KeyDown, MiiListPage_KeyUp) so the name reflects that it tracks
multi-select modifier keys (LeftShift, RightShift, LeftCtrl, RightCtrl); ensure
all references in the class are updated accordingly.

In `@WheelWizard/Views/Patterns/MiiBlock.axaml.cs`:
- Around line 50-79: Move the hover-variant creation out of OnApplyTemplate and
call the existing SetupHoverVariant() instead to remove duplicated logic: inside
OnApplyTemplate(TemplateAppliedEventArgs e) after assigning _miiImageLoader use
a single null-check and invoke SetupHoverVariant(); keep SetupHoverVariant()
body as the sole place that constructs the hoverVariant (using
MiiImageVariants.MiiListTile.Clone(), setting Name, Expression,
ExpirationSeconds and assigning to _miiImageLoader.HoverVariant) so no hover
setup remains duplicated in OnApplyTemplate, and ensure _miiImageLoader is the
same field referenced in both methods.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 49ee10e5-09cb-4a8b-a731-6edc4d7154b7

📥 Commits

Reviewing files that changed from the base of the PR and between da6ee99 and 01f4b54.

📒 Files selected for processing (42)
  • WheelWizard.Test/WheelWizard.Test.csproj
  • WheelWizard/Features/MiiImages/Domain/IMiiIMagesApi.cs
  • WheelWizard/Features/MiiImages/Domain/MiiImageSpecifications.cs
  • WheelWizard/Features/MiiImages/Domain/MiiImageVariants.cs
  • WheelWizard/Features/MiiImages/MiiImagesExtensions.cs
  • WheelWizard/Features/MiiImages/MiiImagesSingletonService.cs
  • WheelWizard/Features/MiiRendering/Configuration/MiiRenderingConfiguration.cs
  • WheelWizard/Features/MiiRendering/Configuration/MiiViewMode.cs
  • WheelWizard/Features/MiiRendering/Domain/IMiiRenderingAssetApi.cs
  • WheelWizard/Features/MiiRendering/MiiRenderingExtensions.cs
  • WheelWizard/Features/MiiRendering/Resources/mii_static_body_3ds_female_LE.rmdl
  • WheelWizard/Features/MiiRendering/Resources/mii_static_body_3ds_male_LE.rmdl
  • WheelWizard/Features/MiiRendering/Services/FflNativeInterop.cs
  • WheelWizard/Features/MiiRendering/Services/IMiiNativeRenderer.cs
  • WheelWizard/Features/MiiRendering/Services/IMiiRenderingResourceInstaller.cs
  • WheelWizard/Features/MiiRendering/Services/IMiiRenderingResourceLocator.cs
  • WheelWizard/Features/MiiRendering/Services/ManagedFflResourceArchive.cs
  • WheelWizard/Features/MiiRendering/Services/MiiRenderingResourceInstaller.cs
  • WheelWizard/Features/MiiRendering/Services/MiiRenderingResourceLocator.cs
  • WheelWizard/Features/MiiRendering/Services/NativeMiiRenderer.cs
  • WheelWizard/Services/Endpoints.cs
  • WheelWizard/Services/PathManager.cs
  • WheelWizard/Views/App.axaml.cs
  • WheelWizard/Views/Pages/MiiListPage.axaml
  • WheelWizard/Views/Pages/MiiListPage.axaml.cs
  • WheelWizard/Views/Pages/UserProfilePage.axaml
  • WheelWizard/Views/Patterns/MiiBlock.axaml
  • WheelWizard/Views/Patterns/MiiBlock.axaml.cs
  • WheelWizard/Views/Patterns/MiiImages/Mii3DRender.axaml
  • WheelWizard/Views/Patterns/MiiImages/Mii3DRender.axaml.cs
  • WheelWizard/Views/Patterns/MiiImages/MiiCarousel.axaml
  • WheelWizard/Views/Patterns/MiiImages/MiiCarousel.axaml.cs
  • WheelWizard/Views/Patterns/MiiImages/MiiImageLoader.axaml.cs
  • WheelWizard/Views/Patterns/MiiImages/MiiImageLoaderWithHover.axaml.cs
  • WheelWizard/Views/Popups/Base/PopupWindow.axaml.cs
  • WheelWizard/Views/Popups/Generic/MiiRenderingSetupPopup.axaml
  • WheelWizard/Views/Popups/Generic/MiiRenderingSetupPopup.axaml.cs
  • WheelWizard/Views/Popups/MiiManagement/MiiCarouselWindow.axaml
  • WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml.cs
  • WheelWizard/Views/Popups/MiiManagement/MiiEditorWindow.axaml
  • WheelWizard/Views/Popups/MiiManagement/MiiEditorWindow.axaml.cs
  • WheelWizard/WheelWizard.csproj
💤 Files with no reviewable changes (3)
  • WheelWizard/Features/MiiImages/Domain/IMiiIMagesApi.cs
  • WheelWizard/Views/Patterns/MiiImages/MiiCarousel.axaml.cs
  • WheelWizard/Views/Patterns/MiiImages/MiiCarousel.axaml

Comment on lines +255 to +276
private static byte[] Decompress(byte[] compressed, byte windowBits)
{
if (windowBits <= 7)
return DecompressWithZlib(compressed);

if (windowBits is >= 8 and <= 15)
return DecompressWithGzip(compressed);

if (windowBits == 16)
{
try
{
return DecompressWithZlib(compressed);
}
catch
{
return DecompressWithGzip(compressed);
}
}

return DecompressWithZlib(compressed);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Bare catch clause swallows all exceptions including fatal ones.

The catch-all at line 269 can swallow critical exceptions like OutOfMemoryException or StackOverflowException. Catch only expected decompression exceptions.

🛡️ Proposed fix to catch specific exceptions
     if (windowBits == 16)
     {
         try
         {
             return DecompressWithZlib(compressed);
         }
-        catch
+        catch (InvalidDataException)
         {
             return DecompressWithGzip(compressed);
         }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private static byte[] Decompress(byte[] compressed, byte windowBits)
{
if (windowBits <= 7)
return DecompressWithZlib(compressed);
if (windowBits is >= 8 and <= 15)
return DecompressWithGzip(compressed);
if (windowBits == 16)
{
try
{
return DecompressWithZlib(compressed);
}
catch
{
return DecompressWithGzip(compressed);
}
}
return DecompressWithZlib(compressed);
}
private static byte[] Decompress(byte[] compressed, byte windowBits)
{
if (windowBits <= 7)
return DecompressWithZlib(compressed);
if (windowBits is >= 8 and <= 15)
return DecompressWithGzip(compressed);
if (windowBits == 16)
{
try
{
return DecompressWithZlib(compressed);
}
catch (InvalidDataException)
{
return DecompressWithGzip(compressed);
}
}
return DecompressWithZlib(compressed);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@WheelWizard/Features/MiiRendering/Services/ManagedFflResourceArchive.cs`
around lines 255 - 276, The bare catch in Decompress should be tightened to only
handle expected decompression failures: wrap the call to DecompressWithZlib
inside a try and catch specific exceptions (e.g., System.IO.InvalidDataException
and System.IO.IOException) and in the catch return
DecompressWithGzip(compressed); leave other exceptions to propagate; update the
catch block in Decompress to reference DecompressWithZlib and DecompressWithGzip
accordingly.

Comment on lines +85 to +123
private async Task<OperationResult<byte[]>> DownloadArchiveAsync(
IProgress<MiiRenderingInstallerProgress>? progress,
CancellationToken cancellationToken
)
{
for (var attempt = 1; attempt <= MaxDownloadAttempts; attempt++)
{
progress?.Report(new($"Downloading archive (attempt {attempt}/{MaxDownloadAttempts})", 0, null));

using var response = await assetApi.DownloadArchiveAsync(cancellationToken);
response.EnsureSuccessStatusCode();

var totalBytes = response.Content.Headers.ContentLength;
await using var responseStream = await response.Content.ReadAsStreamAsync(cancellationToken);
await using var bufferStream = new MemoryStream(totalBytes is > 0 and <= int.MaxValue ? (int)totalBytes.Value : 0);

var bytesDownloaded = await CopyWithProgressAsync(
responseStream,
bufferStream,
totalBytes,
progress,
cancellationToken,
$"Downloading archive (attempt {attempt}/{MaxDownloadAttempts})"
);

var archiveBytes = bufferStream.ToArray();
var validationResult = ValidateArchiveBytes(archiveBytes, totalBytes, bytesDownloaded, attempt);
if (validationResult.IsSuccess)
return archiveBytes;

logger.LogWarning(
"Downloaded Mii rendering archive was invalid on attempt {Attempt}: {Message}",
attempt,
validationResult.Error?.Message
);
}

return Fail("Failed to download a valid Mii rendering archive after multiple attempts.");
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

HTTP failures bypass retry logic.

EnsureSuccessStatusCode() throws HttpRequestException on non-success HTTP status codes, but this exception is not caught inside the retry loop. Only validation failures trigger retries; network or server errors will immediately fail the entire operation.

🐛 Proposed fix to handle HTTP failures in retry loop
     private async Task<OperationResult<byte[]>> DownloadArchiveAsync(
         IProgress<MiiRenderingInstallerProgress>? progress,
         CancellationToken cancellationToken
     )
     {
         for (var attempt = 1; attempt <= MaxDownloadAttempts; attempt++)
         {
             progress?.Report(new($"Downloading archive (attempt {attempt}/{MaxDownloadAttempts})", 0, null));

-            using var response = await assetApi.DownloadArchiveAsync(cancellationToken);
-            response.EnsureSuccessStatusCode();
+            HttpResponseMessage response;
+            try
+            {
+                response = await assetApi.DownloadArchiveAsync(cancellationToken);
+                response.EnsureSuccessStatusCode();
+            }
+            catch (HttpRequestException ex) when (attempt < MaxDownloadAttempts)
+            {
+                logger.LogWarning(ex, "Download attempt {Attempt} failed with HTTP error", attempt);
+                await Task.Delay(TimeSpan.FromSeconds(attempt), cancellationToken); // Exponential backoff
+                continue;
+            }

+            using (response)
+            {
             var totalBytes = response.Content.Headers.ContentLength;
             // ... rest of the download logic
+            }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@WheelWizard/Features/MiiRendering/Services/MiiRenderingResourceInstaller.cs`
around lines 85 - 123, DownloadArchiveAsync currently calls
response.EnsureSuccessStatusCode() which throws HttpRequestException and escapes
the retry loop; modify the method to detect and handle non-success HTTP
responses inside the for loop (either by checking response.IsSuccessStatusCode
before calling EnsureSuccessStatusCode or by catching HttpRequestException
around EnsureSuccessStatusCode) so that HTTP failures increment the attempt and
retry up to MaxDownloadAttempts instead of failing immediately; when an HTTP
error occurs, log a warning including attempt number and the exception/response
status, then continue to the next attempt and only return a final failure after
exhausting attempts.

Comment on lines +11 to +29
public OperationResult<string> GetFflResourcePath()
{
if (!string.IsNullOrWhiteSpace(_resolvedPath) && fileSystem.File.Exists(_resolvedPath))
return _resolvedPath;

var normalized = NormalizeCandidate(configuration.ManagedResourcePath);
if (!string.IsNullOrWhiteSpace(normalized) && fileSystem.File.Exists(normalized))
{
var length = fileSystem.FileInfo.New(normalized).Length;
if (length < configuration.MinimumExpectedSizeBytes)
{
return Fail(
$"Found {MiiRenderingConfiguration.ResourceFileName} at '{normalized}', but it is only {length} bytes. "
+ "This file appears invalid or incomplete."
);
}

_resolvedPath = normalized;
return normalized;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Revalidate _resolvedPath before returning it.

Lines 13-14 only check that the cached file still exists. If that file is later truncated or replaced in-process, every call from NativeMiiRenderer will skip the size guard and keep using a resource this method would otherwise reject.

🧪 Proposed fix
-        if (!string.IsNullOrWhiteSpace(_resolvedPath) && fileSystem.File.Exists(_resolvedPath))
-            return _resolvedPath;
+        if (!string.IsNullOrWhiteSpace(_resolvedPath) && fileSystem.File.Exists(_resolvedPath))
+        {
+            var cachedLength = fileSystem.FileInfo.New(_resolvedPath).Length;
+            if (cachedLength >= configuration.MinimumExpectedSizeBytes)
+                return _resolvedPath;
+
+            _resolvedPath = null;
+        }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@WheelWizard/Features/MiiRendering/Services/MiiRenderingResourceLocator.cs`
around lines 11 - 29, The cached-path check in GetFflResourcePath should
revalidate the file size before returning _resolvedPath: when _resolvedPath is
non-empty and fileSystem.File.Exists(_resolvedPath) is true, also obtain
fileSystem.FileInfo.New(_resolvedPath).Length and compare against
configuration.MinimumExpectedSizeBytes; if the length is below the minimum
return Fail(...) (same message used for normalized), otherwise proceed to return
_resolvedPath. Keep the later logic for normalized as-is and ensure you still
set _resolvedPath = normalized after passing the same size check.

Comment on lines 161 to 175
private void OnVariantChanged(MiiImageSpecifications newSpecifications)
{
ReloadBothVariants();
_hasLoadedHoverVariant = false;
ReloadPrimaryVariant();
if (IsHovered)
TryLoadHoverVariant();
}

protected override void OnMiiChanged(Mii? newMii)
{
// Always reload both variants when Mii changes
// This ensures both images are loaded even if hover variant is set later
ReloadBothVariants();
_hasLoadedHoverVariant = false;
ReloadPrimaryVariant();
if (IsHovered)
TryLoadHoverVariant();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Redundant reload when hovered.

When IsHovered is true, ReloadPrimaryVariant() starts an async load that is immediately cancelled by ReloadBothVariants() (called via TryLoadHoverVariant()). Consider checking IsHovered first to avoid the wasted call.

♻️ Proposed refactor to avoid redundant reload
 private void OnVariantChanged(MiiImageSpecifications newSpecifications)
 {
     _hasLoadedHoverVariant = false;
-    ReloadPrimaryVariant();
-    if (IsHovered)
-        TryLoadHoverVariant();
+    if (IsHovered && HoverVariant != null)
+        ReloadBothVariants();
+    else
+        ReloadPrimaryVariant();
 }

 protected override void OnMiiChanged(Mii? newMii)
 {
     _hasLoadedHoverVariant = false;
-    ReloadPrimaryVariant();
-    if (IsHovered)
-        TryLoadHoverVariant();
+    if (IsHovered && HoverVariant != null)
+        ReloadBothVariants();
+    else
+        ReloadPrimaryVariant();
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@WheelWizard/Views/Patterns/MiiImages/MiiImageLoaderWithHover.axaml.cs` around
lines 161 - 175, Both OnVariantChanged and OnMiiChanged reset
_hasLoadedHoverVariant then unconditionally call ReloadPrimaryVariant which
starts an async load that may immediately be cancelled if IsHovered is true;
change both methods to check IsHovered first: set _hasLoadedHoverVariant =
false, then if IsHovered call TryLoadHoverVariant (or ReloadBothVariants)
instead of ReloadPrimaryVariant, else call ReloadPrimaryVariant; keep the same
semantics for non-hovered cases and apply the identical change to both
OnVariantChanged and OnMiiChanged to avoid redundant async reloads.

Comment on lines +193 to 194
_hasLoadedHoverVariant = HoverVariant != null;
ReloadImages(Mii, variants);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Flag set before async reload completes; failed loads won't be retried.

_hasLoadedHoverVariant is set before ReloadImages completes. If the async operation fails or is cancelled, the flag remains true and subsequent hover events won't retry loading the hover variant. Consider setting the flag after confirming the load succeeded, or adding a retry mechanism.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@WheelWizard/Views/Patterns/MiiImages/MiiImageLoaderWithHover.axaml.cs` around
lines 193 - 194, Currently _hasLoadedHoverVariant is set to true before calling
ReloadImages(Mii, variants), so failures/cancellations leave the flag true and
prevent retries; change the flow so ReloadImages(Mii, variants) is awaited and
only set _hasLoadedHoverVariant = true after confirming the hover variant loaded
successfully (or return a success boolean from ReloadImages and set the flag
based on that), and ensure any exceptions/cancellation reset
_hasLoadedHoverVariant (or leave it false) so subsequent hover events can retry
loading HoverVariant for the given Mii/variants.

Comment on lines +102 to +110
var mainWindow = TryGetVisibleMainWindow();
if (mainWindow != null)
{
Owner = mainWindow;
Position = mainWindow.Position;
}
else
{
WindowStartupLocation = WindowStartupLocation.CenterScreen;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

Avalonia 11.2.5 WindowStartupLocation.CenterOwner: for an owned Window, is it supported and recommended over manually assigning Position = owner.Position before showing the popup?

💡 Result:

Yes—WindowStartupLocation.CenterOwner is supported for an owned Window in Avalonia 11.2.5, and it’s generally the preferred way to “center relative to the owner” compared to manually copying positions.

  • CenterOwner is explicitly defined as “center of the owner window”, and if no owner is specified it falls back to Manual. (api-docs.avaloniaui.net)
  • Passing an owner via ShowDialog(owner) is the standard way to establish ownership for dialogs. (docs.avaloniaui.net)

Why it’s usually better than Position = owner.Position:

  • owner.Position aligns the top-left corners; it does not center (you’d have to compute offsets using sizes, which may not be final until layout happens).
  • CenterOwner lets Avalonia/platform code do the right thing with the actual window bounds (including decorations) instead of you approximating.
  • Manual positioning + ownership can run into platform/DPI edge cases; for example, Avalonia has reported issues around owned windows and positioning/scaling in mixed-DPI setups. (github.com)

Typical usage:

var dialog = new MyDialog
{
    WindowStartupLocation = WindowStartupLocation.CenterOwner,
};
await dialog.ShowDialog(this); // owner

If you need a very specific placement (not centered, or offset from owner, or on a different monitor than the owner), then manual Position is appropriate—but for “center on owner”, CenterOwner is the intended option.

Citations:


Center owned popups instead of pinning them to the main window's top-left.

Line 106 places the popup at the owner's exact Position, which aligns only the top-left corners and doesn't account for the popup's size or final scaled bounds. After setting Owner, use WindowStartupLocation.CenterOwner to let Avalonia properly center the dialog.

Proposed fix
         if (mainWindow != null)
         {
             Owner = mainWindow;
-            Position = mainWindow.Position;
+            WindowStartupLocation = WindowStartupLocation.CenterOwner;
         }
         else
         {
             WindowStartupLocation = WindowStartupLocation.CenterScreen;
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var mainWindow = TryGetVisibleMainWindow();
if (mainWindow != null)
{
Owner = mainWindow;
Position = mainWindow.Position;
}
else
{
WindowStartupLocation = WindowStartupLocation.CenterScreen;
var mainWindow = TryGetVisibleMainWindow();
if (mainWindow != null)
{
Owner = mainWindow;
WindowStartupLocation = WindowStartupLocation.CenterOwner;
}
else
{
WindowStartupLocation = WindowStartupLocation.CenterScreen;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@WheelWizard/Views/Popups/Base/PopupWindow.axaml.cs` around lines 102 - 110,
The popup is being pinned to the owner's top-left by copying
mainWindow.Position; instead, after calling TryGetVisibleMainWindow() and
setting Owner = mainWindow (in the same block where TryGetVisibleMainWindow and
Owner are used), remove the Position assignment and set WindowStartupLocation =
WindowStartupLocation.CenterOwner so Avalonia will center the popup relative to
the owner; adjust the block around TryGetVisibleMainWindow/Owner/Position to use
WindowStartupLocation.CenterOwner instead of assigning Position.

Comment on lines +26 to +29
PathTextBlock.Text = PathManager.MiiRenderingResourceFilePath;
StatusTextBlock.Text = "Download the Mii rendering resource to continue.";
ProgressTextBlock.Text = "Ready to install.";
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Read the install path from the installer, not PathManager.

Line 26 hard-codes the default path, so this popup can point users at the wrong location whenever MiiRenderingConfiguration.ManagedResourcePath is overridden.

🔧 Proposed fix
-        PathTextBlock.Text = PathManager.MiiRenderingResourceFilePath;
         StatusTextBlock.Text = "Download the Mii rendering resource to continue.";
         ProgressTextBlock.Text = "Ready to install.";
@@
     public Task<bool> ShowAndAwaitCompletionAsync()
     {
+        PathTextBlock.Text = ResourceInstaller.ManagedResourcePath;
         Show();
         return _completionSource.Task;
     }

Also applies to: 31-34

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@WheelWizard/Views/Popups/Generic/MiiRenderingSetupPopup.axaml.cs` around
lines 26 - 29, The popup currently sets PathTextBlock.Text from
PathManager.MiiRenderingResourceFilePath which can be wrong when the installer
overrides the path; change it to read from
MiiRenderingConfiguration.ManagedResourcePath (or from the installer/config
object that exposes ManagedResourcePath) instead. Update each place where
PathTextBlock.Text (and similar Status/Progress initialization lines around the
constructor or Init method) uses PathManager.MiiRenderingResourceFilePath to use
MiiRenderingConfiguration.ManagedResourcePath so the UI reflects the
installer-provided path.

}
Editor.Mii.Height = heightResult.Value;
RestartRefreshTimer();
Editor.RefreshImage();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the EditorGeneral.axaml.cs file around lines 114 and 131
cat -n WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml.cs | sed -n '100,140p'

Repository: TeamWheelWizard/WheelWizard

Length of output: 1729


🏁 Script executed:

# Let's see more context to understand the full methods
cat -n WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml.cs | sed -n '105,145p'

Repository: TeamWheelWizard/WheelWizard

Length of output: 1830


🏁 Script executed:

# Let's examine the event handler method definitions to understand which events trigger the refresh
grep -n "ValueChanged\|Length_OnValueChanged\|Width_OnValueChanged" WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml.cs

Repository: TeamWheelWizard/WheelWizard

Length of output: 255


🏁 Script executed:

# Check the MiiEditorWindow.axaml.cs to verify the RefreshImage method flow
cat -n WheelWizard/Views/Popups/MiiManagement/MiiEditorWindow.axaml.cs | sed -n '90,110p'

Repository: TeamWheelWizard/WheelWizard

Length of output: 858


🏁 Script executed:

# Look for any debounce/throttle mechanisms that might have been removed
grep -n "debounce\|throttle\|Throttle\|Debounce\|DispatcherTimer\|Delay" WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml.cs

Repository: TeamWheelWizard/WheelWizard

Length of output: 53


🏁 Script executed:

# Search for any similar patterns in related files
find WheelWizard/Views/Popups/MiiManagement -name "*.cs" -type f

Repository: TeamWheelWizard/WheelWizard

Length of output: 1188


🏁 Script executed:

# Check the XAML file to see if there are any binding triggers or other mitigation
cat -n WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml | head -100

Repository: TeamWheelWizard/WheelWizard

Length of output: 5901


🏁 Script executed:

# Check if other editor pages have debounce patterns
grep -r "DispatcherTimer\|debounce\|Throttle" WheelWizard/Views/Popups/MiiManagement/MiiEditor/ --include="*.cs"

Repository: TeamWheelWizard/WheelWizard

Length of output: 53


🏁 Script executed:

# Look at the full EditorGeneral.axaml.cs to see the complete context
wc -l WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml.cs

Repository: TeamWheelWizard/WheelWizard

Length of output: 146


🏁 Script executed:

# Check one of the other editor files to see if they handle ValueChanged differently
grep -A 10 "ValueChanged" WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorEyes.axaml.cs | head -30

Repository: TeamWheelWizard/WheelWizard

Length of output: 53


🏁 Script executed:

# Check if RefreshCurrentMii methods have any built-in throttling or debouncing
grep -A 15 "RefreshCurrentMii" WheelWizard/Views/Popups/MiiManagement/MiiEditorWindow.axaml.cs

Repository: TeamWheelWizard/WheelWizard

Length of output: 550


🏁 Script executed:

# Search for Mii3DRenderControl and MiiFaceImage implementations
find WheelWizard -name "Mii3DRenderControl*" -o -name "MiiFaceImage*"

Repository: TeamWheelWizard/WheelWizard

Length of output: 53


🏁 Script executed:

# Check if these render control files have any debouncing or throttling
grep -l "DispatcherTimer\|debounce\|Throttle\|throttle" WheelWizard/Views/**/*.cs 2>/dev/null | head -10

Repository: TeamWheelWizard/WheelWizard

Length of output: 53


🏁 Script executed:

# Check the entire EditorGeneral.cs to see the full context
cat -n WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml.cs | tail -60

Repository: TeamWheelWizard/WheelWizard

Length of output: 2515


Debounce slider-driven refreshes to avoid repeated renders during drag interactions.

The Length_OnValueChanged and Width_OnValueChanged handlers currently invoke Editor.RefreshImage() on every ValueChanged event. Since sliders fire this event repeatedly while dragging, this causes numerous direct calls to expensive native rendering methods (Mii3DRenderControl.RefreshCurrentMii() or MiiFaceImage.RefreshCurrentMii()), potentially causing UI stutter or dropped frames. Consider adding a debounce or throttle mechanism, or refresh only when the drag completes (e.g., using PointerReleased instead).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml.cs` at
line 114, The ValueChanged handlers Length_OnValueChanged and
Width_OnValueChanged are calling Editor.RefreshImage() on every slider event
causing repeated expensive renders; change this by debouncing or deferring the
refresh: stop calling Editor.RefreshImage() directly from those handlers and
instead either (a) implement a short debounce timer (e.g., reset/start a single
DispatcherTimer on each ValueChanged and call Editor.RefreshImage() only when it
elapses) or (b) remove the refresh from ValueChanged and wire the Slider
PointerReleased (or Thumb.DragCompleted) event to call Editor.RefreshImage()
once when the user finishes dragging; reference Editor.RefreshImage(),
Mii3DRenderControl.RefreshCurrentMii(), and MiiFaceImage.RefreshCurrentMii()
when updating render calls so only a single refresh runs after the drag ends.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant