Minecraftforge: ItemAxe subclasses are impossible.

Created on 24 Mar 2016  路  10Comments  路  Source: MinecraftForge/MinecraftForge

In the ItemAxe constructor, the attack damage+speed are looked up in a hardcoded array indexed by the input material ordinal. Which means any EnumHelper added materials will immediately crash the game with AIOOBE since their indices are larger than the array -.-

Easy fix would be to just set them to 0 if the range is greater than the ordinal for diamond (or whichever vanilla one is last) and let the mod set it in their ctor

Most helpful comment

I'm not sure why mojang special cased these particular values.
The acceptable way to fix this is to just add another constructor to ItemAxe that takes in these values.
Its a dirty dirty hack but whatever. How many locations explicitly check the instanceof? Looks like 4. It may actually be better to patch those into .isToolType(Types.AXE) {In theory we should move twards using a enum as it's a faster compare == vs .equals}

Ya.. I think that'd be best. Add a Enum with a getOrCreate {similar to what we have for Biome type in the BiomeDictionary} And switch all the places where we use String toolType to Enum.
As I said, this would increase performance. Which would make the isToolType(AXE) replacements of instanceof ItemAxe better.

So two options: New constructor to bypass the array herpa
Or switch to a new system and patch those 4 instanceof checks.

All 10 comments

the proper fix as told to me is to extend ItemTool directly and set the tool class to axe

The problem with this approach is that you can't set the toolClass field, meaning you have to implement both getHarvestLevel and getToolClasses. Maybe the toolClass field could be protected in stead of private.

Yes, there are further problems with being unable to do this, reopening.

  1. What's been mentioned above
  2. This is more critical. Not subclassing ItemAxe means that the unique mechanic they have on shields cannot be inherited (it uses instanceof ItemAxe) without copy pasting the vanilla code into 3 separate event handlers. We need to look at finding a solution for this

I may PR a fix for this shortly

  1. setHarvestLevel.
  2. That should be patched to use the tool classes instead then.

it is checked in over 4 separate places

I'd rather just patch this one ctor than in 4 other places, but I get patching the check as well
@LexManos your thoughts on this?

3 places, one of the 4 is the ItemTool constructor, which doesn't count. And even if you patch the constructor here, you still have to patch the 3 other places, because forge's axe tool-class should behave like actual axes, otherwise it's pointless to have the tool class.

yea I'll probably patch the instanceof checks into tool classes then

I'm not sure why mojang special cased these particular values.
The acceptable way to fix this is to just add another constructor to ItemAxe that takes in these values.
Its a dirty dirty hack but whatever. How many locations explicitly check the instanceof? Looks like 4. It may actually be better to patch those into .isToolType(Types.AXE) {In theory we should move twards using a enum as it's a faster compare == vs .equals}

Ya.. I think that'd be best. Add a Enum with a getOrCreate {similar to what we have for Biome type in the BiomeDictionary} And switch all the places where we use String toolType to Enum.
As I said, this would increase performance. Which would make the isToolType(AXE) replacements of instanceof ItemAxe better.

So two options: New constructor to bypass the array herpa
Or switch to a new system and patch those 4 instanceof checks.

Was this page helpful?
0 / 5 - 0 ratings