ArmorMaterial and ToolMaterial interface overlap
The interfaces ArmorMaterial and ToolMaterial have overlapping method names meaning trying to implement both under one singular "material" instance at a time results in the inability to differentiate between the two. This is often desirable as even the vanilla materials have discrepancies between the armor and tool materials (the enchantability of iron armor is different than that of iron tools). Doing so would be a small quality of life improvement over needing a separate ArmorMaterial instance and ToolMaterial instance for every set of custom tools and armor.
The overlap is as follows:
- Both share a method called
getDurabilityhowever the one inArmorMaterialaccepts anEquipmentSlotwhereas the one inToolMaterialdoes not. Technically I guess these two don't clash but I can imagine it is a little bit confusing implementing both on the same class at the same time. - Both share a method called
getEnchantabilitywith the exact same signatures. - Both share a method called
getRepairIngredientwith the exact same signatures.
My recommendation is to rename these methods in their respective interfaces as follows:
-
getDurability->getArmorDurabilityandgetToolDurabilityrespectively. -
getEnchantability->getArmorEnchantabilityandgetToolEnchantabilityrespectively. -
getRepairIngredient->getArmorRepairIngredientandgetToolRepairIngredientrespectively.
I understand that internally within each interface the addition of these clarifiers is redundant, but in actual practice I believe it would be beneficial to differentiate these methods.
I'm creating an issue here because I have literally no idea how to make a PR for yarn and no idea how to suggest mapping changes etc. If someone would like to give me a quick rundown, or if there's a document you can point me to explaining the process I'd be more than happy to go in and make a PR for this.
@sfPlayer1 I recall forge had such issues before and they create extra mappings to fix that. Should we fix this through remapper adding custom bridges instead?