-
Notifications
You must be signed in to change notification settings - Fork 233
Support custom uninstallation methods and implement it for ReturnOfModding #1365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
V2 extends the original PackageInstaller by adding support for installer specific uninstallation logic. Having two separate versions is a bit hacky and requires the profile installer provider to do some extra checks. However, this allows us to add support for one installer at the time, instead of having to implement them all at once.
Marked as draft because:
|
Uninstalling a mod using ReturnofModding should remove the plugins_data subdir. However, if it contains a cache subdir, the contents of that subdir are kept, while rest of the content is removed. This is done as a way to persist some of the data created by the plugin when the mod is updated, since the update process uninstalls the old version before the new one is installed. The behaviour is documented at https://github.com/xiaoxiao921/ReturnOfModdingBase/tree/master?tab=readme-ov-file#plugins_data
- Actually rename the leaf item of the path - Throw error if source file/dir and target path don't exists to be in line with Node's rename() - Throw error if attempting to rename an item at the root of mocked file system. Rename (and other methods) assume there's some form of dir structure. Fixing this would be too much work now, so just document the "feature"
- Changing the state of the mod loader does nothing, which is in line with what BepInEx does, so I guess it's okay - Disabling a mod renames the plugin's files to have .old file extension and enabling a mod undoes this - The change also affects games using InstallRuleInstaller with the SUBDIR_NO_FLATTEN tracking method, making it match the functionality SUBDIR tracking method already had
Somewhat related to the comment here: #1388 (comment), I think the best approach would be to add optional methods to the current interface (so type hint it as potentially being |
Based on code review feedback, implement the uninstallation methods as optional methods, rather than have a separate version of the abstract class with more abstract methods. Replace the approach with abstract classes and methods with a plain TypeScript interface because having optional methods in abstract classes seems poorly supported in TypeScript.
@MythicManiac changes added in a separate commit 280852c. Note that this drops the abstract class approach since TypeScript seems to support optional methods in abstract classes poorly. |
No description provided.