When playing with Tinker's Construct and this mod combined, using a tool with the Exchanging modifier (swaps the broken block with your off-hand block) does not work properly, resulting in blocks getting dropped twice or nothing being placed.

• Minecraft: 1.19.2
• Forge: 43.4.0
• Tinker's Construct: 3.8.3.39
• Mantle: 1.10.36
• Multi Mine: 1.19.2.4

Issue also reported on the Tinker's construct tracker: SlimeKnights/TinkersConstruct#5260

Multi Mine does throw a BlockEvent.BreakEvent when it does this, which i assume Tinkers could catch and block

You can ban item tags in multi mine, could that cover the modifier?

No. Modifers are applied via NBT, which applies to ItemStack instances, however item tags apply to Item instances, so you would have to ban all tinkers tools rather than ones with the exchanging modifier.

Whilst looking at the code you linked I also noticed you don't do any input validation on the packet you receive. This is incredibly bad as it means clients can spoof packets to do things such as load far away chunks and crash the server. You should never trust the client.

So a multi mine feature to blacklist arbitrary NBT tags would help?

As for security, fair, but i would guess there is a hundred ways for a malicious modded client to crash a server. I never had security in mind

It sounds like the conflict is not in designs but in a mishandling of some step in the block breaking process, so a programmatic solution would be preferred over a blacklisting workaround, if possible.

The behaviour is broken in singleplayer but not quite as obvious as when playing in multiplayer.
I can create a video if extensive proof is needed.

Nono, i believe you. Both mods hook into block dropping and run their own logic, which results in duping. Suggestion would be to add these blocks to the Multi Mine config blacklist.

I'm not sure if that suggestion would work out, as we use the Exchanging modifier to swap multiple kinds of blocks with another on the fly.

Another mod known as Block Swapper was able to work when combined with Multi Mine (at least in 1.20 - I was hoping to use Exchanging in its place for 1.19), so I'm wondering if there's some kind of specific conflict between the way Tinker's Construct is handling it and Multi Mine is catching it?

Ah, yes then indeed the other mod probably needs to adjust something on their end...

I doubt this bug is purely on Tinkers' side. My code just takes over block breaking. Either your code is running when it shouldn't (as I already canceled normal block breaking), or your code is not canceling other block breaking when it should (as if you change how block breaking works you shouldn't be allowing the original logic to work).

Multi Mine serverside pops a block when a player has reached "block break progress" 100%, see https://github.com/AtomicStryker/atomicstrykers-minecraft-mods/blob/1.19/MultiMine/src/main/java/atomicstryker/multimine/common/MultiMineServer.java#L94

The problem here is likely that Tinkers block swapper is technically an instant block break

Mario, cant you put the block swapper tool on the Multi Mine blacklist?

The way Tinker's Construct's Exchanging modifier works is, you break the block normally with any tool that has the modifier, then once the block is broken the off-hand item is automatically placed where the block was.
It doesn't insta-mine the block, and unfortunately the way it works as a modifier means blacklisting the tools makes Multi Mine completely unavailable outside of vanilla tools.

I still don't get why blacklisting is the only solution. Exchanging is not an instant block break. You fully mine the block, all it changes is the behavior that happens on block break (instead of removing the block it replaces the block with something other than air).

Is there not a way you can call the method on the item stack that handles block breaking instead of hardcoding to always break the block in your code? Basically, seems to me like you are missing a forge hook call.

All of this was many years ago, so i may misremember ...

The basic problem was that Minecraft started tracking block breaking progress both on client and serverside. On serverside 1.19 thats basically net.minecraft.server.level.ServerPlayerGameMode#delayedTickStart which is the tick your mine started, and then it does math to determine how long mining should take and when to destroy the block.

Now, Multi Mine overrides the client side "block progress" and "blocks in progress of being mined" rendering cache. But that still meant server side would desync, as it would disagree on when a block was finished mining. I technically could have hacked/overridden #delayedTickStart at this point but i chose the simpler solution of executing the destroy block call myself, when Multi Mine thinks a block should be finished. Yes, that makes the deciding logic clientside. This mod is OLD, it predates minecraft having actual client/server sides.

It triggers net.minecraftforge.common.ForgeHooks#onBlockBreakEvent which is the central api hook.

The API hook in question that tinkers uses is onBlockStartBreak, which is called inside destroyBlock. Normally this hook is triggered on both sides, but our logic only runs serverside.

The issue description says blocks are dropping twice, suggesting that this method is getting called multiple times instead of just once, and your code happens to work in other cases as you cannot actually destroy air blocks.

I think i get what the problem is now. It's a race condition when performing a vanilla block removal. Vanilla MC sends block destroy progress packets, and Multi Mine sends block destroy progress packets. Multi Mine triggers "destroy block" where vanilla may or may not have already broken the block.

Typically that is not an issue as the block can only be broken once either by vanilla or multi mine, and the next destroy call only hits the air block that is left behind.

But with this block swap, there now is a second block at the same coordinates which can fall victim to the second destroy call.

Hmm OK to avoid this race condition there is a couple of things i could do ... track which block is being broken, and make sure Multi Mine only destroys that one ... have Multi Mine override ServerPlayerGameMode#delayedTickStart ... or make sure Multi Mine never triggers for "vanilla" block breaks, e.g. starting and finishing a block in one go.

Testing showed Multi Mine was always beating the vanilla block destruction, and i decided to go with a simpler approach:
be1d6a6

When Multi Mine destroys a block, it will now prevent any further block destruction at those coordinates for 10 ticks. That should be enough to cover race conditions of this manner. I have pushed new builds for 1.19.4 and 1.19.2 to curse

Thanks for the quick updates, I’ll test the new release today and report my findings.
There is another bug this will likely fix that I’ll outline later as well after testing.

Can confirm that the bug with exchanging is fixed by that change, thanks!