Git Product home page Git Product logo

Comments (18)

ChampionAsh5357 avatar ChampionAsh5357 commented on June 15, 2024 1

Besides that the documentation issue regarding optional arguments remains a concern especially for new modders and should be addressed. And it still seems to me that FMLConstructModEvent is way more inline with NeoForge's usual design in favor of the @Mod class constructor.

A bit off topic, but please write an issue on the docs repo when there are documentation issues. I can't address things I can't see or remember, and all it does is leave it incorrect.

The docs does mention the optional arguments, but if you believe it is insufficient, let me know and I can update them properly: https://docs.neoforged.net/docs/gettingstarted/modfiles#javafml-and-mod

from fancymodloader.

shartte avatar shartte commented on June 15, 2024

We're considering removal of this event (or at least, we did). Can you explain your use-case for this?

from fancymodloader.

Fuzss avatar Fuzss commented on June 15, 2024

Just a personal preference for the event style syntax in favor of the constructor. Wouldn't mind the event being removed for streamlining the mod setup process.
Actually there is a use case, see below.

from fancymodloader.

Technici4n avatar Technici4n commented on June 15, 2024

We should have a public getter for the ModContainer IMO. You can grab it from ModLoadingContext right now but it's not the cleanest.

from fancymodloader.

shartte avatar shartte commented on June 15, 2024

What do you actually get from using the event? I might be missing something, but you still have to have the mod class?

from fancymodloader.

Fuzss avatar Fuzss commented on June 15, 2024

Mod setup during construction in multiple places that are not bound to the @Mod class.
For me it's useful for client-only setup in a separate class via marking the class with @EventBusSubscriber(value = Dist.CLIENT).
Examples:
https://github.com/Fuzss/helditemtooltips/blob/main/1.20.4/NeoForge/src/main/java/fuzs/helditemtooltips/neoforge/HeldItemTooltipsNeoForge.java
https://github.com/Fuzss/helditemtooltips/blob/main/1.20.4/NeoForge/src/main/java/fuzs/helditemtooltips/neoforge/client/HeldItemTooltipsNeoForgeClient.java

from fancymodloader.

shartte avatar shartte commented on June 15, 2024

I hate the current way of client-only setup too, but I'd rather allow split mod-classes for that purpose :P
The event still only is fired once per mod-class, making it bound to it 1:1 at least.
And yea, this is not all that great either:

https://github.com/AppliedEnergistics/Applied-Energistics-2/blob/main/src/main/java/appeng/core/AppEngBootstrap.java#L40

from fancymodloader.

Fuzss avatar Fuzss commented on June 15, 2024

For the setup I linked above I just really like that the client setup can be fully separate from common, just like Fabric, where there is ClientModInitializer. I far prefer this over the manual FMLEnvironment.dist check in common that you've linked, which I've also seen in most mods.

And if the goal is to at some point only provide a single method for mod construction I'd honestly rather axe the constructor instead of FMLConstructModEvent. The dedicated construct event is way more inline with how Neo normally works (via events that must be subscribed to and can be used in a decentralised manner, even all the rest of mod loading / setup is done via events and not some poorly documented reflection). The constructor being invoked via reflection really is quite ambiguous regarding what is allowed and what is not, mainly like what arguments are accepted and the order they must be placed in. I haven't found any documentation in Neo itself on that, had to look up the actual implementation in FML to find that e.g. the order does not matter. Just having an event with proper getters and actual documentation makes for a so much cleaner api.

from fancymodloader.

Shadows-of-Fire avatar Shadows-of-Fire commented on June 15, 2024

@Fuzss With the addition of multiple (and dist-specific) mod entrypoints in #126, does this issue report still have merit? The prior usecase appears to be covered by the new features.

from fancymodloader.

Fuzss avatar Fuzss commented on June 15, 2024

Well, the original report is still valid. There remains a disparity between the @Mod class constructor and FMLConstructModEvent. At least turning the ModContainer getter public would be great.

Besides that the documentation issue regarding optional arguments remains a concern especially for new modders and should be addressed. And it still seems to me that FMLConstructModEvent is way more inline with NeoForge's usual design in favor of the @Mod class constructor.

from fancymodloader.

shartte avatar shartte commented on June 15, 2024

Besides that the documentation issue regarding optional arguments remains a concern especially for new modders and should be addressed.

Yes, but new modders will even less likely use FMLConstructModEvent...

from fancymodloader.

Fuzss avatar Fuzss commented on June 15, 2024

Since all other mod loading stages are triggered via events such as FMLCommonSetupEvent, FMLClientSetupEvent, FMLLoadCompleteEvent it just seems much more intuitive and logical for the very first stage to be triggered via an event as well. It's rarely used only because few devs seem to know about the event it seems to me.

But there isn't really an issue here, now that dist can be specified in @Mod. Whichever you guys decide to keep that's just how it is.

from fancymodloader.

Fuzss avatar Fuzss commented on June 15, 2024

Another thing that just came to mind (maybe worth a separate issue): On Fabric client mod initialization is always guaranteed to happen after common mod initialization. This is very useful so that the client part can rely on common stuff to already be available.
On NeoForge with FMLConstructModEvent the order is unpredictable. Not sure if this is any different with dist in @Mod in 1.20.6. It would be great to have a way to run client construction after common. Maybe a new event FMLConstructClientModEvent (and FMLConstructDedicatedServerModEvent respectively)? Or have @Mod with a specified dist be constructed after all classes without by default?

from fancymodloader.

Matyrobbrt avatar Matyrobbrt commented on June 15, 2024

Those kinds of heuristics are very confusing, I would rather have a priority/ordering field in the mod annotation (which would guarantee serial ordering between entrypoints of the same mod). For ordering after the entrypoint of another mod, you need to use the ordering field in the dependency declaration.

from fancymodloader.

Technici4n avatar Technici4n commented on June 15, 2024

The side-specific mod constructor should logically come after the common one. No need for more confusing options.

from fancymodloader.

shartte avatar shartte commented on June 15, 2024

The side-specific mod constructor should logically come after the common one. No need for more confusing options.

Yeah I'd agree with that. Someone had a very odd use-case for wanting to run client before common, but that seems very odd.

from fancymodloader.

Technici4n avatar Technici4n commented on June 15, 2024

This should be resolved once #145 makes it into a NeoForge release.

from fancymodloader.

Fuzss avatar Fuzss commented on June 15, 2024

Great, thanks!

And coming back to the original issue regarding FMLConstructModEvent parameters, what's the conclusion for that?
Can the ModContainer getter be made public?
What's the future of the event, will it be removed now that all shortcomings (multiple mod entry points and specifying sides) of the constructor mod construction method have been addressed?
Can I still use FMLConstructModEvent on 1.20.6 or should I refrain from doing so?

from fancymodloader.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.