Solve the dependency resolving between mods #27

zrckr · 2026-01-18T02:49:55Z

There's big of PR, based on 78d611e

Essentially, the following changes were made:

The Mod class was split into CodeMod and AssetMod to separate responsibilities.
Loading from Mod was moved to... well... to the HAT mod loader class itself (I think it makes sense to put that here)
Dependency resolving between CodeMods now to done it the separate utility class with a graph structure. I found the old ModDepedency class ugly, since all it have to do is to give correct loading order of mods.
Assembly resolving stayed the same as before.

It was also decided to refactor the HAT mod loader class itself, but leave the public interface unchanged. The reason to make the loading process simpler to track changes in the external debugger.

Loading now occurs in stages:

Obtain the proxy files.
Load only metadata and filter out with blacklist.
Load AssetMods based on metadata and sort them by priority list.
Load CodeMods based on metadata into the dependency graph.
Resolve the graph and load CodeMods in the correct order.

The only question that arises is the rid of minor methods for main initialization. On the one hand, it is useful to see potential errors in the exception window. On the other hand, if you have an external debugger, there is no need for separation if the code is more or less correctly decompiled.

…olver - Split Mod class into IMod interface, CodeMod and AssetMod - Add ModDependencyGraph for explicit dependency tracking - Add ModDependencyResolver with iterative topological sort - Simplify Hat initialization with clearer separation of concerns - Remove ModDependency, ModIdentityHelper and unused None status - Rename Info to Metadata, fix typos in method names

Krzyhau · 2026-01-18T13:16:07Z

Source/Hat.cs

+                _mods ??= _assetMods.Concat<IMod>(_codeMods)
+                    .GroupBy(p => p.FileProxy)
+                    .Select(g => g.First())
+                    .ToList();
+                return _mods;


Does that mean publicly accessible Mods list only has either code mod or asset mod, whichever happens to be sorted in a group first? Not sure if I like that.

I'd suggest making a higher level ModIdentity structure, containing references to both asset mod and code mod, as well as to its metadata and file proxy (this way you could minimize stuff that has to be passed to each asset and code mod on initialization).

Good suggestion. The "glueing" with interface looks not that nice, although it was straight to implement

Krzyhau · 2026-01-18T13:19:39Z

Source/Hat.cs

-                DirectoryFileProxy.EnumerateInDirectory(modsDir),
-                ZipFileProxy.EnumerateInDirectory(modsDir),
+                Logger.Log("HAT", LogSeverity.Warning, 
+                    "Main mods directory not found. Creating and skipping mod loading process...");


Flaw inherited from previous implementation - we're continuing with seeking mods and reading the lists even though the log says otherwise. I'd change the log, since I think it's still worth doing the mod loading process. The result is the same either way.

Krzyhau · 2026-01-18T13:27:33Z

Source/Hat.cs

+            _assetMods = [];
+            foreach (var meta in metas)
+            {
+                if (TryLoadAssets(meta.Key, meta.Value, out var assetMod))


You moved it to Hat.cs, leaving AssetMod empty. I'm very much not a fan of creating a super-class like that. Now Hat.cs contains implementation of loading asset mods, loading code mods, reading metadata, handling ignore- and prioritylist... at least some of these things could be separated into their respective modules.

To re-iterate - I think Hat.cs should only have implementation for managing the mods and the process of loading them. That process itself should be dealt with somewhere else.

Ok, I'll consider to move loading methods back to their classes.

Krzyhau · 2026-01-18T13:30:33Z

Source/Hat.cs

        }

-        private void LogLoadedMods()
+        private static bool TryLoadMetadata(IFileProxy proxy, out Metadata metadata)


Considering my suggestion for implementing ModIdentity structure, this could be neatly implemented in there instead of here.

Krzyhau · 2026-01-18T13:36:45Z

Source/Hat.cs

-                var priorityIndex1 = GetPriorityIndexOfMod(mod1);
-                var priorityIndex2 = GetPriorityIndexOfMod(mod2);
+                var priorityIndex1 = GetPriorityIndex(mod1.FileProxy);
+                var priorityIndex2 = GetPriorityIndex(mod2.FileProxy);


It would be worth to consider using dependency ordering as a secondary sorting comparison for asset mods. Likewise, priority index could be used as a secondary sorting comparison for code mods.

On the one hand, if code mod goes with asset mod, it is better to load assets through the graph alone, so we will be sure that the code will use the correct assets.
On the other hand, adding priority file to the process can complicate everything, because there may be a completely different order of priorities to mess up the graph resolver result.

Krzyhau · 2026-01-18T13:39:28Z

Source/Hat.cs

    {
-        private List<string> ignoredModNames = new();
-        private List<string> priorityModNamesList = new();
+        public static readonly Version Version = new("1.2.1");


I'd still like some kind of indication that a debug version of HAT is being used, but it's a minor request and you can ignore it.

For that case there's VersionString getter for prefixing or suffixing the main version string

Copilot

Pull request overview

This pull request refactors the HAT mod loader architecture by splitting the monolithic Mod class into separate CodeMod and AssetMod classes and introducing a graph-based dependency resolution system. The changes aim to improve the separation of concerns and make the loading process more maintainable with clearly defined stages.

Changes:

Split Mod class into CodeMod and AssetMod with a common IMod interface
Introduced ModDependencyResolver with graph-based topological sorting for dependency resolution
Refactored Hat class to use a staged loading process (file proxies → metadata → assets → code mods → dependency resolution)
Implemented a new assembly resolving system with AssemblyResolverRegistry and pluggable resolvers
Replaced ModMetadata with a new Metadata struct that uses System.Version for better version handling

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 25 comments.

Show a summary per file

File	Description
Source/ModDefinition/Metadata.cs	New metadata struct with Version-based properties replacing string-based version handling
Source/ModDefinition/CodeMod.cs	New class for mods with executable code, handles assembly loading and component initialization
Source/ModDefinition/AssetMod.cs	New class for asset-only mods
Source/ModDefinition/IMod.cs	Common interface for both mod types
Source/ModDefinition/ModDependencyResolver.cs	Graph-based dependency resolution with topological sorting
Source/ModDefinition/ModDependencyGraph.cs	Graph data structure for dependency relationships
Source/ModDefinition/ModDependencyStatus.cs	Removed 'None' enum value
Source/Hat.cs	Refactored with staged loading process and new mod management
Source/AssemblyResolving/*.cs	New assembly resolution system with registry pattern
Patches/*.cs	Updated to use new API (Metadata instead of Info, VersionString instead of Version)
FEZ.HAT.mm.csproj	Disabled nullable reference types

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-18T13:48:32Z

Source/ModDefinition/ModDependencyGraph.cs

+            var name = mod.Metadata.Name;
+            if (_nodes.TryGetValue(name, out var existing))
+            {
+                if (mod.Metadata.Version > existing.Mod.Metadata.Version)


The string comparison operator for Version is being used directly without null checking. If either version is null (which could happen if TryParse fails), this will throw a NullReferenceException. Consider adding null checks before the comparison.

Suggested change

if (mod.Metadata.Version > existing.Mod.Metadata.Version)

var newVersion = mod.Metadata.Version;

var existingVersion = existing.Mod.Metadata.Version;

if (newVersion != null && existingVersion != null && newVersion > existingVersion)

Copilot · 2026-01-18T13:48:32Z

Source/ModDefinition/ModDependencyResolver.cs

+                return ModDependencyStatus.InvalidNotFound;
+            }
+
+            if (hatDependency.MinimumVersion != null && Hat.Version < hatDependency.MinimumVersion)


The version comparison operators are being used directly without null checking. If either mod.Metadata.Version or hatDependency.MinimumVersion is null, this will throw a NullReferenceException. Consider adding null checks before the comparison.

Copilot · 2026-01-18T13:48:33Z

Source/ModDefinition/ModDependencyResolver.cs

+                if (dep.MinimumVersion != null && depNode.Mod.Metadata.Version < dep.MinimumVersion)
+                {
+                    node.MarkInvalid(ModDependencyStatus.InvalidVersion,
+                        $"'{dep.Name}' requires >={dep.MinimumVersion}, found {depNode.Mod.Metadata.Version}");
+                    return;


The version comparison operators are being used directly without null checking. If either dep.MinimumVersion or depNode.Mod.Metadata.Version is null, this will throw a NullReferenceException. Consider adding null checks before the comparison.

Suggested change

if (dep.MinimumVersion != null && depNode.Mod.Metadata.Version < dep.MinimumVersion)

{

node.MarkInvalid(ModDependencyStatus.InvalidVersion,

$"'{dep.Name}' requires >={dep.MinimumVersion}, found {depNode.Mod.Metadata.Version}");

return;

if (dep.MinimumVersion != null)

{

var actualVersion = depNode.Mod.Metadata.Version;

if (actualVersion == null || actualVersion < dep.MinimumVersion)

{

node.MarkInvalid(ModDependencyStatus.InvalidVersion,

$"'{dep.Name}' requires >={dep.MinimumVersion}, found {actualVersion}");

return;

}

Copilot · 2026-01-18T13:48:33Z

Source/AssemblyResolving/AssemblyResolveCompability.cs

+        private static bool CompareVersions(Version checkedVersion, Version requiredVersion, bool allowRollForward)
+        {
+            if (allowRollForward)
+            {
+                return checkedVersion >= requiredVersion;
+            }
+
+            return checkedVersion.Major == requiredVersion.Major &&
+                   checkedVersion.Minor == requiredVersion.Minor &&
+                   checkedVersion.Build == requiredVersion.Build &&
+                   checkedVersion.Revision >= requiredVersion.Revision;


The variable name 'checkedVersion' is inconsistent with the naming convention. Consider using 'providedVersion' or 'availableVersion' to better describe what is being checked against the required version.

Suggested change

private static bool CompareVersions(Version checkedVersion, Version requiredVersion, bool allowRollForward)

{

if (allowRollForward)

{

return checkedVersion >= requiredVersion;

}

return checkedVersion.Major == requiredVersion.Major &&

checkedVersion.Minor == requiredVersion.Minor &&

checkedVersion.Build == requiredVersion.Build &&

checkedVersion.Revision >= requiredVersion.Revision;

private static bool CompareVersions(Version providedVersion, Version requiredVersion, bool allowRollForward)

{

if (allowRollForward)

{

return providedVersion >= requiredVersion;

}

return providedVersion.Major == requiredVersion.Major &&

providedVersion.Minor == requiredVersion.Minor &&

providedVersion.Build == requiredVersion.Build &&

providedVersion.Revision >= requiredVersion.Revision;

Copilot · 2026-01-18T13:48:33Z

Source/AssemblyResolving/HatSubdirectoryAssemblyResolver.cs

+                }
+            }
+
+            return null;


The method returns null when an assembly cannot be provided. According to the AssemblyResolve event documentation, returning null is correct, but the method signature could be clearer if it had a nullable return type annotation (Assembly?). Since nullable is now disabled in the project, this is less critical but still worth noting.

Copilot · 2026-01-18T13:48:37Z

Source/Hat.cs

+            _assetMods = [];
+            foreach (var meta in metas)
+            {
+                if (TryLoadAssets(meta.Key, meta.Value, out var assetMod))
+                {
+                    _assetMods.Add(assetMod);
+                }
+            }



This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Suggested change

_assetMods = [];

foreach (var meta in metas)

{

if (TryLoadAssets(meta.Key, meta.Value, out var assetMod))

{

_assetMods.Add(assetMod);

}

}

_assetMods = metas

.SelectMany(meta =>

{

if (TryLoadAssets(meta.Key, meta.Value, out var assetMod))

{

return new[] { assetMod };

}

return Array.Empty<AssetMod>();

})

.ToList();

Copilot · 2026-01-18T13:48:37Z

Source/ModDefinition/CodeMod.cs

+            foreach (var type in Assembly.GetExportedTypes())
+            {
+                if (typeof(GameComponent).IsAssignableFrom(type) && type.IsPublic && !type.IsAbstract)
+                {
+                    // NOTE: The constructor accepting the type (Game) is defined in GameComponent
+                    var gameComponent = (GameComponent)Activator.CreateInstance(type, [game]);
+                    Components.Add(gameComponent);
+                }
+            }


This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Copilot · 2026-01-18T13:48:38Z

Source/Hat.cs

+            foreach (var meta in metas)
            {
-                var sameNamedMods = Mods.Where(mod => mod.Info.Name == modName).ToList();
-                if (sameNamedMods.Count() > 1)
+                if (TryLoadAssembly(meta.Key, meta.Value, out var codeMod))
                {
-                    sameNamedMods.Sort(CompareDuplicateMods);
-                    var newestMod = sameNamedMods.First();
-                    Logger.Log("HAT", LogSeverity.Warning, $"Multiple instances of mod {modName} detected! Leaving version {newestMod.Info.Version}");
-
-                    foreach (var mod in sameNamedMods)
-                    {
-                        if (mod == newestMod) continue;
-                        Mods.Remove(mod);
-                    }
+                    codeMods.Add(codeMod);
                }
            }


This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Copilot · 2026-01-18T13:48:38Z

Source/ModDefinition/ModDependencyResolver.cs

+                        foreach (var dep in node.Dependencies)
+                        {
+                            if (dep.Status != ModDependencyStatus.Valid && node.Status == ModDependencyStatus.Valid)
+                            {
+                                node.MarkInvalid(ModDependencyStatus.InvalidDependencyTree,
+                                    $"Depends on '{dep.Mod.Metadata.Name}' which is invalid");
+                                break;
+                            }
+                        }


This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Copilot · 2026-01-18T13:48:38Z

Source/ModDefinition/ModDependencyResolver.cs

+                    foreach (var dep in node.Dependencies)
+                    {
+                        if (!visited.Contains(dep.Mod.Metadata.Name))
+                        {
+                            stack.Push(new Progress(dep, false));
+                        }
+                    }


This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Also: - Move the loading methods back to their classes. - Rework HAT.Initialize logic to add skiping the initialization process. - Remove logic for prioritizing of mods (for now).

The priority list now influences the load order of mods while respecting dependency constraints. When multiple valid topological orderings exist, mods are ordered according to their position in the priority list. Dependencies are always loaded before their dependents regardless of priority.

Krzyhau and others added 8 commits January 14, 2026 00:20

feat: reimplement assembly resolving

520649b

fix: prevent nullable type detection by monomod

0e9de15

fix: improve assembly name search and comparison

78d611e

fix: correct deserialization of mod dependency list

09dfcae

refactor: make the mod metadata class as DTO

a40f9be

refactor: use Version class for version comparisons

f27d18a

refactor: simplify the details messaging of resolver result

dca54b6

Krzyhau reviewed Jan 18, 2026

View reviewed changes

Krzyhau requested a review from Copilot January 18, 2026 13:42

Copilot started reviewing on behalf of Krzyhau January 18, 2026 13:42 View session

Copilot AI reviewed Jan 18, 2026

View reviewed changes

zrckr added 2 commits January 19, 2026 17:02

refactor: combine CodeMod and AssetMod with ModIdentity class

ccdffe7

Also: - Move the loading methods back to their classes. - Rework HAT.Initialize logic to add skiping the initialization process. - Remove logic for prioritizing of mods (for now).

-                if (mod.Metadata.Version > existing.Mod.Metadata.Version)
+                var newVersion = mod.Metadata.Version;
+                var existingVersion = existing.Mod.Metadata.Version;
+                if (newVersion != null && existingVersion != null && newVersion > existingVersion)

Solve the dependency resolving between mods #27

Are you sure you want to change the base?

Solve the dependency resolving between mods #27

Uh oh!

Conversation

zrckr commented Jan 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zrckr Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zrckr Jan 18, 2026 •

edited

Loading