Let's talk mod loading again

[JamiesWhiteShirt]

Let's talk mod loading again

[falkreon]

Implementing ClientModInitializer/DedicatedServerModInitializer may have unexpected effects if implemented in a class that also implements another initializer interface.

How did you expect to implement both ClientModInitializer and DedicatedServerModInitializer without loading classes from the client?

There is also currently no way to declare dependently loaded interfaces for add-on resolution. I imagine initializers used by a mod like JEI might contain environment specific parts.

Oh. Okay. So:

Interfaces are only loaded if you try to make a class that extends from them. (including when you try to make an instance of a class that extends from them, or when you try to Class.forName any of the above). Forge had a heavy ASM system to conditionally strip an interface from a class, but... that's ASM. I am not aware of a way to do that which is not ASM.

So you're going to have to make do with simply not referencing or instantiating classes if they implement an interface, and storing them bare/type-unsafe as Object if you must refer to them, unless you want to get into the mess which even forge is actively moving away from

TehNut has already gotten us 90% of the way there by taking the JEI-style loader and making it generic- plugin-loader. The rest - such as adjudicating between many mods' copies of classes for a given power API - can be solved by jar-in-jar, which is on its way.

[JamiesWhiteShirt]

How did you expect to implement both ClientModInitializer and DedicatedServerModInitializer without loading classes from the client?

It could actually be done, it's just very prone to breakage. No one should, but someone could. However, at the time of writing this issue, environment stripping was not implemented. ClientModInitializer and DedicatedServerModInitializer are now stripped which makes it impossible even if the initializer is loaded.

If we decide implementing multiple interfaces on an initializer is invalid, fabric-loader should make it so and throw an exception. That makes us more dependent on ASM and doesn't validate well with Java.

We know Forge's Optional is not coming to Fabric, so that is not an option.

TehNut has already gotten us 90% of the way there by taking the JEI-style loader and making it generic- plugin-loader. The rest - such as adjudicating between many mods' copies of classes for a given power API - can be solved by jar-in-jar, which is on its way.

Add-on resolution is one of the use cases of the initializer system but the loading behavior leaves a little to be desired. I learned that initializers are not loaded if they implement an interface which is not on the classpath. It's definitely better than trying to load the class. It is similar to the client/server initializer loading. It uses ASM, has the same bug mentioned, and is unintuitive.

If there is a need for a plugin-loader then Fabric needs to improve its initializers or not do add-on resolution at all. IMO we shouldn't have to depend on a separate mod for this.

[asiekierka]

I was considering using annotations on the class for this, but I'm not sure how liked that idea would be.

[asiekierka]

So, here's an odd proposal:

• Deprecate all initializers and just make initializers required to implement either Runnable or Consumer,
• Ensure that all initializers get loaded in order,
• Find a compact way to define initializers with siding requirements.

So we have "scala:[class]" now, as a way to define classes to be loaded explicitly by the Scala language adapter. What if, instead of keying initializers by interfaces, we keyed them by strings?

"client/scala:mypackage.MyClass", say. Now, instead of .getInitializers, you would have .initialize(String key) + .initialize(String key, Object argument) in Loader. (It would also prevent re-entrant calling of initializers, which should NEVER BE DONE anyway)

This is rough and off-the-top-of-my-head, so please discuss!

[asiekierka]

After mild discussion:

"mypackage.MyClass::myMethod" <- keyed to "main", calls static myMethod (must return void, but can have any number of arguments)

"scala/mypackage.MyClass::myMethod" <- same, but for Scala language adapter

"client:mypackage.MyClass::myMethod" <- keyed to "client", yadda yadda

"client:scala/mypackage.MyClass::myMethod" <- etc

Initializers are no longer instantiated per se. If you want instances, you handle them yourself. If you want an API instance, you can either define an initializer key for a kind of proto-dependency injection (any number of arguments, remember?) or a static get()

[JamiesWhiteShirt]

The initializer system has been reworked into entry points in loader 0.4, so this issue is no longer relevant.