Skip to content
This repository has been archived by the owner on Feb 28, 2024. It is now read-only.

Added late loading functionality for mods. #55

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Psychpsyo
Copy link
Contributor

This makes the modloader watch the nml_mods directory and attempt to load any new .dll file it finds in there during runtime as a new mod.

This also adds a new function, OnLateInit() to the NeosMod class. This can be overriden by a mod to handle late loading differently from being loaded at startup.

When late loading a mod, the modloader will use the FrooxEngine.LoadingIndicator to indicate success or failure to the user so that a new mod appearing isn't entirely invisible.

Multiple sections of code in the ModLoader and one in AssemblyLoader have been turned into their own functions to be reused during late loading.

This makes the ModLoader watch the nml_mods directory and attempt to load any new .dll file it finds in there as a mod.
@XDelta
Copy link
Member

XDelta commented Aug 3, 2022

Would it make sense to also be prompting the user before actually loading new assemblies. Could provide the name and if the maybe if the mod has explicit support for being hotloaded?

@EIA485
Copy link
Member

EIA485 commented Aug 3, 2022

maybe config options for enabling/disabling hotloading for anymod/not explicitly supported mods

@Psychpsyo
Copy link
Contributor Author

Psychpsyo commented Aug 3, 2022

Would it make sense to also be prompting the user before actually loading new assemblies. Could provide the name and if the maybe if the mod has explicit support for being hotloaded?

Could do that. Though currently a mod has no way of explicitly identifying itself as hotloadable.

Also: I just found a bug with auto-loading mods from subdirectories so definitely no merging this yet. Fixed that now

Also fixed line endings in NeosMod.cs
Also added a new config option to disable late loading.
@EIA485
Copy link
Member

EIA485 commented Aug 3, 2022

if the mod implements onlateload then its hotloadable?

@Psychpsyo
Copy link
Contributor Author

Psychpsyo commented Aug 3, 2022

if the mod implements onlateload then its hotloadable?

On the one hand, I like this. On the other hand, that'll just make a lot of mods override OnLateLoad with the same thing again. (just calling OnEngineInit)

@ljoonal
Copy link
Member

ljoonal commented Aug 3, 2022

Time to start a NML2 branch and start doing the refactors to make this better but maybe break backwards compatibility slightly?

@Psychpsyo
Copy link
Contributor Author

Psychpsyo commented Aug 3, 2022

Sounds like that might make sense.

Though I had one idea that would work as a temporary solution:
Make a new, generic OnInit function for mods that takes a bool, telling it if the mod was loaded at startup or later. (The mod can then do with that whatever it wants)
If a mod overrides this new OnInit, the modloader uses that, otherwise it defaults to OnEngineInit for backwards compatibility.
Then we could have a config argument for whether or not it should be allowed to hotload mods without OnInit.

Mods now have an OnInit(bool atStartup) which replaces the old OnEngineInit().

If it is available, that signals to the modloader that the mod supports hotloading.
@Psychpsyo
Copy link
Contributor Author

Psychpsyo commented Aug 3, 2022

I have now done this so mods can implement a new OnInit(bool atStartup) function.
This function signals to the modloader that the mod supports hotloading.
The modloader uses this function both on startup and when hotloading so OnEngineInit() would be a legacy thing now that's only still there for backwards compatibility.

I have documented the config parameters that control this (hotloading and hotloadunsupported) but I haven't written any documentation on the function itself yet since OnEngineInit() is still technically completely fine.

I think that this is a clean way of doing it since all the backwards compatibility for OnEngineInit() could be removed for NML2.

@XDelta
Copy link
Member

XDelta commented Aug 3, 2022

As described OnEngineInit wouldn't be legacy? as that would be how a mod continues to say it doesn't support hotloading. Unless the intention is that a mod that doesn't support hotloading uses OnInit and just stops itself if it isn't at startup? That seems like it existing at all would signal to the mod loader that it can be hotloaded despite not and trying to explicitly state it doesn't

@XDelta
Copy link
Member

XDelta commented Aug 3, 2022

Is a mod signalling to the modloader in anyway that it actually supports being hotloaded aside from the existence of a method that would eventually be the single Init function again? Or only the other way around of the modloader informing a mod when it is loaded from the start vs later on. Dunno how much it actually matters I suppose

@Psychpsyo
Copy link
Contributor Author

Psychpsyo commented Aug 4, 2022

You're right. OnInit() more just signals "I support this new way of handling initialization" and then the mod can decide to not do anything if it gets hotloaded but doesn't like that. The idea there is that most mods should probably be able to support hotloading and if they don't, they'll just not do anything until you restart the game.

There could be a separate variable in the mod that explicitly tells the modloader if it likes being hotloaded but I'm not sure how often that would be used since I can't think of many reasons for a mod to not support hotloading. (if programmed properly with hotloading in mind)
So I'd count missing hotloading support as more of a bug in the mod than something that should be negotiable.

@EIA485
Copy link
Member

EIA485 commented Aug 4, 2022

i dont really see an issue with just using OnEngineInit for mods that dont support hot loading.

@EIA485
Copy link
Member

EIA485 commented Aug 5, 2022

maybe there should be a field in the mod class that a mod can set that overrides nml's config for if it should attempt to late load OnEngineInit

@Psychpsyo
Copy link
Contributor Author

Psychpsyo commented Aug 6, 2022

maybe there should be a field in the mod class that a mod can set that overrides nml's config for if it should attempt to late load OnEngineInit

I don't think there should be. With the new OnInit function being fine for both early and late loading (and a mod being able to distinguish via the passed-in bool) I think that a mod should instead just replace their OnEngineInit() with a OnInit(bool atStartup). It's a one-line change and lets us deprecate OnEngineInit() to not keep lumping around two init functions where we only really need one.

@EIA485
Copy link
Member

EIA485 commented Aug 6, 2022

my reasoning for wanting the loader to know if a mod explicitly does not support late loading is we can have it log that it wont load x mod due to it not supporting late loading

@EIA485
Copy link
Member

EIA485 commented Aug 7, 2022

there should probably be a method or delegate for other mods to know when a mod has been late loaded. this would be useful for mods like the config mod or ExposePatchedMethods to know when to update their objects

@ljoonal
Copy link
Member

ljoonal commented Aug 7, 2022

If we're going to be doing things that deprecate methods, I'd much rather introduce an assembly level attribute to indicate that the mod supports it rather than parameters or class overrides. Since my opinion is that almost all of the NeosMod fields should just be moved to assembly attributes so that they're easier to read (for external programs like auto-updaters & such) and be fully static.

@EIA485
Copy link
Member

EIA485 commented Aug 7, 2022

thats a good point, if a mod explicitly not supporting late loading was an assembly attribute we could check that before loading any assemblies

@XDelta
Copy link
Member

XDelta commented Aug 7, 2022

I agree on using assembly attributes, would allow more options before loading an assembly

@Psychpsyo
Copy link
Contributor Author

I just discovered that this crashes if nml_mods doesn't exist yet (cause it creates the file system watcher on it without checking)
I need to fix that too.

@zkxs
Copy link
Collaborator

zkxs commented Aug 16, 2022

Reading over this, it seems like there are two possible approaches. Either way we add an OnLateLoad() function to NeosMod. The only difference is in the default implementation:

  1. If we want existing mods to late load by default:

    public virtual OnLateLoad() {
      OnEngineInit(); // awful hack for compatibility. Remove in NML2.
    }
  2. If we want to only late load if the mod specifically opts in:

    public virtual OnLateLoad() {
      throw new UnsupportedOperationException("This mod does not support late loading.");
    }

I don't see a need to alter OnEngineInit(), or replace it with some sort of OnInit(bool) function. OnEngineInit was never intended to be a "your mod's init code goes here" method. All it says is "FrooxEngine is initializing now. You wanna do something?"

The assembly attribute is a fine idea, except existing mods won't have it so if we go for option 1 we'll need to load them and call OnLateLoad() anyways.

@zkxs
Copy link
Collaborator

zkxs commented Aug 21, 2022

@Psychpsyo The merge conflict is due to us screwing with all the whitespace in the project (sorry). You can fix it trivially with these commands:

git merge -X ours origin/master
dotnet format

@Psychpsyo
Copy link
Contributor Author

Alright.
I'll get to fixing the rest of this up later today and then I'll try to fix that.

@zkxs zkxs added the enhancement New feature or request label Feb 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants