Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

RFC: Entrypoint reworks #119

Open
coderbot16 opened this issue Jul 11, 2020 · 4 comments
Open

RFC: Entrypoint reworks #119

coderbot16 opened this issue Jul 11, 2020 · 4 comments
Labels
rfc Requesting comments and thoughts
Milestone

Comments

@coderbot16
Copy link
Member

coderbot16 commented Jul 11, 2020

Patchwork's current entrypoint system needs a rework. I've drafted up some ideas on what that might look like. Please tell me what you think about them!

Introduction

One of the future refactors I have in mind for Patchwork Patcher is to rework the entrypoint system. However, Patchwork API (more specifically, patchwork-dispatcher) would need to be modified to support and use these entrypoints, which is why I'm posting this here. The current single, unsided entrypoint system introduced back in September 2019 is showing its age, and has a few definite limitations:

  • No support for getting the mod instance
  • No (proper) support for sided automatic event bus subscribers (excluding using something like DistExecutor, which is arguably a hack)
  • Classes containing @ObjectHolder and @CapabilityInject annotations are potentially loaded too early (during mod construction rather than after registry creation)

When implementing these entrypoints in Patcher, I intend on adding the methods to existing classes, and then using method references, as opposed to generating separate entrypoint classes.

As such, I propose adding a few different entrypoints:

patchwork:mod_instance

A marker entrypoint to denote classes originally annotated with @Mod. Used get the mod instance. This would be an alternate approach to solve the same problem as PatchworkMC/patchwork-patcher#53. As a side effect, fabric-loader will construct the mod for us when we query the entrypoint, which is convenient.

public interface ModInstance {
    // Loader has a feature similar to this, but since it doesn't support having multiple modids for the same jar
    /// this is still needed. - Glitch
    String getModId();
}

patchwork:common_automatic_subscribers

Register unsided @EventBusSubscribers. Uses ModInitializer. Method name used in method references: patchwork$onInitialize. TODO: Should the automatic subscriber entrypoints use a custom interface that e.g. provides the MOD / FORGE bus in the entrypoint method?

patchwork:client_automatic_subscribers

Register client-only @EventBusSubscribers. Uses ClientModInitializer. Method name used in method references: patchwork$onInitializeClient.

patchwork:server_automatic_subscribers

Register dedicated-server-only @EventBusSubscribers. Uses DedicatedServerModInitializer. Method name used in method references: patchwork$onInitializeServer.

patchwork:object_holders

Register all object holders with the ObjectHolderRegistry here. Fired after new registries have been created, but before registry events have been dispatched.

public interface ObjectHolderInitializer {
	void registerObjectHolders(ObjectHolderRegistry registry);
}

patchwork:capability_injections

Basically the same thing as patchwork:register_object_holders (and fired right afterwards) but for @CapabilityInject.

public interface CapabilityInjectionInitializer {
	void registerCapabilityInjections(CapabilityManager manager);
}
@coderbot16 coderbot16 added the rfc Requesting comments and thoughts label Jul 11, 2020
@TheGlitch76
Copy link
Member

TheGlitch76 commented Jul 11, 2020

What if we used loader's method reference system and instead of consolidating all of the registrations, passing around directly to each class in a given mod?
IIRC i509 had a PR merged that lets you get the modid with out any special things.

@coderbot16
Copy link
Member Author

what if we used loader's method reference system and instead of consolidating all of the registrations, passing around directly to each class in a given mod.

This is essentially what I've proposed, I think:

When implementing these entrypoints in Patcher, I intend on adding the methods to existing classes, and then using method references, as opposed to generating separate entrypoint classes.


IIRC i509 had a PR merged that lets you get the modid with out any special things.

Yup, this is why patchwork:mod_instance being an empty interface for example would suffice.

@TheGlitch76
Copy link
Member

Ah okay, i skimmed too much.

@rikka0w0
Copy link
Contributor

rikka0w0 commented Aug 7, 2020

Just noticed something really important:
Fabric mods are constructed at the very begining of MinecraftClient.init(), but Forge mods are constructed after some init work has been done:

this.options.addResourcePackProfilesToManager(this.resourcePackManager); // line 507
// Construct Forge mods here
// Forge and Forge mods: register custom modelloaders
this.resourcePackManager.scanPacks(); // line 508

image

For example, forge mods may access MinecraftClient.getInstance().getResourceManager() in their constructor, they will get a null and eventually crash.

I would suggest that we should not use ModInitializer.onInitialize for client-side mod construction, we should have our own callback inject to the correct location in MinecraftClient.init().

We may need further discussion on how this change will affect our existing code, but at this stage, I dont think this change will break anything existing code. We have a lot of ModInitializer and ClientModInitializer, all of them are used for fabric event registration, and those events will never be fired so early.

@TheGlitch76 TheGlitch76 added this to the 1.15 milestone Oct 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
rfc Requesting comments and thoughts
Projects
None yet
Development

No branches or pull requests

3 participants