search

Rearth / Oritech

Minecraft fabric tech mod
https://moddedmc.org/en/mod/oritech/docs
Creative Commons Zero v1.0 Universal
65 stars 17 forks source link

Fixed duping issue with portable tank #56

Closed jules552 closed 3 months ago

jules552 commented 3 months ago

This PR address issue #55.

The stack variable had one issue since it kept containing the item that the block was holding inside his inventory but at the same time we were also dropping to the ground this same item, causing the duplication.

I don't really know if the fix I've made is ideal, maybe you want to keep the functionnality of writeNbt the same but remove from the nbt data the item directly in the onBreak function?

jshipley commented 3 months ago

Wouldn't this make any buckets in the portable tank disappear when you log out?

A better fix would be to empty out the inventory slots when spawning items in the block's onBreak method.

jules552 commented 3 months ago

Wouldn't this make any buckets in the portable tank disappear when you log out?

A better fix would be to empty out the inventory slots when spawning items in the block's onBreak method.

Yeah you were right it wasn't saving properly anymore, I've changed the way of handleling things so that everything is done in the onBreak method. Tell me if it seems better to you. (Sorry I haven't really worked with Fabric in my life so)

Rearth commented 3 months ago

Hi, thanks for the bugreport, and the PR for it. I see a few small issues with the changes here: Code from both the writeNbt and the itemstack creation is partially duplicated. I'm not sure if you saw it, but there's a method in the tankentity class for creating the itemstack with the proper data. This also includes a custom name, which breaks with your PR. I think the easiest and cleanest solution is to just remove the stored items from the dropped item nbt. This will work with any future changes to the nbt data, and requires way fewer changes.

I hope you don't mind it, but I just implemented this change on main while skipping this PR. As it's only one line, I think this way is easier than updating the PR to reflect the change. You can check it out here in this commit: https://github.com/Rearth/Oritech/commit/8ce278bfa3708ab43d0b98b998ba9fd595f2d201

Still, huge thanks for creating the PR. Even if I won't take this one, all help is appreciated.