Redo asset loading, add a proper loading screen - #80
Conversation
Mod assets are now loaded one by one instead of all at once. Introduced more APIs for fine-grained control over asset loading.
I noticed that this PR is removing the way it was previously separating things into tasks and loading them after. I think that's fine, but since we're doing that, it actually lets us clean things up and simplify things a little more, so now instead of adding all the assets afterwards, we're able to just do it in place while loading the assets. I also removed the LoadDirectoryRecursive function, because it didn't really feel needed after simplifying. I did have to change the order a little to accommodate this, because it means shaders must be loaded before models. I also moved the modFs null check to the beginning, since every asset type was checking that, so it's simpler to just do that first. I also noticed a bug where because of these changes, every mod was adding the vanilla skin. I reworked this, so the vanilla skin is now just set up when we create the vanilla mod. I also fixed a minor bug, where I think we need to call `Language.Current.Use();` after all mods are loaded, in case any new characters are used that need to be registered. I also changed it to show the mod name during asset load, instead of the Id. Since this is something that's displayed to the player, I think using the mod name is cleaner here. Aside from that, I also made a few minor formatting and naming tweaks.
The main optimization here is that we only reload assets for the mod whose assets have been changed. This saves us plenty of time that we spent waiting for unnecessary asset reloads before.
…a setting is changed. Previously if a mod was enabled or disabled, or if a mod setting was changed that was marked as needing reload, it would always reload all mods. But because of these recent changes, we can change that to only reload mods that were actually touched, which should make that reload happen faster. I also changed it so disabled mods don't trigger a hot reload if a file is changed, since that file isn't going to be loaded anyways. And I also added some code to clear out hooks on reload as well.
…eload single mods Mod dependencies should now be reloaded as well when a mod is changed through hot reloading or through the settings. It also should get handled better when a mod is enabled or disabled and the dependents and dependencies get disabled/enabled as well. It should load the dependencies similarly to how it does during initial load, so dependencies should get loaded before the mods that depend on them. I also updated it so enabling and disabling mods should recursively affect dependencies and dependents. Previously, it only went 1 level up or down.
jasminegamedev
left a comment
jasminegamedev
left a comment
There was a problem hiding this comment.
I think I am pretty satisfied with where this is at right now. I've just got one small comment, and if we can get that addressed, then this can probably be merged.
| public World? World => Scene as World; | ||
|
|
||
| internal bool NeedsReload = false; | ||
| internal bool doUpdate = true; |
There was a problem hiding this comment.
Is this actually needed for anything? It seems to not be doing anything currently, since it's always true
There was a problem hiding this comment.
It's a leftover from an implementation I was working on and subsequently scrapped. It can be safely removed
|
Resolved merge conflicts |
|
|
||
| Log.Info($"--- User has initiated a{(Input.Keyboard.CtrlOrCommand ? " full" : String.Empty)} manual reload. ---"); | ||
| ReloadAssets(Input.Keyboard.CtrlOrCommand); // F5 - Reload changed; Ctrl + F5 - Reload all | ||
| } |
There was a problem hiding this comment.
We actually kind of already have a hotkey for reloading assets, where if you press Ctrl+R it will also call the ReloadAssets function. This actually comes from the base game.
It's a little bit different, because it doesn't let you filter to changed vs all like this, but we probably don't need two different hotkeys for this, since they're kind of doing the same thing.
There was a problem hiding this comment.
I'd be fine with replacing vanilla's ctrl+r with this implementation.
|
I think this seems good to me. It is a little bit of a critical area, so we should definitely keep an eye if people run into any asset loading bugs that come out of this, but this is probably good for now. |
2 participants
Introduces a substantial overhaul of the way asset loading is done:
Additional features:
Things left to do: