Fixed NullPointerException on startup
PLEASE READ THE GUIDELINES BEFORE MAKING A CONTRIBUTION
- Please check if the PR fulfills these requirements
- [x] The commit message are well described
- [x] Docs have been added / updated (for features or maybe bugs which were noted). If not, please update the needed documentation here. Feel free to remove this check if you don't need it
- [x] All changes have fully been tested
-
What kind of change does this PR introduce? (Bug fix, feature, ...) Bug Fix
-
What is the current behavior? (You can also link to an open issue here)
When trying run minecraft 1.20.4 with neoforge and AP installed
Loading throws ´java.lang.NullPointerException: Cannot invoke "net.neoforged.neoforge.registries.DeferredHolder.get()" because "de.srendi.advancedperipherals.common.setup.BlockEntityTypes.ME_BRIDGE" is null
is thrown
This is because APAddons::commonSetup is called after Registration::register
public static final DeferredHolder<BlockEntityType<?>, BlockEntityType<MeBridgeEntity>> ME_BRIDGE = APAddons.ae2Loaded ? Registration.BLOCK_ENTITIES.register("me_bridge", () -> new BlockEntityType<>(MeBridgeEntity::new, Sets.newHashSet(Blocks.ME_BRIDGE.get()), null)) : null;
the issue is, that APAddons.ae2Loaded has not been initialized so it defaults to false
Default Value: The default value of a boolean variable is false
- What is the new behavior (if this is a feature change)?
Whenever something checks if a mod is loaded, the ModList will directly be checked
- Does this PR introduce a breaking change? (What changes might users need to make in their scripts due to this PR?)
i dont think so :D
- Other information:
I think we can put that to static class init block instead of commonSetup?
Why not use ModList.get().isLoaded(....) where APAddons.....Loaded is used?
I wonder if it may has less performance than check a boolean, but I may over thinking it.
I wonder if it may has less performance than check a boolean, but I may over thinking it.
Well ModList.get() just returns its singleton instance; private static ModList INSTANCE .isLoaded just runs a containsKey on a map
so i dont think that this will have any performance impact
I wonder if it may has less performance than check a boolean, but I may over thinking it.
Maybe a bit. But saving that as a variable in a field like we do it currently is nothing harder to do than calling ModList... everytime. So I just prefer caching the mod loaded state like we do it currently and just move the init of that class to an earlier state
Putting that into a static class init would also be a solution, it just loads them when it is needed. I always think they are ugly, but it's a good solution
But we already just use the same method, just renamed it and moved it to the top of the constructor in 1.21 to fix that issue. If there is no particular reason to move to the static init, this should be done for 1.20.4 too.