search

Super-Santa / EssentialAddons

Fabric Carpet extension that adds things from the Spigot plugin Essentials, or other features I think are needed for Minecraft
MIT License
50 stars 8 forks source link

Initialization order prevents modded entities from getting scarpet events, also spamming logs at runtime #48

Closed altrisi closed 2 years ago

altrisi commented 2 years ago

Carpet registers entity load events for every entity at startup, in CarpetEventServer: https://github.com/gnembon/fabric-carpet/blob/feb0dc1efb3bf97a5aa4741b583bdd9cf4d0c4fb/src/main/java/carpet/script/CarpetEventServer.java#L1017-L1028

The CarpetEventServer class is normally initialized after mods have registered their entities, therefore the loop is able to make events for modded entities and Carpet expects them to be there.

However, EssentialAddons initializes the CarpetEventServer class too early, before most mods have been able to register their entities. The path is: EssentialAddons (clinit) > ScriptPacketHandler (clinit) -> ScriptPacketHandler.PacketEvent (clinit) -> CarpetEventServer

That means that mod entities won't get events, and therefore scarpet apps trying to attach events to those won't work, and carpet will also log the error every time it's unable to find the event, here: https://github.com/gnembon/fabric-carpet/blob/981d89b947fa7b9b07d7298fc231b26b8c486983/src/main/java/carpet/mixins/PersistentEntitySectionManager_scarpetMixin.java#L33

I believe that init path is the issue, though I haven't actually tested whether it is the case. But I've been told it's EssentialAddons who causes this, else it doesn't happen.

senseiwells commented 2 years ago

From what I understand this is an issue with carpet? From reading the JavaDoc for scarpetApi method in CarpetExtension it should be fine to do what I have done?

altrisi commented 2 years ago

No, it's event creation running in the static class initializer of the ModInitializer (links in the path above go to the specific lines that I think cause this).

senseiwells commented 2 years ago

I believe this should now be fixed

altrisi commented 2 years ago

From a quick look that should do it, but I believe now your event will be registered late enough that some apps may fail to find it (it's registered when the constructor is ran, and now that's only ran (from the static field init) when the first packet comes).

senseiwells commented 2 years ago

Yes, you are right, haha, completely forgot that it still needed to be loaded for scripts to use it.