Make Rich Soil use a HashMap for Mushroom Colonies#1089
Make Rich Soil use a HashMap for Mushroom Colonies#1089Ultrasquid9 wants to merge 5 commits intovectorwing:1.21from
Conversation
|
i suport this |
| public static HashMap<Item, MushroomColonyBlock> COLONIES = new HashMap<>(); | ||
| public static final int PLACING_LIGHT_LEVEL = 13; | ||
| public final Holder<Item> mushroomType; | ||
| public final Item mushroomType; |
There was a problem hiding this comment.
This is not a proper change, Holder should be used as addons could register mushroom colonies for their own mushrooms, where the item is registered after the blocks and thus not available in the block's constructor.
Also, the COLONIES field should not be exposed, use proper register and get with checks for duplicate registers, also since the Item is not available in the constructor, do not register to the COLONIES in the constructor, move it FMLCommonSetupEvent instead.
Since we are moving away from hardcoding, let's not restrict the value class to MushroomColonyBlock, and the key class could just use Block instead of Item since we are not doing the register in MushroomColonyBlock's constructor anyways.
.gitignore
Outdated
| *.iml | ||
| .idea | ||
|
|
||
| # vscode |
There was a problem hiding this comment.
Not sure what occurred in your workspace, but I'd rather leave the .gitignore unchanged. Can you please revert this edit?
| }; | ||
|
|
||
| public MushroomColonyBlock(Holder<Item> mushroomType, Properties properties) { | ||
| public MushroomColonyBlock(ItemLike mushroomType, Properties properties) { |
There was a problem hiding this comment.
I don't think it's good to use ItemLike as the key, the Item, Block, DefferedItem and DefferedBlock all matches, but neither of them are equal to others by implementation.
Since you are using the Item to get value from the map, a modded colony which passed in a DeferredItem (or DefferedBlock) as the mushroomType parameter will simply fail when checked by the Rich Soil.
Please just revert this to use the Holder<Item> as the parameter type and key type, Holders are basically singleton and has proper equals and hashCode implementation on NeoForge, even on Fabric the Holders should still work totally fine as long as it's the Holder.Reference from registry.
There was a problem hiding this comment.
Alternatively just move the registration away from the constructor as requested above (the Apr 7 one), which would eliminate the need of laziness of the key.
This allows addons to add their own Mushroom Colonies without needing to mixin
RichSoilBlock.This also changes the
MushroomColonyBlockto use anIteminstead of aHolder<Item>, as thebuiltInRegistryHoldermethod is deprecated and using anItemworks fine.