If EnderZoo is installed, Khendrel Knights spawn when dirt or grass is harvested by mattock.
happysmash27 opened this issue ยท 12 comments
Issue description:
If EnderZoo is installed, Khendrel Knights spawn when dirt or grass is harvested by mattock. According to this GitHub issue, other people's mods should implement a fix instead of EnderZoo? Maybe @HenryLoenwind will understand better.
Versions:
- Minecraft: 1.12.2
- Forge: 8.0.99.99
- Mantle: 1.3.3.55
- Tinkers Construct: 1.12.2-2.13.0.183
Can it be reproduced with just Tinkers Construct? If not, list the other mods required to reproduce the issue.
EnderZoo.
From my understanding, khendrel knights spawn when you use an incorrect tool on dirt. Mattocks are a correct tool for dirt, so it shouldn't cause them to spawn. I would argue Forge's hook is ineffective here as it clearly misses that not everything that can mine dirt is a shovel. And vanilla has a perfectly acceptable method which we implement (you can ask the tool if its effective on the block), so I see absolutely no need for the Forge hook to exist in this case.
As for special casing our dirt, why do you need to, and why has this never been brought up to us before today? If you would have asked, we could have solved the problem so you do not need to special case our dirt.
Now, as for a fix. We could mark the mattock as a shovel, but that would be wrong as its not a shovel. Being a shovel gives it a lot of extra behavior we don't want, notably it does not mine every dirt type block and cannot make paths.
You can mark mattocks as one of the acceptable tools you check for, but that just goes to highlight the flaw with the Forge hook.
You can call the other Forge method,
getHarvestLevel, which will properly return -1 for ineffective tools or 0+ for our tools/vanilla tools. No special casing required, will support all odd tools you encounter.
So, this could all be solved, by changing literally a single line of code in EnderZoo:
if (state.getBlock().isToolEffective(type, state) || "shovel".equals(type)) {
to:
if (state.getBlock().getHarvestLevel(state) || "shovel".equals(type)) {
?
No, because getHarvestLevel returns an integer. You need to ensure its 0 or more, in other words >= 0. You also need the item stack method, not the block state method.
Closing as 1.16 received a beta making 1.12 end of support.
For the record, I still believe Tinkers' Construct is doing tools correctly here and Ender Zoo is using the wrong method to check if a shovel can mine a block. So ultimately its there issue to fix. I get that forge has a confusing method that seems to do the job, but the stack method I mentioned should really be used.
net.minecraftforge.common.ForgeHooks.isToolEffective(IBlockAccess, BlockPos, ItemStack)
What is the thing you're checking for @HenryLoenwind? I don't get the context. Under what circumstances should it not be effective (or is it wrongly effective)?
@SubscribeEvent
public static void onBlockHarvest(HarvestDropsEvent event) {
if (...
|| !(event.getState().getBlock() instanceof BlockDirt || event.getState().getBlock() instanceof BlockGrass)) {
return;
}
if (!isToolEffective(event.getState(), event.getHarvester().getHeldItemMainhand())
&& ...) {
EntityDireSlime direSlime = new EntityDireSlime(event.getWorld());The Mattock on Dirt triggers this. That's nothing new and I'm quite sure there's at least one old issue for this already, I just responded to the ping to make it easier to see what this is about.
It looks like the mattock is not internally a shovel type, its type is mattock, but it overrides getHarvestLevel to be able to harvest the specific blocks (if I remember, not every shovel/wood block is considered a mattock block, plus too many mods assume shovel means you can make paths). Any reason you don't use the method on the tool where it claims its effective on a block instead of relying on the shovel class?
KnightMiner: There are two ways to check, asking the tool or asking the block. Unlike with other mechanisms those checks don't execute the other check also. I decided to use the check that Forge offers with ForgeHooks.isToolEffective() (I already had to copy that code to special-case Tinkers dirt...) because I try to follow Forge's guidelines on how to do things as long as possible for maximum cross-mod compatibility.
From my understanding, khendrel knights spawn when you use an incorrect tool on dirt. Mattocks are a correct tool for dirt, so it shouldn't cause them to spawn. I would argue Forge's hook is ineffective here as it clearly misses that not everything that can mine dirt is a shovel. And vanilla has a perfectly acceptable method which we implement (you can ask the tool if its effective on the block), so I see absolutely no need for the Forge hook to exist in this case.
As for special casing our dirt, why do you need to, and why has this never been brought up to us before today? If you would have asked, we could have solved the problem so you do not need to special case our dirt.
Now, as for a fix. We could mark the mattock as a shovel, but that would be wrong as its not a shovel. Being a shovel gives it a lot of extra behavior we don't want, notably it does not mine every dirt type block and cannot make paths.
You can mark mattocks as one of the acceptable tools you check for, but that just goes to highlight the flaw with the Forge hook.
You can call the other Forge method, getHarvestLevel, which will properly return -1 for ineffective tools or 0+ for our tools/vanilla tools. No special casing required, will support all odd tools you encounter.
And the mattock is effective on grass or dirt. Still it's not a shovel. So I'd say everything is working as intended? I don't know what Khendrel knights do, so why would that even be an issue if I use a mattock or a shovel?