Add a user-friendly OpenKH launcher - #1271
Conversation
WalkthroughThe change adds a Windows WPF launcher, installation-aware update services, legacy installation migration, desktop shortcut creation, and a release packaging workflow. The workflow validates and packages the launcher, Mod Manager, Panacea files, documentation, and advanced tools. ChangesLauncher release integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Launcher
participant ModManager
participant UpdateService
participant ReleaseArchive
User->>Launcher: Start launcher
Launcher->>ModManager: Start packaged Mod Manager
Launcher->>UpdateService: Check and install update
UpdateService->>Launcher: Report progress and restart target
ReleaseArchive->>User: Provide organized release package
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
|
Looks good to me |
|
Addressed the update and shortcut concerns in
I validated both legacy migration paths, the organized update archive, a complete Release build, and the Mod Manager test suite. |
|
Follow-up update in
The new behavior was validated in the packaged Release build. Automatic detection showed the indicator with no download and zero Mod Manager processes started. |
|
Final CI status for
The PR is now green. |
|
Hi Thanks for your contribution to OpenKh. I have tested the new Currently, I cannot reproduce the new folder structure of So I have tested the behaviour of PS V:\OpenKh-review-1271> .\pre-build.ps1
PS V:\OpenKh-review-1271> .\build.ps1
PS V:\OpenKh-review-1271> cd .\bin\
PS V:\OpenKh-review-1271\bin> .\OpenKh.Launcher.exe
The list is empty. And the problem is that when I close the launcher, the majority of OpenKh files are removed. before closing 
after closing 
I feel that this behavior is neither clearly documented nor easily predictable, and it feels kind of dangerous. And also, please allow me to turn on the coderabbitai review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
OpenKh.Tools.Launcher/MainWindow.xaml.cs (1)
43-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
OpenkhInstallationinstead of duplicating the path resolution.Lines 48-57 repeat the logic of
OpenkhInstallation.GetModManagerExecutable. The launcher already compilesOpenkhInstallation.cs(csproj line 28), and line 189 already callsOpenkhInstallation.Directory. Two copies can drift when the packaged layout changes.♻️ Proposed refactor
- private string BaseDirectory => AppContext.BaseDirectory; - private string ModManagerPath - { - get - { - var packagedPath = Path.Combine( - BaseDirectory, - ApplicationsDirectory, - ModManagerDirectory, - ModManagerExecutable - ); - - return File.Exists(packagedPath) - ? packagedPath - : Path.Combine(BaseDirectory, ModManagerExecutable); - } - } + private string BaseDirectory => OpenkhInstallation.Directory; + private string ModManagerPath => OpenkhInstallation.GetModManagerExecutable(BaseDirectory); private string AdvancedToolsPath => Path.Combine(BaseDirectory, AdvancedToolsDirectory); private string CompatibilityModManagerPath => Path.Combine(BaseDirectory, ModManagerExecutable);The constants
ApplicationsDirectoryandModManagerDirectoryat lines 17-18 then become unused and can be removed.🤖 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 `@OpenKh.Tools.Launcher/MainWindow.xaml.cs` around lines 43 - 61, Replace the duplicated path resolution in the ModManagerPath property with the existing OpenkhInstallation.GetModManagerExecutable logic, preserving the launcher’s resolved executable behavior. Remove ApplicationsDirectory and ModManagerDirectory if they become unused, and continue using OpenkhInstallation.Directory where applicable.OpenKh.Tools.Launcher/OpenKh.Tools.Launcher.csproj (1)
23-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider extracting the shared update services into a library instead of linking source.
Lines 28-30 compile three ModsManager source files into the launcher assembly, creating independently compiled
OpenKh.Tools.ModsManager.Services.*types in each executable. If update state is later shared between assemblies, source linking will not share the same type identity. A small shared library referenced by both projects removes this duplication.Add support for
Nullablein the ModsManager services if the shared types leave external contract members nullable.🤖 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 `@OpenKh.Tools.Launcher/OpenKh.Tools.Launcher.csproj` around lines 23 - 31, Extract the shared update service types OpenkhInstallation, OpenkhUpdateCheckerService, and OpenkhUpdateProceederService into a small library referenced by both OpenKh.Tools.Launcher and OpenKh.Tools.ModsManager, then remove the linked Compile entries from the launcher project. Enable nullable support in the shared library if any externally visible service contract members are nullable.
🤖 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 `@OpenKh.Tools.Launcher/App.xaml.cs`:
- Around line 11-18: Update OnStartup to schedule migration cleanup only when
the legacy layout exists and Apps/ModManager, AdvancedTools, and openkh-release
are absent. Wrap TryStartModManager and migration cleanup in exception handling
that records errors, while ensuring failures do not prevent MainWindow from
being created and shown. Preserve the existing early shutdown only when
TryStartModManager successfully starts the manager.
In `@OpenKh.Tools.Launcher/LegacyInstallationMigration.cs`:
- Around line 100-108: Update ScheduleCleanupIfNeeded to require a persisted
completed-migration marker and verified legacy-layout state before scheduling
cleanup. Restrict the collected legacyFiles and legacyDirectories to artifacts
confirmed as created by the old package, rather than deleting every matching
manifest or fallback path; otherwise skip cleanup entirely.
In `@OpenKh.Tools.Launcher/MainWindow.xaml.cs`:
- Around line 189-196: Update the launcherPath initialization in the update flow
to use Environment.ProcessPath instead of constructing the executable name with
the hardcoded "OpenKh.Launcher.exe" literal. Pass this running-process path
unchanged to OpenkhUpdateProceederService.UpdateAsync so both termination and
restart target the current launcher.
In `@OpenKh.Tools.ModsManager/Services/OpenkhUpdateProceederService.cs`:
- Around line 56-67: Update the OpenkhUpdateProceederService restart flow and
CreateBatchFileAsync invocation to terminate both the restartExecutable process
and the Mod Manager process before copying files. Preserve the existing restart
target while adding the executable name resolved by modManagerExecutable to the
batch generator’s process-stop list.
---
Nitpick comments:
In `@OpenKh.Tools.Launcher/MainWindow.xaml.cs`:
- Around line 43-61: Replace the duplicated path resolution in the
ModManagerPath property with the existing
OpenkhInstallation.GetModManagerExecutable logic, preserving the launcher’s
resolved executable behavior. Remove ApplicationsDirectory and
ModManagerDirectory if they become unused, and continue using
OpenkhInstallation.Directory where applicable.
In `@OpenKh.Tools.Launcher/OpenKh.Tools.Launcher.csproj`:
- Around line 23-31: Extract the shared update service types OpenkhInstallation,
OpenkhUpdateCheckerService, and OpenkhUpdateProceederService into a small
library referenced by both OpenKh.Tools.Launcher and OpenKh.Tools.ModsManager,
then remove the linked Compile entries from the launcher project. Enable
nullable support in the shared library if any externally visible service
contract members are nullable.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: 1f4ec05b-5fd4-4fd6-8abc-ec6fdc9cb5d2
⛔ Files ignored due to path filters (3)
images/openKH_Old.icois excluded by!**/*.icoimages/openkh-launcher-home.pngis excluded by!**/*.pngimages/openkh-launcher-tools.pngis excluded by!**/*.png
📒 Files selected for processing (19)
.github/scripts/prepare-release.ps1.github/workflows/dotnet.yml.gitignoreOpenKh.Tests.ModsManager/OpenkhInstallationTest.csOpenKh.Tools.Launcher/App.xamlOpenKh.Tools.Launcher/App.xaml.csOpenKh.Tools.Launcher/DesktopShortcutService.csOpenKh.Tools.Launcher/LegacyInstallationMigration.csOpenKh.Tools.Launcher/MainWindow.xamlOpenKh.Tools.Launcher/MainWindow.xaml.csOpenKh.Tools.Launcher/OpenKh.Tools.Launcher.csprojOpenKh.Tools.ModsManager/App.xaml.csOpenKh.Tools.ModsManager/Services/ConfigurationService.csOpenKh.Tools.ModsManager/Services/OpenkhInstallation.csOpenKh.Tools.ModsManager/Services/OpenkhUpdateCheckerService.csOpenKh.Tools.ModsManager/Services/OpenkhUpdateProceederService.csOpenKh.slnREADME.mddistribution/README-FIRST.txt
| if (LegacyInstallationMigration.TryStartModManager()) | ||
| { | ||
| Shutdown(); | ||
| return; | ||
| } | ||
|
|
||
| LegacyInstallationMigration.ScheduleCleanupIfNeeded(); | ||
| new MainWindow().Show(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the migration and cleanup logic that is not included in this review context.
set -euo pipefail
fd -t f 'LegacyInstallationMigration.cs' --exec cat -n {}
fd -t f 'DesktopShortcutService.cs' --exec cat -n {}
rg -n -C3 'Directory\.Delete|File\.Delete|rd /s|del /' --iglob '*.cs' --iglob '*.ps1'Repository: OpenKH/OpenKh
Length of output: 11456
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)LegacyInstallationMigration.cs$|(^|/)App\.xaml\.cs$|legacy-release-(files|directories)\.txt$|OpenKh\.Tools\.ModsManager\.exe$'
echo
echo "App.xaml.cs:"
fd -t f 'App.xaml.cs' --exec cat -n {}
echo
echo "Migration manifests:"
while IFS= read -r file; do
echo "--- ${file#"$PWD/"}"
cat -n "$file" | sed -n '1,220p'
done < <(fd -t f 'legacy-release-(files|directories)\.txt')
echo
echo "Search file deletions/scripts:"
rg -n -C3 'Directory\.Delete|File\.Delete|rm -rf|rmdir /s|rd /s|del /f /q|Remove-Item|legacy-release-(files|directories)' --iglob '*.cs' --iglob '*.ps1' --iglob '*.bat' --iglob '*.txt'Repository: OpenKH/OpenKh
Length of output: 13616
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Launcher App.xaml.cs:"
cat -n OpenKh.Tools.Launcher/App.xaml.cs | sed -n '1,40p'
echo
echo "LegacyInstallationMigration.cs:"
cat -n OpenKh.Tools.Launcher/LegacyInstallationMigration.cs | sed -n '1,80p;80,220p'
echo
echo "Manifest files:"
find . -name 'legacy-release-files.txt' -o -name 'legacy-release-directories.txt' | while IFS= read -r file; do
echo "--- ${file#"$PWD/"}"
cat -n "$file" | sed -n '1,260p'
done
echo
echo "Delete/delete-related references:"
rg -n -C2 'Directory\.Delete|File\.Delete|rmdir /s /q|del /f /q|Remove-Item|rd /s /q|legacy-release-(files|directories)' --iglob '*.cs' --iglob '*.ps1' --iglob '*.bat' --iglob '*.txt'Repository: OpenKH/OpenKh
Length of output: 9568
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Manifest files:"
find . -type f \( -name 'legacy-release-files.txt' -o -name 'legacy-release-directories.txt' \) -print | while IFS= read -r file; do
echo "--- ${file#"$PWD/"}"
wc -l "$file"
cat -n "$file" | sed -n '1,280p'
done
echo
echo "Delete-related references:"
rg -n -C2 'Directory\.Delete|File\.Delete|rmdir /s /q|rd /s /q|del /f /q|Remove-Item|legacy-release-(files|directories)' --iglob '*.cs' --iglob '*.ps1' --iglob '*.bat' --iglob '*.txt' --iglob '*.md'
echo
echo "Relevant files in OpenKh.Tools.Launcher:"
git ls-files OpenKh.Tools.Launcher | sortRepository: OpenKH/OpenKh
Length of output: 195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Manifest files:"
git ls-files | rg 'legacy-release-(files|directories)\.txt$' || true
find . -type f \( -name 'legacy-release-files.txt' -o -name 'legacy-release-directories.txt' \) -print || true
while IFS= read -r file; do
echo "--- ${file#"$PWD/"}"
wc -l "$file" || true
sed -n '1,320p' "$file" || true
done < <(git ls-files | rg 'legacy-release-(files|directories)\.txt$' || true)
echo
echo "Delete-related references in cs/txt/ps1/bat:"
git ls-files | rg '\.(cs|txt|ps1|bat|md)$' | while IFS= read -r file; do
rg -n -C2 'Directory\.Delete|File\.Delete|rmdir /s /q|rd /s /q|del /f /q|Remove-Item|legacy-release-(files|directories)' "$file" || true
done
echo
echo "Relevant launcher files:"
git ls-files OpenKh.Tools.Launcher | sortRepository: OpenKH/OpenKh
Length of output: 220
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Manifest files from git:"
git ls-files | rg 'legacy-release-(files|directories)\.txt$' || true
echo "Manifest files from find:"
find . -type f \( -name 'legacy-release-files.txt' -o -name 'legacy-release-directories.txt' \) -print || true
echo "Read manifest files:"
git ls-files | rg 'legacy-release-(files|directories)\.txt$' | while IFS= read -r file; do
echo "--- ${file#"$PWD/"}"
wc -l "$file"
sed -n '1,320p' "$file"
done
echo
echo "Delete-related refs in tracked text/coding files:"
git ls-files | rg '\.(cs|txt|ps1|bat|md)$' | while IFS= read -r file; do
rg -n -C2 'Directory\.Delete|File\.Delete|rmdir /s /q|rd /s /q|del /f /q|Remove-Item|legacy-release-(files|directories)' "$file" || true
done
echo
echo "Launcher file list:"
git ls-files OpenKh.Tools.Launcher | sortRepository: OpenKH/OpenKh
Length of output: 223
Guard startup cleanup and keep the launcher launchable.
ScheduleCleanupIfNeeded()is called unconditionally and can generate a migration batch that deletes top-level files matching the legacy file selector, including.dllfiles, and theFallbackLegacyResourceDirectoriesbefore the launcher has checked the new packaged layout. Add a guard that runs cleanup only when the legacy layout is present and newer paths likeApps/ModManager,AdvancedTools, oropenkh-releaseare absent.- If
TryStartModManager()throws outside the existingProcess.Start()block, or if later startup throws,OnStartupexits beforeMainWindowopens. Catch migration-related exceptions, record an error, and continue to createMainWindow.
🤖 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 `@OpenKh.Tools.Launcher/App.xaml.cs` around lines 11 - 18, Update OnStartup to
schedule migration cleanup only when the legacy layout exists and
Apps/ModManager, AdvancedTools, and openkh-release are absent. Wrap
TryStartModManager and migration cleanup in exception handling that records
errors, while ensuring failures do not prevent MainWindow from being created and
shown. Preserve the existing early shutdown only when TryStartModManager
successfully starts the manager.
| private static void ScheduleCleanupIfNeeded(string installationDirectory) | ||
| { | ||
| var legacyFiles = GetLegacyApplicationFiles(installationDirectory) | ||
| .Where(File.Exists) | ||
| .ToArray(); | ||
| var legacyDirectories = GetLegacyResourceDirectories(installationDirectory) | ||
| .Select(directoryName => Path.Combine(installationDirectory, directoryName)) | ||
| .Where(Directory.Exists) | ||
| .ToArray(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Require a verified migration before cleanup.
ScheduleCleanupIfNeeded() runs during normal launcher startup. This code deletes every root directory named by the manifest, or by the fallback list such as resources and runtimes. A user can lose files after moving content into one of these directories before closing the launcher.
Persist a completed-migration marker. Delete files only after verifying the legacy layout. Restrict deletion to artifacts that the old package created.
🤖 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 `@OpenKh.Tools.Launcher/LegacyInstallationMigration.cs` around lines 100 - 108,
Update ScheduleCleanupIfNeeded to require a persisted completed-migration marker
and verified legacy-layout state before scheduling cleanup. Restrict the
collected legacyFiles and legacyDirectories to artifacts confirmed as created by
the old package, rather than deleting every matching manifest or fallback path;
otherwise skip cleanup entirely.
| var launcherPath = Path.Combine(OpenkhInstallation.Directory, "OpenKh.Launcher.exe"); | ||
| await new OpenkhUpdateProceederService().UpdateAsync( | ||
| checkResult.DownloadZipUrl, | ||
| rate => Dispatcher.Invoke(() => | ||
| CheckForUpdatesButton.Content = $"Downloading {rate:P0}"), | ||
| CancellationToken.None, | ||
| launcherPath | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Derive the restart path from the running process.
Line 189 hardcodes "OpenKh.Launcher.exe". That literal duplicates <AssemblyName> in OpenKh.Tools.Launcher.csproj. If the assembly name changes, the batch file restarts a path that does not exist, and the user is left with no running application after the update. The update service also derives the process to terminate from this same value, so both actions break together.
Use Environment.ProcessPath, which always names the running launcher.
🐛 Proposed fix
CheckForUpdatesButton.Content = "Downloading Update...";
- var launcherPath = Path.Combine(OpenkhInstallation.Directory, "OpenKh.Launcher.exe");
+ var launcherPath = Environment.ProcessPath
+ ?? Path.Combine(OpenkhInstallation.Directory, "OpenKh.Launcher.exe");
await new OpenkhUpdateProceederService().UpdateAsync(📝 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.
| var launcherPath = Path.Combine(OpenkhInstallation.Directory, "OpenKh.Launcher.exe"); | |
| await new OpenkhUpdateProceederService().UpdateAsync( | |
| checkResult.DownloadZipUrl, | |
| rate => Dispatcher.Invoke(() => | |
| CheckForUpdatesButton.Content = $"Downloading {rate:P0}"), | |
| CancellationToken.None, | |
| launcherPath | |
| ); | |
| CheckForUpdatesButton.Content = "Downloading Update..."; | |
| var launcherPath = Environment.ProcessPath | |
| ?? Path.Combine(OpenkhInstallation.Directory, "OpenKh.Launcher.exe"); | |
| await new OpenkhUpdateProceederService().UpdateAsync( | |
| checkResult.DownloadZipUrl, | |
| rate => Dispatcher.Invoke(() => | |
| CheckForUpdatesButton.Content = $"Downloading {rate:P0}"), | |
| CancellationToken.None, | |
| launcherPath | |
| ); |
🤖 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 `@OpenKh.Tools.Launcher/MainWindow.xaml.cs` around lines 189 - 196, Update the
launcherPath initialization in the update flow to use Environment.ProcessPath
instead of constructing the executable name with the hardcoded
"OpenKh.Launcher.exe" literal. Pass this running-process path unchanged to
OpenkhUpdateProceederService.UpdateAsync so both termination and restart target
the current launcher.
| var modManagerExecutable = File.Exists(packagedModManagerExecutable) | ||
| ? Path.Combine(copyTo, "Apps", "ModManager", "OpenKh.Tools.ModsManager.exe") | ||
| : OpenkhInstallation.GetModManagerExecutable(copyTo); | ||
| var restartExecutable = string.IsNullOrWhiteSpace(executableToRestart) | ||
| ? modManagerExecutable | ||
| : executableToRestart; | ||
| await CreateBatchFileAsync( | ||
| tempBatFile: tempBatFile, | ||
| copyFrom: copyFrom, | ||
| copyTo: copyTo, | ||
| processToStop: Path.GetFileName(restartExecutable), | ||
| execAfter: $"start \"\" \"{restartExecutable}\"" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Terminate the Mod Manager as well as the restart executable.
Line 66 stops only the process named by restartExecutable. When the launcher starts the update, that name is OpenKh.Launcher.exe. A Mod Manager instance started earlier from the launcher (MainWindow.xaml.cs line 150) keeps running and holds locks on Apps/ModManager. Robocopy then fails to replace those binaries, if errorlevel 8 pause blocks the console, and the installation ends in a mixed-version state.
Pass both process names to the batch generator, or always add the Mod Manager process to the terminate list.
🛠️ Proposed fix to stop both processes
- await CreateBatchFileAsync(
- tempBatFile: tempBatFile,
- copyFrom: copyFrom,
- copyTo: copyTo,
- processToStop: Path.GetFileName(restartExecutable),
- execAfter: $"start \"\" \"{restartExecutable}\""
- );
+ var processesToStop = new[]
+ {
+ Path.GetFileName(restartExecutable),
+ Path.GetFileName(modManagerExecutable),
+ }
+ .Distinct(StringComparer.OrdinalIgnoreCase)
+ .ToArray();
+
+ await CreateBatchFileAsync(
+ tempBatFile: tempBatFile,
+ copyFrom: copyFrom,
+ copyTo: copyTo,
+ processesToStop: processesToStop,
+ execAfter: $"start \"\" \"{restartExecutable}\""
+ );Update the generator accordingly:
private async Task CreateBatchFileAsync(
string tempBatFile,
string copyFrom,
string copyTo,
- string processToStop,
+ IReadOnlyCollection<string> processesToStop,
string execAfter
)
{
var bat = new StringWriter();
bat.WriteLine($"chcp 65001");
- bat.WriteLine($"taskkill /im {EscapeRobocopyArg(processToStop)}");
+ foreach (var processToStop in processesToStop)
+ bat.WriteLine($"taskkill /im {EscapeRobocopyArg(processToStop)}");
bat.WriteLine($"robocopy {EscapeRobocopyArg(copyFrom)} {EscapeRobocopyArg(copyTo)} /e");📝 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.
| var modManagerExecutable = File.Exists(packagedModManagerExecutable) | |
| ? Path.Combine(copyTo, "Apps", "ModManager", "OpenKh.Tools.ModsManager.exe") | |
| : OpenkhInstallation.GetModManagerExecutable(copyTo); | |
| var restartExecutable = string.IsNullOrWhiteSpace(executableToRestart) | |
| ? modManagerExecutable | |
| : executableToRestart; | |
| await CreateBatchFileAsync( | |
| tempBatFile: tempBatFile, | |
| copyFrom: copyFrom, | |
| copyTo: copyTo, | |
| processToStop: Path.GetFileName(restartExecutable), | |
| execAfter: $"start \"\" \"{restartExecutable}\"" | |
| var modManagerExecutable = File.Exists(packagedModManagerExecutable) | |
| ? Path.Combine(copyTo, "Apps", "ModManager", "OpenKh.Tools.ModsManager.exe") | |
| : OpenkhInstallation.GetModManagerExecutable(copyTo); | |
| var restartExecutable = string.IsNullOrWhiteSpace(executableToRestart) | |
| ? modManagerExecutable | |
| : executableToRestart; | |
| var processesToStop = new[] | |
| { | |
| Path.GetFileName(restartExecutable), | |
| Path.GetFileName(modManagerExecutable), | |
| } | |
| .Distinct(StringComparer.OrdinalIgnoreCase) | |
| .ToArray(); | |
| await CreateBatchFileAsync( | |
| tempBatFile: tempBatFile, | |
| copyFrom: copyFrom, | |
| copyTo: copyTo, | |
| processesToStop: processesToStop, | |
| execAfter: $"start \"\" \"{restartExecutable}\"" |
🤖 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 `@OpenKh.Tools.ModsManager/Services/OpenkhUpdateProceederService.cs` around
lines 56 - 67, Update the OpenkhUpdateProceederService restart flow and
CreateBatchFileAsync invocation to terminate both the restartExecutable process
and the Mod Manager process before copying files. Preserve the existing restart
target while adding the executable name resolved by modManagerExecutable to the
batch generator’s process-stop list.
shananas
left a comment
shananas
left a comment
There was a problem hiding this comment.
Didnt review in great detail all the xaml stuff. From what i can tell looks good.
Question I did mention in discord why is mod manager in its own folder. From my testing all the created files/folder from mod manager are in the base folder. Which is still much a cleaner base folder and imo should stay there (idk if you were planning on changing that in the future or anything) If this is the case to keep them there why not just have one subfolder with all tools mods manager included. any standard user is going to either use the launcher or shortcut anyways. Mods manager does refrence some other programs in the toolkit for some of its features and im sure its all linked properly I just dont see any reason for not jsut having 1 subfolder with everything (at least with the PR as it is from a few suggested changes already).
My only real request then just have all apps in 1 subfolder and keep the base fodler as it is currently (Launcher, Mods Manager compate exe and the created files and folders from mods manager)
As for my discord comment about some files possibly not being cleaned up I was using the latest OpenKh release when testing but I went ahead and did a proper test on my fork and all files were cleaned up properly so disregard that comment. Not sure why that happened.
|
@kenjiuno not sure why all your stuff here is empty I set up 2 proper releases on my fork one before this PR and one after and all the tools are there and cleaned up properly. Also see some leftover files in your images I did have in a test I did without using Github releases so not sure exactly how that happens but at least going through github on my fork downloading release 1 then updating to release 2 it removes all extra files and has tools in the proper folders. 
Defenitely understand your thought on warning popups or documentation about the deletion of the old files/folders |
|
This would certainly be helpful and slightly less intimidating for everyday people to use. I approve of it. |
Meant to just mark as review as I did kind of request a change.
|
I have addressed the latest feedback in commit All applications now share a single The migration also removes the previous I also added persistent tool favorites. Each modding tool now has a star button. Favorites are moved to the top of the list and remain selected after restarting or updating OpenKH. Validation completed:
Updated screenshot:
|
5 participants
Summary
Apps.AdvancedToolsandApps\ModManagerdirectories when migrating an earlier preview layout.Update Availablewhen a new release exists.Screenshots
Launcher
Modding tools and favorites
Validation
Appscontains the Mod Manager, 28 graphical modding tools, command-line programs, and shared dependencies.AdvancedToolsnor the previousApps\ModManagerdirectory.Apps\OpenKh.Tools.ModsManager.exeand user files.OpenKh.Tests.ModsManager: 16 tests passed.