-
Notifications
You must be signed in to change notification settings - Fork 92
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
base: master
Are you sure you want to change the base?
Changes from 12 commits
41c9f54
a8d1c21
3d45283
7f96f2c
82f5fb1
723c58e
9e4a3e4
9ef2793
0325ec9
325e81c
48666fd
0656490
8443231
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,13 +110,16 @@ private string GetGlobalSettingsPath() | |
| string globalSettingsFileName = $"{GetType().Name}.GlobalSettings.json"; | ||
|
|
||
| string location = GetType().Assembly.Location; | ||
| string directory = Path.GetDirectoryName(location); | ||
| string globalSettingsOverride = Path.Combine(directory, globalSettingsFileName); | ||
| if (string.IsNullOrEmpty(location)) | ||
| { // TODO: not set while hot reloading | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is kind of annoying. If I'm understanding this feature, if you have a Of course, while reloading, we can't look up the path of the assembly. I think lumafly puts each mod into a directory of the same name, but if you manually copy Maybe this is fine for a niche use case in a development feature.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Settings overrides are generally intended for mod pack distribution, personally I don't mind if we just say this isn't supported with hot reloading but I have no decision power |
||
| string directory = Path.GetDirectoryName(location); | ||
| string globalSettingsOverride = Path.Combine(directory, globalSettingsFileName); | ||
|
|
||
| if (File.Exists(globalSettingsOverride)) | ||
| { | ||
| Log("Overriding Global Settings path with Mod directory"); | ||
| return globalSettingsOverride; | ||
| if (File.Exists(globalSettingsOverride)) | ||
| { | ||
| Log("Overriding Global Settings path with Mod directory"); | ||
| return globalSettingsOverride; | ||
| } | ||
| } | ||
|
|
||
| return Path.Combine(Application.persistentDataPath, globalSettingsFileName); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,24 @@ public class ModHooksGlobalSettings | |
| /// </summary> | ||
| public int PreloadBatchSize = 5; | ||
|
|
||
| /// <summary> | ||
| /// When enabled, listens for filesystem changes to mod DLLs. | ||
| /// On modification, mods will be unloaded, and the new copy of the assembly gets loaded as well. | ||
| /// <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, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| /// make sure to Destroy them in the mod's Unload function. | ||
| /// </description></item>k | ||
| /// <item><description> | ||
| /// Dependencies of mods cannot be hot reloaded. When you modify a dependency DLL, no change will be made to the mod depending on it. | ||
| /// </description></item> | ||
| /// <item><description> | ||
| /// <c>Assembly.location</c> will return an empty string for hot-reloaded mods | ||
| /// </description></item> | ||
| /// </list> | ||
| /// </summary> | ||
| public bool EnableHotReload = false; | ||
|
|
||
| /// <summary> | ||
| /// Maximum number of days to preserve modlogs for. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd ideally give an env variable for this because these paths aren't universal (external drive steam library, flatpak, etc). Or Directory.Build.props. Can still default if it's unset but then people with less standard paths don't have to mess with the checked in csproj
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly,
Directory.Build.Propsautomatically overrides the solution properties, so puttingnext to the solution will just work.
Of course you have to know that this exists. I guess you could leave a comment mentioning it here? Or have a
Directory.Build.Props.examplechecked into git.