Make settings values type safe and centralize path reloads by DirkDoes · Pull Request #499 · TeamWheelWizard/WheelWizard

DirkDoes · 2026-09-28T21:03:11Z

Purpose of this PR:

Replace object-based setting access with Setting and typed manager methods. Incorrect value types are rejected by the compiler. Runtime parsing stays at JSON/INI/TOML boundaries, with domain validation retained.

Reload Dolphin settings centrally when its executable/user path changes. Settings UI edits surface persistence failures. Persisted property names and representations stay unchanged.

Based on #498. Merge after #498; works without later layers.

How to Test:

dotnet test WheelWizard.sln

551 tests passed (532 unit, 19 headless UI), including the typed API contract and the existing settings/path suites.

What Has Been Changed:

See the focused implementation above. The existing settings JSON contract and imported translation sheets are preserved.

Checklist before merging

You have created relevant tests

Summary by CodeRabbit

New Features
- Decimal values are now supported in Dolphin and Wheel Wizard settings.
- Settings changes that fail to save display the relevant error message when available, with a fallback warning.
Bug Fixes
- Improved reliability when updating settings, including handling save failures and restoring values when updates cannot be applied.
- Window layout scaling now uses the current configured scale consistently.

coderabbitai · 2026-09-28T21:03:17Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: Advanced
Run ID: 43240b35-3d2c-4337-96fd-354761b82637

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 7d4fcf07-0d9e-477e-a518-7485c9f0f7d8

📥 Commits

Reviewing files that changed from the base of the PR and between 65e43f8 and 6f1a537.

📒 Files selected for processing (29)

WheelWizard.Test/Features/Dolphin/FeaturePathOwnershipTests.cs
WheelWizard.Test/Features/Launching/DolphinLaunchServiceTests.cs
WheelWizard.Test/Features/Launching/LaunchPathTests.cs
WheelWizard.Test/Features/Launching/RetroRewindLaunchServiceTests.cs
WheelWizard.Test/Features/MiiRepositoryServiceTests.cs
WheelWizard.Test/Features/Settings/DolphinSettingsTests.cs
WheelWizard.Test/Features/Settings/SettingsPersistencePathTests.cs
WheelWizard.Test/Features/Settings/SettingsRecoveryTests.cs
WheelWizard.Test/Features/Settings/SettingsTests.cs
WheelWizard.Test/Features/Settings/VirtualSettingsTests.cs
WheelWizard.Test/Features/Settings/WhWzSettingsTests.cs
WheelWizard.Test/Views/HomeDolphinLaunchTests.cs
WheelWizard/Features/Settings/DolphinSettingManager.cs
WheelWizard/Features/Settings/ISettingsServices.cs
WheelWizard/Features/Settings/RecompSettingManager.cs
WheelWizard/Features/Settings/SettingsManager.cs
WheelWizard/Features/Settings/Types/DolphinSetting.cs
WheelWizard/Features/Settings/Types/RecompSetting.cs
WheelWizard/Features/Settings/Types/Setting.cs
WheelWizard/Features/Settings/Types/SettingConstants.cs
WheelWizard/Features/Settings/Types/VirtualSetting.cs
WheelWizard/Features/Settings/Types/WhWzSetting.cs
WheelWizard/Features/Settings/WhWzSettingManager.cs
WheelWizard/Views/Layout.axaml.cs
WheelWizard/Views/Pages/Settings/OtherSettings.axaml.cs
WheelWizard/Views/Pages/Settings/RecompSettings.axaml.cs
WheelWizard/Views/Pages/Settings/SettingsEditing.cs
WheelWizard/Views/Pages/Settings/VideoSettings.axaml.cs
WheelWizard/Views/Pages/Settings/WhWzSettings.axaml.cs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Walkthrough

Walkthrough

Settings now use generic value types and typed manager APIs. Setting implementations share a generic base, and manager registration uses setting-specific interfaces. Settings views route writes through a helper that handles failed updates. Tests use generic setting types and cover decimal parsing.

Changes

Typed Settings

Layer / File(s)	Summary
Generic setting model `WheelWizard/Features/Settings/Types/Setting.cs`, `WheelWizard/Features/Settings/Types/{DolphinSetting,RecompSetting,WhWzSetting,VirtualSetting}.cs`, `WheelWizard/Features/Settings/Types/SettingConstants.cs`	The non-generic setting base now provides shared change and validity behavior. Generic setting classes provide typed values and validators. Dolphin and Wheel Wizard parsing now accept decimal values in the updated tests; recomp double parsing rejects non-finite values.
Typed manager integration `WheelWizard/Features/Settings/ISettingsServices.cs`, `WheelWizard/Features/Settings/SettingsManager.cs`, `WheelWizard/Features/Settings/{DolphinSettingManager,RecompSettingManager,WhWzSettingManager}.cs`	Settings properties, manager methods, registration, and validation use typed settings. The managers store setting-specific interfaces. Recommended-settings writes now throw an `IOException` when a component setting cannot be saved.
Settings view updates `WheelWizard/Views/Pages/Settings/SettingsEditing.cs`, `WheelWizard/Views/Pages/Settings/{OtherSettings,RecompSettings,VideoSettings,WhWzSettings}.axaml.cs`, `WheelWizard/Views/Layout.axaml.cs`	Settings view handlers use `SettingsEditing.Set`, which displays a save error or invalid-path warning when a write fails. Location updates use `Setting<string>`; the success path no longer reloads Dolphin settings or displays the previous saved-path message.
Typed API tests `WheelWizard.Test/Features/Settings/`, `WheelWizard.Test/Features/{Dolphin,Launching}/`, `WheelWizard.Test/Features/MiiRepositoryServiceTests.cs`, `WheelWizard.Test/Views/HomeDolphinLaunchTests.cs`	Tests use generic settings and typed manager arguments. Dolphin and Wheel Wizard decimal parsing tests now assert successful parsing and values.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Suggested reviewers: patchzyy

Merge Risk: ⚪ Minimal · up to 6f1a5

No confirmed issue remains that should block merging after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6f1a5

The settings and path-change behavior warrant design review, but the inspected code retains validation and reloads the selected Dolphin profile after a successful path change. No introduced security issue was established. Failure and recovery behavior is not fully covered by the available evidence.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — A changed Dolphin executable or user directory can select a different local Dolphin configuration profile for subsequent reads and writes. The inspected entrypoint is a local settings edit, not an established remotely reachable entrypoint.

Trust Boundaries and Controls

observed — Typed access does not replace runtime validation: Setting validates updates, and Dolphin configuration values are parsed at the file boundary before they become setting values.

Resilience and Maintainability Implications

observed — Reload clears the Dolphin manager’s loaded flag before reading the newly resolved profile. File read errors in that loader are handled as empty input, leaving recovery after transient read failure dependent on a later reload.

Hardening Proposals

proposed — Exercise path edits through the view across failed persistence, transient profile-read failure, retry, and restart to verify that subsequent Dolphin writes use the intended profile.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.72% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 138 functions across 29 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main changes: type-safe settings values and centralized path reloads.
Description check	✅ Passed	The description includes all required template sections, explains the purpose and changes, provides test instructions and results, identifies related stack context, and confirms that relevant tests we…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

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

_{A rabbit checks each typed setting with care
A decimal hops through parsing to land
The settings now carry their types
A save error gets shown when writes fail
The tests plant fresh values in their rows
Then the rabbit bounds off, ears held high}

_{Comment @coderabbitai help to get the list of available commands.}

patchzyy

reviewed this layer against #498, including the typed setting api, json/ini/toml conversion, path reloads, and settings ui callers.

the existing tests pass locally: 551 unit tests and 19 ui tests. i did not find a new blocking issue in this layer.

the partial-save issue noted on #498 is still inherited here and should be fixed in that lower layer before merging the stack.

Updated yml to include new l10n on latest version of WhWz (v2.5.7), plus some mistake corrections on a few existing translations. Translation support is now back to 100%.

* i10n: Update German translation * fix: Update name

* l10n: Add missing German translations * l10n: Fix mistranslation of "scaling" * l10n: Fix issues in code review * l10n: Add translation for "polish" string --------- Co-authored-by: Dirk <dirkroosendaal04@gmail.com>

* l10n(leaderboard): Add l10n to basic strings * l10n(leaderboard): Add l10n to placements * l10n(leaderboard): Add l10n to snackbar messages * l10n(leaderboard): Add German translations * l10n: Fix rank labels not using localization * l10n: Fix retry/refresh buttons not using localization * l10n(Leaderboard): Translate unknown Mii name * l10n(Leaderboard): Use localization for podium placements * l10n(Leaderboard): Fix German retry/refresh translation * fix(LeaderboardPodiumCard): Make placement a Avalonia property --------- Co-authored-by: WantToBeeMe | Dirk <dirkroosendaal04@gmail.com>

Co-authored-by: Dirk <dirkroosendaal04@gmail.com>

* Preserve settings files and report persistence failures * Propagate recommended settings save failures * Recalculate virtual settings after partial save failures

DirkDoes added 2 commits September 28, 2026 22:43

Preserve settings files and report persistence failures

65e43f8

Make settings values type safe and centralize path reloads

6f1a537

DirkDoes added this pull request to stack #503 September 28, 2026 21:03

DirkDoes mentioned this pull request Sep 28, 2026

Add CLDR plural categories with a legacy translation path #500

Merged

1 task done

DirkDoes marked this pull request as ready for review September 28, 2026 21:20

DirkDoes requested a review from patchzyy September 28, 2026 21:20

Merge settings persistence updates into typed settings

90fdf02

patchzyy reviewed Oct 3, 2026

View reviewed changes

SpartaYoshi and others added 13 commits October 4, 2026 11:09

l10n: Spanish full translation update & improvements (#471)

5dcc55c

Updated yml to include new l10n on latest version of WhWz (v2.5.7), plus some mistake corrections on a few existing translations. Translation support is now back to 100%.

i10n: Update German translation (#481)

35b3a66

* i10n: Update German translation * fix: Update name

l10n: Add missing German translations (#487)

4b92373

* l10n: Add missing German translations * l10n: Fix mistranslation of "scaling" * l10n: Fix issues in code review * l10n: Add translation for "polish" string --------- Co-authored-by: Dirk <dirkroosendaal04@gmail.com>

i18n: import Czech, Spanish and Finnish translations (#507)

f2237c6

fix: remove unused build scripts (#497)

36a4fef

fix: respect tiling window managers better on linux (#467)

646ca7e

Co-authored-by: Dirk <dirkroosendaal04@gmail.com>

Preserve settings files and report persistence failures

f0396b5

Propagate recommended settings save failures

68eee20

Recalculate virtual settings after partial save failures

0ab000d

Merge updated settings persistence base into typed settings

7fda10d

Preserve settings files and report persistence failures (#498)

a5d429c

* Preserve settings files and report persistence failures * Propagate recommended settings save failures * Recalculate virtual settings after partial save failures

Reconcile typed settings with merged persistence changes on main

7c8ea1b

DirkDoes merged commit 5f87e26 into main Oct 4, 2026
3 checks passed

An error occurred while trying to automatically change base from task/settings-persistence to main October 4, 2026 09:58

DirkDoes deleted the task/typed-settings branch October 4, 2026 09:58

Navigation Menu

Repository navigation

Make settings values type safe and centralize path reloads - #499

Make settings values type safe and centralize path reloads#499
DirkDoes merged 16 commits into
task/settings-persistencefrom
task/typed-settings

DirkDoes commented Sep 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 28, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

patchzyy left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

DirkDoes commented Sep 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of this PR:

How to Test:

What Has Been Changed:

Related Issue Link:

Checklist before merging

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

patchzyy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

DirkDoes commented Sep 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 28, 2026 •

edited

Loading