Fix GT tools not working for some mod actions #1706
Conversation
| private static final Set<String> HAMMER_TOOL_CLASSES = new HashSet<String>() {{ | ||
| add("hammer"); add("pickaxe"); | ||
| }}; |
There was a problem hiding this comment.
Probably better to use ImmutableSet.of("hammer", "pickaxe") or similar here. If the Set itself is mutable then a caller could modify it, and the double-brace approach will also create an anonymous inner class which is probably not desirable.
There was a problem hiding this comment.
I also think these should be immutable from our side to keep them safe. Regarding double braces they have to be addressed too.
| private static final Set<String> HAMMER_TOOL_CLASSES = new HashSet<String>() {{ | ||
| add("pickaxe"); add("hammer"); | ||
| }}; |
There was a problem hiding this comment.
Same thing as other comment, perhaps also should be called JACKHAMMER_[...] rather than HAMMER_[...]
There was a problem hiding this comment.
It uses "hammer" in canMineBlock, so it should be consistent:
There was a problem hiding this comment.
All these if statements could be changed to something like:
return HAMMER_TOOL_CLASSES.contains(tool) ||
It will be slightly less efficient, but easier to maintain in future.
i.e. you won't have subtle bugs introduced by inconsistencies between getToolClasses() and canMineBlock()
There was a problem hiding this comment.
It uses "hammer" in canMineBlock, so it should be consistent:
I clearly meant the name of the constant, not the value of the strings in the Set.
All these if statements could be changed to something like:
Not sure what this has to do with this block of code; perhaps create a separate "conversation" linked to the code you're talking about so we have the appropriate context.
|
Besides the review comments and some more cases of the same issues, the code looks fine. |
What:
This PR fixes GT tools not working in some cases for other mods. For example as detailed in #1704, trees from the Dynamic Trees mod cannot be chopped with a GT Axe or Saw.
How solved:
I overrode
getToolClassesinToolMetaItem, and specified tool classes in each tool type as applicable.Outcome:
Closes #1704
Additional info:
As we are now marking "extra" tools as their proper classes, it is possible that additional compatibility with other mods will achieved. Particularly our Wrench and Crowbar.