Load modules sorted by dependencies #923

Tharylia · 2023-10-10T10:19:38Z

This PR implements module loading sorted by dependencies.
This is needed for contexts to work properly if the context class is contained inside the module.

Discussion Reference

All new features must be discussed prior to code review. This is to ensure that the implementation aligns with other design considerations. Please link to the Discord discussion:

https://discord.com/channels/531175899588984842/599270434642460753/1160999293361061979

Is this a breaking change?

Breaking changes require additional review prior to merging. If you answer yes, please explain what breaking changes have been made.

No

dlamkins · 2023-10-10T13:18:43Z

Blish HUD/GameServices/ModuleService.cs

@@ -300,7 +300,10 @@ public class ModuleService : GameService {
                    Logger.Warn("Failed to load module from path {modulePath}.", ApplicationSettings.Instance.DebugModulePath);
                }

-                debugModule?.TryEnable();
+                if (debugModule != null) {
+                    debugModule.State.Enabled = true;


This impacts the state even after we're no longer using --module, which is a change in behavior we'll just want to keep in mind.

Before: when no longer using --module, a module would adhere to the state it was previously in.
After: when no longer using --module, the module will be enabled until disabled again.

Blish HUD/GameServices/ModuleService.cs

Blish HUD/GameServices/Modules/ModuleManager.cs

dlamkins · 2023-10-10T13:25:42Z

Blish HUD/Properties/launchSettings.json

@@ -5,7 +5,7 @@
    },
    "PowerShell HUD": {
      "commandName": "Project",
-      "commandLineArgs": "-p powershell -w ConsoleWindowClass"
+      "commandLineArgs": " --process WindowsTerminal --window \"Windows PowerShell\" "


This appears to be completely unrelated.

That's true.. not sure how that got in here. How do I remove that again?

dlamkins · 2023-10-10T13:34:34Z

I did a first-pass lookover.

Other things that need answering:

What is the expected behavior when a module is allowed to start prior to GW2? How does this affect the load order?
What happens when a user manually disables a dependency?
What UI feedback is provided to the user to explain a module not enabled because of a dependency issue? Is it just the dependency section?

Regarding your use case, it's still unclear to me why the dependent module has a different implementation of an assembly being loaded, making the load order important. Can you provide a minimal code example that exhibits this issue so that I can see it?

Load order doesn't explicitly ensure that a module's assemblies are loaded prior to another in the event that the module doesn't attempt to use an assembly first.

Blish HUD/GameServices/ModuleService.cs

Tharylia · 2023-10-10T16:24:42Z

I did a first-pass lookover.

Other things that need answering:

What is the expected behavior when a module is allowed to start prior to GW2? How does this affect the load order?

What happens when a user manually disables a dependency?

What UI feedback is provided to the user to explain a module not enabled because of a dependency issue? Is it just the dependency section?

Nothing, the same as prior to this PR. The dependent module needs to check for this case. In case of contexts: The state of Expired.
No change. It is still the same as prior to the PR. Its only visible in the dependency section. I considered ScreenNotifications but I'm not sure if we want to potentially spam users.

Tharylia · 2023-10-10T16:55:33Z

BlishHUD Module Load Order Test.zip

ModuleB registers a context and ModuleA is dependent on B as it wants to use it.

Steps:

Build both modules and place in modules folder
Start blish and start ModuleB and then ModuleA
See log for INFO | AnotherTest.ModuleA | Found context of module B. entry
Restart blish and wait for module A to load completely ( takes 30 seconds )
See log for INFO | AnotherTest.ModuleA | Could not find context of module B after 30 seconds.
Close blish
Prefix the module file "LoadOrderTest.bhm" with "00_" to make it first in list and simulate a different load order
Start blish
See log for INFO | AnotherTest.ModuleA | Found context of module B.