search

ochotonida / curious-armor-stands

MIT License
2 stars 4 forks source link

Curios added via tags, without the curio capability, can not be added to armor stand #2

Closed gigaherz closed 3 years ago

gigaherz commented 3 years ago

I was looking at this mod as a means to have Tool Belt items placed in armor stands for display, but I noticed the logic to equip/unequip on the armorstand relies on CuriosApi.getCuriosHelper().getCurio(stackInHand).ifPresent to run its logic, but curios can be defined via tags, without implementing the capability, which means those items will not get equipped at all even if they work fine on the player curios slots.

gigaherz commented 3 years ago

I realized the mod doesn't use the standard curio slot names in the canEquip, so even when I implemnt ICurio, my mod's system won't find the belt because "armor_stand_curio" is not "belt".

I'm not sure this is the right way for me to approach this feature, as it would require me to add a lot of extra conditions just to support rendering of the belt in armor stands (I don't use the stock rendering from curios).

ochotonida commented 3 years ago

Thanks for the report, I'll get this fixed as soon as possible.

For your own mod, you should generally avoid checking for specific slots. Users can change or add the slot types of items using datapacks, and the 'curio' slot type can hold items of any type. Instead, you can use CuriosApi.getCuriosHelper().findEquippedCurio(item, entity) to find the first equipped item, or you can loop over CuriosApi.getCuriosHelper().getEquippedCurios(player) if you want all equipped stacks (keep in mind that both these methods allocate new objects). For rendering, you also need to consider cosmetic slots & the cosmetic button. You can do that by looping over the equipped curios directly like this: https://github.com/TheIllusiveC4/Curios/blob/b182e7e8a56faf0c00ea62e6852f63e555955995/src/main/java/top/theillusivec4/curios/client/render/CuriosLayer.java#L45-L55

gigaherz commented 3 years ago

Hmmm.... I had always thought of it on terms of it being a belt so it would make no sense for it to be on any slot other than "belt", but thinking about it.... why should I force that? 🤔 If someone wants to make it so the belt can be equipped in the head slot... who am I to contradict them? As for cosmetic slots, I didn't support them at all before, but as part of the refactoring I did to try to support belts equipped in non-player bipeds, it can now query the belt (for display only), from cosmetic curios.

So between the time I started writing this reply, and now, I refactored the curios integration component a bit to query belts from any slot type, meaning this is not an issue anymore. Thanks for the suggestion.

I'm leaving the issue open for the part where you can't insert items defined exclusively via datapack.

ochotonida commented 3 years ago

Should be fixed in the latest patch