Skip to content

Basic hot reload support for mod development #160

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

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

Conversation

jakobhellermann
Copy link
Contributor

Similar to how BepInEx/ScriptEngine works.

When a file is changed, the old loaded mod will be Unloaded (only works for IToggleable mods) and the new version of the assembly is loaded next to it. To fix conflicts, the assemblies name gets modified to $originalName-$currentTimestamp using Cecil.

Configurable via EnableHotReload.

/// <para><b>Limitations:</b></para>
/// <list type="bullet">
/// <item><description>
/// The old assembly does not get unloaded. If you have created any unity game objects or components,
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems rough, not because of needing to destroy stuff in unload but because it makes a reload costly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Costly compared to what?

The regular use case should be unchanged, even with EnableHotReload enabled, and only subsequent changes will incur the extra assembly overhead.
And I think even that overhead is not too bad, when developing with scriptengine in nine sols I rarely had to restart the game because it became laggy after too many hot reload cycles.

string mods = Path.Combine(ManagedPath, "Mods");

string[] modDirectories = Directory.GetDirectories(mods)
.Except([Path.Combine(mods, "Disabled")])
Copy link
Contributor

Choose a reason for hiding this comment

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

Guessing we are not using modern enough build tooling for c#13 to be supported here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can of course revert to the old syntax, it's not a big deal.

However, I think that this is a syntax-only feature and I've been using it locally without problem, so if I were to simply bump dotnet from 6.0 (end of life since November 12, 2024) to 8.0 in the github actions, everything should work even on the old net472 runtime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I can't actually make that change in this PR, since the action from main gets used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BadMagic100
Copy link
Contributor

If you referenced bepinex.scriptengine heavily, make sure everything is on board in terms of licensing. It looks like it probably isn't because

  1. At the very minimum you need license text and copyright attribution which I don't see
  2. They use lgpl which is more restrictive than the mit license we use

@jakobhellermann
Copy link
Contributor Author

If you referenced bepinex.scriptengine heavily

I don't think I did?

I briefly read their code, but wrote the code here myself.
In the end there's a few lines of the same boilerplate for the FileSystemWatcher or Cecil AssemblyDefiniton but nothing I'd consider copyrightable.

I can of course still add comment crediting the original code for the inspiration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants