You're registering stuff at the wrong time
jaredlll08 opened this issue ยท 1 comments
Issue description:
You're registering the following at the wrong time:
Rebuilding a cache too late:
https://github.com/aidancbrady/Mekanism/blob/1.12/src/main/java/mekanism/common/Mekanism.java#L657-L663
that cache should be built in postinit, it is the last stage that recipes can be changed for them to still appear in JEI, and is used in many mods as a "well now that everything is done, lets do stuff to the registered things", maybe just build the cache during that stage instead of rebuilding it?
This is just a thought:
Shouldn't this code be in FMLServerStoppedEvent?
https://github.com/aidancbrady/Mekanism/blob/1.12/src/main/java/mekanism/common/Mekanism.java#L669-L691
Doing it this way, if a mod is using FMLServerStopping properly, and taking long to do so, it may cause issues if those lists are still in use or something? Just seems a tiny bit risky to clear them when the server is still technically running.
not really an issue, but just naming pls
https://github.com/aidancbrady/Mekanism/blob/1.12/src/main/java/mekanism/common/Mekanism.java#L694
that method name is completely wrong.
Events should be done in preinit
https://github.com/aidancbrady/Mekanism/blob/1.12/src/main/java/mekanism/common/Mekanism.java#L759-L760
https://github.com/aidancbrady/Mekanism/blob/1.12/src/main/java/mekanism/common/Mekanism.java#L772
https://github.com/aidancbrady/Mekanism/blob/1.12/src/main/java/mekanism/common/Mekanism.java#L840
Entities should be registered via the event:
https://github.com/aidancbrady/Mekanism/blob/1.12/src/main/java/mekanism/common/Mekanism.java#L800
According to forge docs, this should be done in Preinit, but that seems stupid, I personally would move this to the Register event or init
https://github.com/aidancbrady/Mekanism/blob/1.12/src/main/java/mekanism/common/Mekanism.java#L837
Lex has mentioned that TileEntities should be registered with blocks in the Register event
https://github.com/aidancbrady/Mekanism/blob/1.12/src/main/java/mekanism/common/Mekanism.java#L617-L642
Not too big, but the mod isn't fully loaded at this state
https://github.com/aidancbrady/Mekanism/blob/1.12/src/main/java/mekanism/common/Mekanism.java#L829
This stuff should happen later, right now this gets called as soon as the Mekanism class is loaded, so FMLConstructingEvent essentially, I would change it to at least preinit, maybe init.
Version (make sure you are on the latest version before reporting):
Forge: NA
Mekanism: 1.12 branch
Other relevant version:
Rebuilding a cache too late:
Not really a cache, but I'm not sure why its done then.
Events should be done in preinit
A) not according to https://mcforge.readthedocs.io/en/latest/conventions/loadstages/#initialization
B) they are player events, it literally doesnt matter
C) they're so simple i might even just move them to the static annotation at some point
Shouldn't this code be in FMLServerStoppedEvent
Those lists shouldn't be changed at that point, unless the server doesn't actually stop
According to forge docs, this should be done in Preinit, but that seems stupid, I personally would move this to the Register event or init
Nope, that's to do stuff after other mods have done all their stuff. It's not adding to the oredict.
Lex has mentioned that TileEntities should be registered with blocks in the Register event
As long as its done before world load its a non-issue
This stuff should happen later, right now this gets called as soon as the Mekanism class is loaded, so FMLConstructingEvent essentially, I would change it to at least preinit, maybe init.
it's a static list, why does that matter?