BlockGenericPipe#getDrops() drops items in world by itself
MineMaarten opened this issue ยท 11 comments
For block filter purposes I'm looking up the drops of blocks in a certain radius using the Block#getDrops() method. However, this causes BC pipes within range to drop their contents.
I'm fairly certain this is because the method is dropping the contents. As this is might be a fairly big fix (as it probably also handles things like pipe plugs drops) I decided to not PR it. Here's a reference to the code, where I think it goes wrong:
That... that looks so wrong to me. I'll debug it and try to fix it.
@SpaceToad - I believe this should go in 6.0.19 as it's a fairly big thing.
That's not a player behavior issue, but and issue with implementing a third party mod, right?
Fairly annoying I agree. But for the sake of stability, probably worth taking care of in 6.1.x
I see why that is in, @MineMaarten. We can't be sure if a block trying to getDrops() won't use getDrops() to implement a custom drop system, and I do not believe making pipes send their contents to getDrops() is anywhere near safe enough. What do you think?
In my perception getDrops() only should 'get the drops', not 'get some drops, and drop other drops'. I myself use the method to do some filtering depending on what the block will drop. I've seen this behaviour too in Steve's Factory Manager for example, and I'm sure there are others. There's only this method you can use to get every drop of a block, and I don't see any reason why you would drop an ItemStack yourself in this method when you can just add that ItemStack to the list you return in this method.
So, to actually give my opinion on your comment :P
"We can't be sure if a block trying to getDrops() won't use getDrops() to implement a custom drop system" , If it does, it should handle the actual block breaking too. And if it does, I don't really see a problem?
"and I do not believe making pipes send their contents to getDrops() is anywhere near safe enough", That's what we do with Blue Power tubes in the end (as it's a multipart). I'm not sure what the worst thing is that a mod can do to it?
@MineMaarten - I guess I could do that. I just fear some mods doing unexpected things and causing dupe bugs.
Let's do that for 6.1.4 and see what happens.
@MineMaarten - could you take a look at this fix?
@asiekierka I am worried that it might be problem right now. Using Steve's Factory Manager you can check block for its drops. It is more than possible that it uses Block#getDrops()
@asiekierka Looks good to me :). Thanks! For some reason I expected it to be a harder fix, that the drop --> getDrop code would have to be changed for things like facads and what not too. However, this looks like the fix :). (If I knew it would be that simple I would've PR-ed, sorry :P)
@asiekierka @MineMaarten,
I do not know if this is fixed in 1.7.10 but is this bug is still there in 1.7.10:
When you break a block in 1.6.4 the TileEntity got removed before the getDrops()/getBlockDropped() function got called. Is that still exsisting in 1.7.10?
I would test that if you do not know that.
@Speiger - apparently not anymore, judging from testing.
@asiekierka good thing to know. I had to make a way around.
My solution for this if it does not work still in 1.7.10
https://github.com/TinyModularThings/TinyModularThings/blob/1.6.4/Spmod_Common/Source/speiger/src/spmodapi/common/blocks/cores/SpmodBlockContainerBase.java#L515
https://github.com/TinyModularThings/TinyModularThings/blob/1.6.4/Spmod_Common/Source/speiger/src/spmodapi/common/blocks/cores/SpmodBlockContainerBase.java#L539
https://github.com/TinyModularThings/TinyModularThings/blob/1.6.4/Spmod_Common/Source/speiger/src/spmodapi/common/blocks/cores/SpmodBlockContainerBase.java#L35