Making extension mods work under fabric

[Sturmlilie]

I'm leaving this as a draft PR for now, since it's more for discussion and not meant to be merged in the current state

Corresponding sample extension: https://github.com/Sturmlilie/TIS-3D-Additions

The impulse for thinking about this came when I tried to write a simple "sample extension mod" that would make use of the public font API to prove that it works as intended. I discovered that from my understanding of the code, I could actually not make this work. My reasoning: The intended static entrypoints for registering anything (modules/manual pages/etc) have the following form:

public static void addProvider(final ModuleProvider provider) {
    if (API.moduleAPI != null) {
        API.moduleAPI.addProvider(provider);
    }
}

The order in which mods in fabric are initialized is completely undefined. So if I write an extension that wants to register a module provider and calls this function, whether it actually ends up registered, or just ignored because TIS-3D happened to be initialized later, is a matter of luck.

A solution to this was pointed out to me on Discord: In fabric, you can define arbitrary entrypoints in fabric.mod.json. 99% of mods will just define the standard main (and possibly client/server), which are called by the fabric loader, at a reasonable time (I don't actually now when, but things like registries are assumed to be available). The initialization step looks like this:
loader(minecraft) -> mod

I'm going to propose to use this same method to initialize any extension mods that want to register additional TIS-3D content:
TIS-3D -> extension

Fabric's loader has a very flexible API; we can query a list of all entrypoints declared under a specific string id; for my mockup, I used tis3d:main for the common init code and tis3d:client for the client-side only init code, in order to mirror the loader convention.

In practice, it looks like this: TIS-3D defines ExtInitializer and ClientExtInitializer interfaces (we need both so manual pages don't get registered on servers) that mirror the mod initializer ones. Mods wishing to extend TIS-3D implement these and then declare them in their fabric.mod.json, like so (taken from my sample extension):

"entrypoints": {
    "tis3d:main": [
      "ancurio.tis3dadd.common.Init"
    ],
    "tis3d:client": [
      "ancurio.tis3dadd.client.ClientInit"
    ]
  },

During TIS-3D init, after every API has been setup, we can ask the loader, "give us a list of all entrypoints that have been specified like this, under the following string id (tis3d:main/client)", and simply invoke them. The extension init code can thus completely rely on our API being fully setup with zero guessing involved:

In TIS-3D bootstrap functions:

// Setup all API singletons
// [...]
// Init extensions
final List<ExtInitializer> extensions = FabricLoader.getInstance().getEntrypoints("tis3d:main", ExtInitializer.class);
for (ExtInitializer initializer : extensions) {
    initializer.onInitialize();
}

In any extension mods:

public class Init implements ExtInitializer {
    @Override
    public void onInitialize() {
        // Do stuff with API here, register module providers etc.
    }
}

An interesting side effect of this is that, unless TIS-3D is present, no present extension mod will be initialized (since it doesn't define a toplevel main entrypoint, the loader ignores it). In fact, this pattern looks a lot like one is writing a mod for TIS-3D and not minecraft itself.

Note that we are completely free to define how the ExtInitializer type interfaces look like; I first tested with passing a simple string along and it worked just as well. My proposal though is to not pass anything along, because the convention in Minecraft is already to use global registry-like singletons; an extension would simply use API.moduleAPI.addProvider(...) directly, but it's also not necessary.

Picking up the conversation from before:

So the main gain would be to get rid of the statics and null checks, yes? And I guess the most basic conversion would be having a 'main' API interface with getters for what are now fields in the API class [1], an implementation of which gets passed to [Client]ExtensionInitializer.onInitialize? And then it's the mods' responsibility to keep a reference of that around for future communication?

The main gain really would be that extension mods work at all; removing the null checks can be done but doesn't have to. What's important is that the null checks never return early, if they exist. How the API should be accessed now that it is guaranteed to be initialized is a separate topic where I at most will make suggestions. For example, I think getters might be overblown as the static fields of API are already interfaces whose implementation can be changed anytime. Also, passing interfaces to initializers is overkill IMO due to the registry convention I touched on above. Therefore, no need to split up initializer classes based on which API interface they interact with.

Edit: I wrote this, but in the process of creating my own mod, realized that passing along the API instead of having it exposed via static did make sense after all; it conveys a much stronger message of "you can use this now, it's fully setup, and you also shouldn't ever be able to use this before your own init is called." In fact, in my own mod, I store the passed API reference, and later check that for null to determine whether the API-providing mod is present at runtime.

[1] I feel splitting it up into an initializer per sub-api could be annoying when you need more than one? More flexible in the long run, of course, but not sure that's needed here.

Splitting per sub-api would be overkill, I think the split based on common and client-side is more than enough.

What's the initialization order here then; would mods have to define themselves as being a dependency of TIS-3D for them to be initialized before the extension initialization code gets run? Would Fabric do that automatically because it knows what other mod the interface they're implement is coming from? Would Fabric run the initialization of the mods that declare an implementation of the extension interfaces at the time those are retrieved if that hasn't happened yet? Something else?

I think extensions should still declare a static dependency on TIS-3D in their mod.json, simply for the reason so that users don't wonder why nothing is loaded if they accidentally only put the extension but not TIS-3D into the mod folder (however it wouldn't naturally lead to crashes or even state corruption). The order is directly defined by us, as I've hopefully shown above: We query the extension initializers in our own code from the loader, we directly invoke them at any point we want. The loader itself doesn't care about any entrypoints that aren't main/client/server.

Edit: This comment refers to extension mods which do nothing but add additional TIS-3D content. A mod merely wishing to add some form of intermod compat would not declare such a hard dependency.

[Sturmlilie]

For reference, I implemented a similar system in my mod Duyguji; it's not a pure extension relationship (the api-consuming mod can run on its own), but it initializes the parts of other mods that wish to use the api. By only ever passing the api context object in the initializers, I try to prevent accidental usage before the api is ready. Here's how it looks like on the api-providing side And here's the api-consuming side.

I hope to get around to writing the sample extension mod for this proposal soon, which should serve as a good demo.

[Sturmlilie]

I've updated the sample extension mod to include a two digit hexadecimal display, here it is in action: https://streamable.com/mluf53

Edit: I had to wrangle with the Font API quite a bit (there's just so many layers to go through); maybe that could be simplified in the future? Also note how I am not performing any null-checks on the new font API, because it's implicit they're already initialized.

One thing I don't yet know how to approach is the in-game documentation. The main document linking to all the modules has them hardcoded. Maybe I should just add a separate tab for all of the extension's content?

[Sturmlilie]

I can add a tab just fine, but for some reason I can't make it show my index.md. Specifically, calling

ManualAPI.addTab(new TextureTabIconRenderer(new Identifier("tis3d-additions", "textures/gui/manual_tis3d_additions.png")), "tis3d-additions.manual.index", "%LANGUAGE%/additions/index.md");

with the referenced file being here:

doesn't work. What am I missing?

[fnuecke]

I'll have a look at the technicalities on Sunday! For the manual tab, if you don't already, you probably have to register a content provider first. At least that's what Computronics does.

[Sturmlilie]

Note to self: The Z offset from the block model that every module render method automatically gets should be exposed as a constant / query somehow, I think (I subdivide it if I need multiple layers).

[fnuecke]

So as you may have guessed, I didn't have a look at this on Sunday >_> I got sucked into updating another mod to 1.15... this weekend though, honest!

[Sturmlilie]

Don't worry, take your time, and don't feel the need to make time-based promises, they just cause unnecessary stress =) If you do take a look at things, the 1.16.2 PR would be appreciated though. Also, have you considered adding the fabric category to CurseForge? I know it's a bit controversial because it might make others think the mod is fabric-exclusive. But many people filter by this tag to find fabric mods, and to them, TIS-3D will be invisible otherwise I fear. (CurseForge replacement is direly needed)

[Sturmlilie]

Btw I have a discord, if you wanna hop on to chat: https://discord.gg/A8xHE8P

[fnuecke]

Ehh, all good, I need my deadlines to get things done :P

Fabric category: didn't know that existed! Sure, I'll add it.

So, re the extension stuff, I think I get it now. So the main thing really is the execution order of mod initializers. And if I followed that correctly, you'd lean towards passing an API object along instead of having the statics, to make things less error-prone, yes? So basically make all the static sub-api fields in the main API class instance fields and pass an API instance along I imagine?

One thing that leaves me wondering a little: in your example mod you create the item to register in the extension callback. What if a mod wants to register an item it always creates (in its regular initializer) in a foreign mod initializer, too? This would loop back to that initialization order issue, no? Is there really no mechanism to support inter-mod initialization? Like a "late-init"/"mod-interop-init" or so?

Aside from that, the changes look good to me, but I'm assuming you'll want to change it to pass along the API object, yes?

Edit: oh, regarding discord, I'm in too many servers already, but if you'd like to poke me about going AWOL again, or talk something through more interactively, feel free to PM me! (Sangar#6354)

[Sturmlilie]

Let me preface this by saying that everything regarding mod-interop, extension mods and general the touching of custom entrypoints in fabric seems like a very unexplored territory yet, judging by the community reactions I got to my questions. We have no "best practices" to rely on; rather, we might be the first to explore them.

And if I followed that correctly, you'd lean towards passing an API object along instead of having the statics, to make things less error-prone, yes?

After trying out this method in my mod, I've become a fan of it, so to speak. If TIS-3D was my mod alone, I'd implement it this way, but I assume you'd not want to immediately diverge that far away from what is more customary in Forge (and maybe you'd like to port TIS-3D to a modern Forge version one day); hence my suggestion of keeping the statically-accessed API entrypoints. Rephrasing the matter in this way, I think even leaving in the null-checks would be fine. All I personally care about is safety from the viewpoint of an extending / compat-initializing mod.

So basically make all the static sub-api fields in the main API class instance fields and pass an API instance along I imagine?

Something in that ballpark, yeah. The important part is that the API should not be statically accessible (if we're going with my "radical" idea for how things could work).

One thing that leaves me wondering a little: in your example mod you create the item to register in the extension callback. What if a mod wants to register an item it always creates (in its regular initializer) in a foreign mod initializer, too? This would loop back to that initialization order issue, no? Is there really no mechanism to support inter-mod initialization? Like a "late-init"/"mod-interop-init" or so?

I might not have fully understood. If a mod wants to create an item that always exists, regardless of whether TIS-3D is present at runtime, that would no longer fit into my definition of a "pure extension mod", and fall into the "standalone mod with TIS-3D compatibility / extra content included". If you are referring to this latter case: The mod-intrinsic content / item would be initialized in the normal fabirc main/client initializers. Only the parts (maybe a TIS-3D module which interacts with the item?) would go into the tis3d:main/tis3d:client initializers. The former will always be invoked by the fabric loader, the latter only when TIS-3D is actually present as well.

Aside from that, the changes look good to me, but I'm assuming you'll want to change it to pass along the API object, yes?

I think I'll experiment with them a bit more, using my sample extension. So far things work really well and I haven't found any serious defects either code or design-wise, but my sample also isn't accessing the whole range of APIs TIS-3D theoretically makes available.

Edit: One thing I've been thinking about recently (more in the context of my own mod rather than TIS-3D) is whether it would make sense, and if so, how to version specific entrypoints; eg. on heavy API break, should the initialization move to tis3d:mainv2 or something similar. Going the "entrypoints should remain stable forever and never change"-route would then either imply making sure whatever is passed to the initializers is rock stable and API-break resistant, or fully falling back to static API access.

[fnuecke]

Oh, I'd be perfectly fine with changing the API like that!

Regarding init, yes, that's what I meant. The mod init is called from the providing mod's API though, no? So the mod initializer can't rely on the consuming mod's initializer having run yet?

E.g. TIS3D and mod X. Mod X always adds item T in its initializer, registers it as module in InitializerExt. InitializerExt is called from TIS3D's mod initializer. There's no guarantee that mod X init has happened when InitializerExt runs, no? So item may be null, right?

Edit: l fixed broken email reply formatting.

[Sturmlilie]

E.g. TIS3D and mod X. Mod X always adds item T in its initializer, registers it as module in InitializerExt.

This would indeed not work, and I'm not sure how it would be supposed to work? If item T is always present, then it must be something unrelated, or at least not reliant on any TIS-3D concepts, an item that is useful on its own. How such an item could at the same time be a TIS-3D module is unclear to me. What you'd do is initialize a separate, TIS-3D-only module in the ext initializer that somehow interacts with item T (but on which item T is in no way reliant upon).

[fnuecke]

Ah, I see. I agree that in the context of TIS3D this is an unlikely requirement. I had to look at the API again to realize just how unlikely, oops. Given the in-between provider logic. So I'll agree that this should not matter here.

That said, I can very well imagine cases where this could be relevant. For example, an item that can optionally be charged by some other mod's power system. The item would always exist, but be used when telling another mod that it supports its power system. So then that would be a real problem, and I assumed there should be a solution for that, and if there is I thought it would make sense to use that more flexible approach even when it's not strictly required in TIS3D.

(unless in that case the solution would be something similar to the providers here.)

[Sturmlilie]

Just to be clear, we're talking about a scenario here where mod X provides item T, and in the (optional, from X's viewpoint) presence of mod Y, can be charged by some kind of other item S or block B provided by mod Y, yes?

Here it depends which side is the "dominant one", and which one is the other, "compat-code shipping one". A scenario where a 3rd, purely compat-shipping mod exists is also thinkable, though that's a bit more complicated so I'll ignore it for now. I'll assume mod Y takes the dominant role here.

I think I'm starting the issue you were getting at; if item T implements an IEnergyChareable interface provided by Y, it would still have to be registered in some kind of ChargeableItemRegistry, also created by Y. The registration would have to happen in the specialized initializer invoked by Y, but at this point, item T is not yet guaranteed to exist. So the registry would have to be either statically-initialized (guaranteed to exist before any mod inits run at all), or some callback-type system would be necessary. I'll have to think about this a little bit more.

[fnuecke]

Just to be clear, we're talking about a scenario here where mod X provides item T, and in the (optional, from X's viewpoint) presence of mod Y, can be charged by some kind of other item S or block B provided by mod Y, yes?

Yes, exactly.

And yeah, that's what I'm getting at :) Though as I was writing this down in my previous message, I realized that given the "right" choice of API design, this issue could be circumvented -- e.g. by using a provider logic as TIS-3D does. I.e. have an intermediary class instance that gets queried later on in the game when the item it would check for has definitely been registered, and have that intermediary be registered with that -- optional -- other mod.

Soo... I may have already answered my own question. Though it would be interesting what Fabric's "official" stance on this topic is!

[Sturmlilie]

Yeah. I've thought about it a bit more, and I think the "compat code for an element must be initializable before the element itself is created" concept is key. The reason is because it's unlikely that fabric will ever support top-level mod initialization ordering, and personally I think that's a good choice, reason being that ordering makes cross-compat impossible. By cross-compat I mean, "mod X invokes compat-initializing code in mod Y, and mod Y invokes compat-initializing code in mod X". There is no ordering that will ever make this work implicitly, so rather, we have to explicitly work with that in mind. The "provider" system is a good solution IMO. Going back to the item example:
Mod X has something like

public class TItem extends Item, implements IEnergyChareable {
...
}

public class XItems {
    public static TItem t_item = null;
}

and this is initialized in main like you'd expect:

...
XItems.t_item = new TItem();
...

in y:main, the compat code uses indrection instead of referring to the item directly:

YChargeableRegistry.registerProvider( () -> return XItems.t_item );