fireBlockHarvesting called multiple times on the same block if builder's inventory is full
LordRW opened this issue ยท 14 comments
Issue description:
Builder with a clearing quarry card was quarrying with a single chest on top powered by 180rf with ledstone fluxducts, the chest became full: when suddenly my base was quite literately filled with about 1000 silver fish. they kept spawning at a pace that seemed like the time it would take for the builder to break a block with the power that was supplied. the silverfish spawned out of the builder block.
went in to peaceful mode, the silverfish continued trying to spawn, when going back in to normal they continued, clearing the storage chest of items fixed the issue.
my assumption is that the builder was going to break a block that had a silverfish inside, when it had no inventory space inside the chest it just repeatedly probed the block spawning silverfish somehow.
later the builder was replaced by ballistic magma when put mine blocks 25000 blocks away after mining some blocks
Steps to reproduce:
none, it quite literally filled my base and every cobble stone block with silverfish.
Versions:
- Minecraft: 1.12.2
- Forge: 14.23.2.2618
- CompatLayer (only if on Minecraft 1.10 or 1.11):
- McJtyLib:1.12-2.6.5
- RFTools: 1.12-7.30,
Single player instance of lost souls.
pack: Forever stranded lost souls v. 1.0.3.10-b3
I don't think this is related to the builder. I tried attaching a builder to a full chest and having it try to quarry a monster egg, but this didn't happen to me. Also, I checked the builder's code, and the way that it breaks blocks looks like it will never produce a silverfish. I'm guessing that it's a coincidence that your base started to fill up with them while the builder was running. Can you provide any other information to help me reproduce this issue?
Also, what's ballistic magma?
a side note that shouldn't really matter is that it was in the nether, it was basaltic magma from primal core, i read it wrong ;) however the blocks that are broken in the pack do not "contain monster eggs" they are nether blocks. they do not spawn from the block like monster eggs would. i do not know what mod adds it, my guess is primal core, nether ex or cyclic. seeing how silverfish from monster egg spawns on the block broken and not the breaker.serched for the mods that added add the hellfish and silverfish in the nether. and came across this: "NEVER quarry nether when you're using nether ore mod if you don't tweak the hellfish config, it will break your server. " so apparently someone else has had the same issue.
i suggest adding a single inventory space to the builder that if chest is full it will put it in that and if that is full it will stop the builder, and that will push to chest if chest is emptied.
Okay, there are two bugs here.
The first bug is that the builder would pass the wrong position to fireBlockHarvesting
, so things that are supposed to happen to the block being quarried in HarvestDropsEvent
would instead happen to the builder block itself. This explains silverfish spawning around the builder at all (Cyclic), and it getting turned into basaltic magma (PrimalCore). Coincidentally, I fixed this in RFTools 7.32 (via commit 3ea70c4) two days before you opened this issue.
The other bug is that if the builder's inventory fills up, fireBlockHarvesting
will be called multiple times on the same block. This explains why the silverfish would keep spawning when the inventory was full (it would repeatedly call the event, see it doesn't have room for the drops, then not break the block). This one isn't fixed yet.
If you update to 7.32 now, you'll still get masses of silverfish if your inventory fills up while on an infested ore block, but they'll spawn at said block rather than at the builder. You also won't have to worry about the builder turning into basaltic magma anymore.
Side note:
"NEVER quarry nether when you're using nether ore mod if you don't tweak the hellfish config, it will break your server"
The post in question was talking about Nether Ores, a 1.7.10 only mod, and about hellfish rather than silverfish. It's completely irrelevant to what's happening here.
Why did you close this? It's not fixed yet.
Your fix does seem like a reasonable idea. I'll see what I can do.
Hmm hold it. This doesn't sound like a good solution. Can't this be solved without that extra inventory in the builder? That's a rather clumsy solut2
So here's the current sequence of events:
- We make a list of drops for the block we're working on, via
block.getDrops
and firingHarvestDropsEvent
- We check whether there's room to hold all of the drops we just found
- If there is room, we add them to the inventory and then call
clearOrDirtBlock
. If there's not room, we leave the block there and just callwaitOrSkip
.
The problem is that HarvestDropsEvent
(and potentially getDrops
) can have side effects. The expectation is that the block will be immediately dropped and removed after calling these. We violate this expectation when we call them but then leave the block around. This problem is compounded when we repeatedly retry the same block, causing these to be called very rapidly on the same block.
There are two solutions for this that I can think of:
- When the inventory fills up, keep breaking blocks, and just put their drops in the world as entities instead of putting them in anything.
- When the inventory fills up, force the builder to pause regardless of its settings. Keep track of the items that we couldn't insert (but don't expose them as inventory). Once the inventory has room again, put said items in it and resume work.
Solution 2 is what I was originally thinking of. Let me know whether you're okay with this, or if you know of a better way to fix this.
if you break the builder in solution 2 you it would void the blocks no? or would you keep them saved?
Re solution 1, yeah I kind of figured that, I just listed it for completeness.
Re solution 2, I'd make breaking the builder drop the saved drops along with itself.
there would have to be a limit, if it continued to run for 1million blocks and someone broke the builder
There is a limit:
When the inventory fills up, force the builder to pause regardless of its settings
So only 1 block's worth of drops could ever be inside of it at a time.