Biomes O' Plenty

Biomes O' Plenty

151M Downloads

BoP Causing Duplication Glitches with NeoTech

Dyonovan opened this issue ยท 10 comments

commented

For some reason, whenever BoP is installed along with Neotech, it is causing some weird duplication issues.

It being called from biomesoplenty.common.handler.AchievementEventHandler on your blockplace override.

Log: https://gist.github.com/anonymous/8b1f0e7c074805980dba

TeamBR-Modding/Neotech#162

commented

Yes, this just happened on my server. We crafted 1 flushable chest and ended up with infinite amounts :)

https://sx.artdude.uk/hotlink-ok/darkosto/sZhT-973d2-1456982599.png

commented

See my response to #704, since they're both essentially the same issue

commented

I'm not sure expecting a valid state for item inventory damage should be assumed. For example, we use the index value of the EnumFacing to save and return from meta data. For things that do not face up or, there is no 0 which is the default damage on itemstacks. Since we don't care what direction the block is facing when not in world, there is no need to have the ItemBlock care about metadata because the meta the world block needs is set on place. While I can see saying mods should have a default 0 meta I don't believe you can say that all mods will use meta in that way. Perhaps 0 means something different than default for some mods. I think that it wouldn't be a bad idea to just catch errors as I'm sure many mods will have the same issue. The connection between inventory meta and world meta is not assumed to be connected

commented

Made a clarification about that a few mins ago. It appears that's the exact issue that getMetadata should solve, provided it's implemented to account for individual blocks. I think that's what will still have to be done anyways, but I feel like that's the 'proper' way of handling situations such as what you mentioned above.

commented

I'm not sure I clearly understand what you are asking, are you stating that we should be implementing an Itemblock that will move the meta to a value that is acceptable if the default meta on the stack does not match a world meta?

commented

Correct, and that's how Vanilla appears to handle it as well(e.g. leaves uses damage | 4 because item metadata doesn't have the 'player placed' flag). So a custom ItemBlock uses getMetadata(int damage) to translate an item's metadata values to metadata of a valid block state which would be placed in the world.

commented

The only reason they do that is to add that flag when placed. We do not have access to the player's rotation from that method so we can't get what value we want on place from there so that is why we set it on block placed. Again, while it would be nice to have that method give the values you need some blocks just don't have the ability to get the correct value at that point. Asking everyone to implement an itemblock for all blocks with non matching item/world block meta seems to be a bit much. I agree that that getMetadata method should be the best way to handle this situation but I fear you're just going to have lots of issues with mod interaction.

On our end, to prevent this issue since it sounds as though you are not willing to handle catching errors (regardless of which end is doing things differently) I will add a statement to return the default state when requesting metadata 0. That does not correspond to the correct world state as that can't be parsed from the given variables, but should prevent this issue. I hope you at least see the point we are trying to make, I do understand what you are saying; however, I feel that the connection between item damage and world meta is not presented as a direct connection.

commented

Ah, did not see your commit, I'll still but some extra checks in place but thank you for handing things on your end as well. No hard feelings, I know it may sound harsh through text but glad to work with you

commented

Yeah, it's a fair conclusion and ultimately why I just went with it anyway. It seems like a simple problem that should have a simple 'non-hacky' solution, but I guess things are never that easy. At the end of the day, it's pointless waiting for tons of issues to pop up and tons of dismissals when it can just be easily avoided.

commented

I don't think this is fixed, I'm getting the same error using BoP version 1.9-4.0.2.2018 and NeoTech version 1.9-3.0.6. It's not crashing when breaking/placing the block, but when picking up the dropped item.

Crash log: https://gist.github.com/JCMais/24db69f67ba82c9db85ae23ac05ef292

it's a client-only crash, the server only drops the connection.

Edit.

This is the line affected

IBlockState state = block != null ? block.getStateFromMeta(stack.getItemDamage()) : null;
Looks like the same issue, but in another code path.