Add workspace to MiniInstaller by DashingCat · Pull Request #1037 · EverestAPI/Everest · GitHub
Skip to content

Add workspace to MiniInstaller - #1037

Merged
SnipUndercover merged 8 commits into
EverestAPI:devfrom
DashingCat:983-add-workspace-to-miniinstaller
Jun 28, 2026
Merged

SnipUndercover merged 8 commits into
EverestAPI:devfrom
DashingCat:983-add-workspace-to-miniinstaller

Conversation

@DashingCat

@DashingCat DashingCat commented Dec 10, 2025

Copy link
Copy Markdown
Contributor

This PR adds a workspace folder to the MiniInstaller installation process, so it doesn't overwrite Celeste.exe early (for example, during the step applying Everest patches, after coreification).

The workspace is used to store the Celeste.exe/Celeste.dll files, until after the step applying Everest patches, reducing the time window during which the process can be interrupted and left in a state which would allow orig/Celeste.exe to be coreified.

This is a work in progress, the following still needs to be done:

  • Testing on Windows
  • Testing on MacOS
  • Applying the changes to MiniInstaller's "FastMode" (not needed)

Fixes #983.
Fixes #984.
As far as I understand, #984 occurs only because of #983.

@maddie480-bot maddie480-bot added the 0: draft This PR is not ready for review yet (bot-managed) label Dec 10, 2025
@DashingCat
DashingCat marked this pull request as ready for review December 14, 2025 18:47
@maddie480-bot maddie480-bot added 1: review needed This PR needs 2 approvals to be merged (bot-managed) and removed 0: draft This PR is not ready for review yet (bot-managed) labels Dec 14, 2025

@Wartori54 Wartori54 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just some polish needed here and there.
About your fast mode question, I'd be fine with merging without any changes to fast mode, it is meant for advanced users only, so if the installation breaks the user will notice that and be able to fix it.

Comment thread MiniInstaller/Program.cs Outdated
Comment thread MiniInstaller/LibAndDepHandling.cs
Comment thread MiniInstaller/Program.cs
Comment thread MiniInstaller/Program.cs Outdated
@maddie480-bot maddie480-bot added 2: changes requested This PR cannot be merged because changes were requested (bot-managed) and removed 1: review needed This PR needs 2 approvals to be merged (bot-managed) labels Dec 16, 2025
@DashingCat

Copy link
Copy Markdown
Contributor Author

@DashingCat
DashingCat requested a review from Wartori54 December 17, 2025 23:57

@Wartori54 Wartori54 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Other than this LGTM. Would appreciate some testing on other platforms before merging though.

Comment thread MiniInstaller/DepCalls.cs Outdated
Comment thread MiniInstaller/DepCalls.cs Outdated
@DashingCat

Copy link
Copy Markdown
Contributor Author

The latest commit uses Path.PathSeparator instead of :.

@DashingCat
DashingCat requested a review from Wartori54 December 18, 2025 22:45
@maddie480-bot maddie480-bot added 1: review needed This PR needs 2 approvals to be merged (bot-managed) and removed 2: changes requested This PR cannot be merged because changes were requested (bot-managed) labels Dec 22, 2025

@microlith57 microlith57 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

code looks good, just a few missing param names (non-blocking). still would be good to test on windows/macos :p

Comment thread MiniInstaller/MiscUtil.cs Outdated
public static void MoveExecutable(string srcPath, string dstPath) {
File.Delete(dstPath);
File.Move(srcPath, dstPath);
File.Move(srcPath, dstPath, true);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nitpick: (for readability)

Suggested change
File.Move(srcPath, dstPath, true);
File.Move(srcPath, dstPath, overwrite: true);

(& similarly for the other File.Move calls below)

Comment thread MiniInstaller/LibAndDepHandling.cs Outdated
string steamworksLibDst = Path.Combine(Globals.PathGame, "Steamworks.NET.dll");
File.Delete(steamworksLibDst);
File.Copy(steamworksLibSrc, steamworksLibDst);
File.Copy(steamworksLibSrc, steamworksLibDst, true);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nitpick: (for readability)

Suggested change
File.Copy(steamworksLibSrc, steamworksLibDst, true);
File.Copy(steamworksLibSrc, steamworksLibDst, overwrite: true);

Comment thread MiniInstaller/Program.cs Outdated

if (Directory.Exists(Globals.PathMiniInstallerWorkspace)) {
Logger.LogLine("MiniInstaller workspace already exists, cleaning before continuing.");
Directory.Delete(Globals.PathMiniInstallerWorkspace, true);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nitpick: (readability)

Suggested change
Directory.Delete(Globals.PathMiniInstallerWorkspace, true);
Directory.Delete(Globals.PathMiniInstallerWorkspace, recursive: true);

(& similarly for the other recursive Directory.Delete)

@maddie480-bot

Copy link
Copy Markdown
Member

The pull request was approved and entered the 3-day last-call window.
If no further reviews happen, it will end on Jan 6, 2026, 12:08 AM UTC, after which the pull request will be able to be merged.

@maddie480-bot maddie480-bot added 3: last call window This PR was approved, and is in the 5-day last-call window before getting merged (bot-managed) and removed 1: review needed This PR needs 2 approvals to be merged (bot-managed) labels Jan 3, 2026

@SnipUndercover SnipUndercover left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Adding a "request changes" review just so that this doesn't get accidentally merged without testing, will turn into an approval when it's done.

@SnipUndercover SnipUndercover self-assigned this Jan 3, 2026
@maddie480-bot maddie480-bot added 2: changes requested This PR cannot be merged because changes were requested (bot-managed) and removed 3: last call window This PR was approved, and is in the 5-day last-call window before getting merged (bot-managed) labels Jan 3, 2026
@microlith57 microlith57 added the 2: testing needed This PR needs to be tested on one or more platforms label Jan 8, 2026
@DashingCat

Copy link
Copy Markdown
Contributor Author

Parameter names have been added in the latest commit.

@DashingCat
DashingCat force-pushed the 983-add-workspace-to-miniinstaller branch from 33be988 to 186809b Compare May 29, 2026 20:43

@SnipUndercover SnipUndercover left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code-wise looks good, but upon testing MiniInstaller crashes when patching FNA. It attempts to resolve the Celeste assembly, but it finds Celeste.exe which is a Piton apphost on already modded installs. MonoMod then tries to read that as a managed .NET assembly and promptly crash.

MonoMod does not consider the coreified vanilla Celeste copy in the workspace, as it is named Celeste.Core.dll instead of Celeste.dll.

I cannot commit to your branch, so I have sent a patch file in #modding_dev on the Celeste Discord server.
I tested it by running MiniInstaller twice from a fresh copy of Celeste and confirmed it works.

@SnipUndercover

SnipUndercover commented May 29, 2026

Copy link
Copy Markdown
Member

Also this doesn't really solve the problem of Celeste.dll being missing when the installation is interrupted. We could skip deleting Celeste.dll and just have the copy from the workspace overwrite the dll?

@DashingCat

Copy link
Copy Markdown
Contributor Author

From my testing, @SnipUndercover patches work on Linux as well.

@SnipUndercover SnipUndercover left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Confirmed the PR works on MacOS.
LGTM.

@maddie480-bot

Copy link
Copy Markdown
Member

The pull request was approved and entered the 3-day last-call window. Since no PR should be merged within 3 days of the next rolling release, the last-call window is extended further.
If no further reviews happen, it will end on Jun 28, 2026, 12:00 AM UTC, after which the pull request will be able to be merged.

@maddie480-bot maddie480-bot added 3: last call window This PR was approved, and is in the 5-day last-call window before getting merged (bot-managed) and removed 2: changes requested This PR cannot be merged because changes were requested (bot-managed) labels Jun 24, 2026
@DashingCat DashingCat removed the 2: testing needed This PR needs to be tested on one or more platforms label Jun 24, 2026
@maddie480-bot

Copy link
Copy Markdown
Member

@maddie480-bot maddie480-bot added 4: ready to merge This PR was approved and the last-call window is over (bot-managed) and removed 3: last call window This PR was approved, and is in the 5-day last-call window before getting merged (bot-managed) labels Jun 28, 2026
@SnipUndercover
SnipUndercover merged commit 384acc1 into EverestAPI:dev Jun 28, 2026
3 checks passed
maddie480 added a commit that referenced this pull request Jul 25, 2026
…-miniinstaller"

This reverts commit 384acc1, reversing
changes made to 6988c0d.
maddie480 added a commit that referenced this pull request Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4: ready to merge This PR was approved and the last-call window is over (bot-managed)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Installing Everest with coreified vanilla always fails and deletes Celeste.dll orig/Celeste.exe is coreified and does not run

6 participants