HypixelDev / FabricModAPI

Implementation of the Hypixel Mod API for the Fabric Loader.
MIT License
10 stars 3 forks source link

Add support for Fabric's event system #2

Closed AzureAaron closed 6 months ago

AzureAaron commented 6 months ago

It would be amazing if an alternative system to listening for packets was added with Fabric's native event system because the only way to register handlers to receive packets is through the official Mod API itself which is highly inconvenient for a number of reasons:

Since this library is specifically for Fabric, an alternative that uses Fabric's Event System should be offered and such an event should supply a ClientboundVersionedPacket rather than the type-specific ones which has a number of benefits that enable far greater flexibility and developer convenience:

Implementing this solution would only require creating a class with the necessary functional interface and event, and then invoking the event once a packet has been received.

While it does seem the intention of this library is for end-users to install it, that won't be the case; In order to properly depend upon this developers would need to add a statement to the depends part of their FMJ, and then to use the Modrinth Maven (unless this gets published to an official Maven) to import the library into their development environments so that trying to use runClient doesn't break or cause malfunctions due to a missing dependency. Many Fabric developers including those who make Hypixel related mods do conduct testing of their features on the server via runClient so this isn't a special or uncommon situation.

Given that developers will be importing this as a dependency, adding an additional integration with Fabric's event system would be highly appreciated and will greatly help developers interested in using this library.

ConnorLinfoot commented 6 months ago

The current idea with the implementation of the Mod API is to keep it simple and something that can work across different versions and loaders. For example, right now we can tell developers to depend upon the core Mod API, and regardless of whether they are using forge or fabric, the implementation itself is identical. 

There is also potential added overhead to implementing more loader-specific features, such as an event system. While I'm not super familiar with Fabric's system, I know in the past there have been major changes to mod loader APIs that have led to updating to newer versions being more of a chore, and this may result in some extended time between updates if we take this route. We also have an added inconsistency where you can either, use the Mod APIs implementation, or you can use the Fabric implementation, which personally feels a little odd.

We're not hard against implementing some things that can make it easier for specific mod loaders, but it would need a lot of consideration on how we move forward with the overall implementation and having more developer resources to both implement and maintain these, unless developers wish to contribute potential changes themselves.

As for the issue of using the API locally, I personally had it work with the mod being in the mods folder of my development environment. But if there is a specific way that this is normally handled, then we can look into that. If you have any further information on it, that'd be great.

(I'll keep this issue open for now to allow some continued discourse, but we may look at moving it to discussions or something depending on how we choose to maintain these repos.)

ConnorLinfoot commented 6 months ago

@AzureAaron Is this the sort of thing you were thinking? https://github.com/HypixelDev/FabricModAPI/pull/5. My personal knowledge with Fabric isn't much at the moment so let me know if this is what you were asking for or if I'm missing something

AzureAaron commented 6 months ago

@AzureAaron Is this the sort of thing you were thinking? #5. My personal knowledge with Fabric isn't much at the moment so let me know if this is what you were asking for or if I'm missing something

Yeah this was what I was thinking, a simple Fabric callback event for listening to received packets that developers can register to. It looks good.