Exctraction Edge Case

[shananas]

Dont extract a game if it was checked but has since been uninstalled, moved, or the filepath is no longer correct for some other reason

[coderabbitai]

📝 Walkthrough

Walkthrough

When extracting PC game data, the ViewModel now derives a local GamesFound from PcReleaseSelections and proactively disables incompatible extraction flags before calling ExtractGameData. Separately, the ConfigurationService setters now remove all matching entries from _config.GamesToExtract when disabling a game.

Changes

PC Edition Extraction Logic

Layer / File(s)	Summary
Decision / Input OpenKh.Tools.ModsManager/ViewModels/SetupWizardViewModel.cs	Introduces local GamesFound = PcReleaseSelections inside ExtractGameDataCommand for PC edition.
Control Flow OpenKh.Tools.ModsManager/ViewModels/SetupWizardViewModel.cs	Conditionally sets Extractkh3d = false when GamesFound == "1.5+2.5", and sets Extractkh1/Extractkh2/Extractbbs/Extractrecom = false when GamesFound == "2.8", before calling ExtractGameData(null, GameDataLocation).
Behavioral Impact OpenKh.Tools.ModsManager/ViewModels/SetupWizardViewModel.cs	Prevents invoking extraction for editions incompatible with the selected PC release(s) by mutating extraction properties locally prior to extraction invocation.

ConfigurationService: GamesToExtract Cleanup

Layer / File(s)	Summary
Setter Behavior OpenKh.Tools.ModsManager/Services/ConfigurationService.cs	Each disable-branch setter (Extractkh1, Extractkh2, Extractbbs, Extractrecom, Extractkh3d) now removes all matching entries from _config.GamesToExtract using RemoveAll(x => x == "...") instead of Remove("...").
Persistence OpenKh.Tools.ModsManager/Services/ConfigurationService.cs	Existing save behavior (_config.Save(ConfigPath)) and getter semantics remain unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Setup Wizard and Mod Manager Optimization #1228: Modifies PC game-selection and ISO/validation logic in SetupWizardViewModel, related to PC release selection and visibility/flow.

Suggested reviewers

• Delta-473

Poem

🐰 The wizard checks each chosen game with care,

KH1, KH2, BBS, and KH3D beware—
Selections steer which files we seek,
Clean removals make the config sleek.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	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 references 'Extraction Edge Case' which aligns with the changeset's focus on handling extraction scenarios when games are uninstalled or moved, though it contains a spelling error ('Exctraction').
Description check	✅ Passed	The description directly explains the motivation for the changes: preventing extraction of games that were checked but have since become unavailable, which matches the changeset's implementation of disabling extraction targets based on detected game availability.
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.

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

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 0/1 reviews remaining, refill in 60 minutes.}

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

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

OpenKh.Tools.ModsManager/ViewModels/SetupWizardViewModel.cs (1)

1861-1884: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard the empty PC-release case before calling extraction.

If the selected install was moved or uninstalled, these gates can make every extraction flag false, but ExtractKhPcEditionAsync still enters its onProgress(0) path and never reaches a completion update. That leaves the wizard stuck in the busy state with no feedback.

Suggested fix

                         var langFolder = ConfigurationService.PCVersion == "Steam" ? "dt" : _pcReleaseLanguage == "jp" ? "jp" : "en";
                         string GamesFound = PcReleaseSelections;
+                        if (string.IsNullOrEmpty(GamesFound))
+                        {
+                            MessageBox.Show(
+                                "No valid PC release was found. Please reselect the install directory before extracting.",
+                                "Extraction error",
+                                MessageBoxButton.OK,
+                                MessageBoxImage.Warning);
+                            return;
+                        }

                         await _gameDataExtractionService.ExtractKhPcEditionAsync(

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 31702327-06ba-4979-9656-93e072efe092

📥 Commits

Reviewing files that changed from the base of the PR and between d814753 and f8145d5.

📒 Files selected for processing (1)

• OpenKh.Tools.ModsManager/ViewModels/SetupWizardViewModel.cs

[coderabbitai]

🧹 Nitpick comments (1)

OpenKh.Tools.ModsManager/Services/ConfigurationService.cs (1)

495-503: ⚡ Quick win

Add on the true branch is still unconditional — duplicates can re-accumulate.

Each setter's true branch calls _config.GamesToExtract.Add(...) without a Contains guard. If the setter is invoked with true more than once (e.g., wizard re-entered), a duplicate entry is appended again. RemoveAll will clean it up on the next false assignment, but the persisted YAML will accumulate duplicates in the meantime, which is precisely the problem this PR is solving.

The minimal fix is a Contains check before Add; a stronger fix is to make GamesToExtract a HashSet<string> (requires a YAML-serialization-compatible wrapper) or deduplicate on load.

♻️ Proposed fix (guard Add with Contains — example for all five setters)

         if (value)
         {
-            _config.GamesToExtract.Add("kh1");
+            if (!_config.GamesToExtract.Contains("kh1"))
+                _config.GamesToExtract.Add("kh1");
         }

Apply the same pattern for "kh2", "bbs", "Recom", and "kh3d".

Also applies to: 511-519, 527-535, 543-551, 559-567

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@OpenKh.Tools.ModsManager/Services/ConfigurationService.cs` around lines 495 -
503, The setter code in ConfigurationService.cs unconditionally calls
_config.GamesToExtract.Add(...) (e.g., for "kh1", and similarly for
"kh2","bbs","Recom","kh3d") which allows duplicate entries to accumulate; modify
each setter so that before calling _config.GamesToExtract.Add("...") you check
if !_config.GamesToExtract.Contains("...") and only then add, then still call
_config.Save(ConfigPath); apply this guard to all five setters (the blocks
currently manipulating _config.GamesToExtract for
"kh1","kh2","bbs","Recom","kh3d") to prevent duplicate entries being persisted.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@OpenKh.Tools.ModsManager/Services/ConfigurationService.cs`:
- Around line 495-503: The setter code in ConfigurationService.cs
unconditionally calls _config.GamesToExtract.Add(...) (e.g., for "kh1", and
similarly for "kh2","bbs","Recom","kh3d") which allows duplicate entries to
accumulate; modify each setter so that before calling
_config.GamesToExtract.Add("...") you check if
!_config.GamesToExtract.Contains("...") and only then add, then still call
_config.Save(ConfigPath); apply this guard to all five setters (the blocks
currently manipulating _config.GamesToExtract for
"kh1","kh2","bbs","Recom","kh3d") to prevent duplicate entries being persisted.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0e10df05-ad60-40ff-9668-2c39798bd7f7

📥 Commits

Reviewing files that changed from the base of the PR and between f8145d5 and 19cdb22.

📒 Files selected for processing (2)

• OpenKh.Tools.ModsManager/Services/ConfigurationService.cs
• OpenKh.Tools.ModsManager/ViewModels/SetupWizardViewModel.cs

🚧 Files skipped from review as they are similar to previous changes (1)

• OpenKh.Tools.ModsManager/ViewModels/SetupWizardViewModel.cs