search

stfwi / engineers-decor

Engineer's Decor
MIT License
35 stars 16 forks source link

Conflict with PMMO causing Labeled crates to void items #173

Closed HopsandBarley closed 3 years ago

HopsandBarley commented 3 years ago

There is a conflict between Engineer's decor labeled crates and Project MMO, that is causing contents of Labeled crates to be voided when the crates are moved or broken. Do no know the exact conflict, but when PMMO is uninstalled, the bug goes away. (1.16)

Likely an issue with PMMO, but I dont know for sure, so I wanted to bring it to your attention.

KoloNiko commented 3 years ago

Same issue on Enginners Life 2.

stfwi commented 3 years ago

Hi guys, I gonna double check on the ED side, too. Is there already related issue dropped at pmmo so that @Harmonised7 knows? Cheers!

KoloNiko commented 3 years ago

Hi, yes it's post in bug section on Discord's PMMO. Have a good day.

HopsandBarley commented 3 years ago

The issue in engineers life 2 is because of the conflict between engineers decor and pmmo. That's where I found it.

On Sat, Apr 10, 2021, 3:23 AM KoloNiko @.***> wrote:

Hi, yes it's post in bug section on Discord's PMMO. Have a good day.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/stfwi/engineers-decor/issues/173#issuecomment-817093147, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASVGY7ASORAA5YVIZ6IAYILTH74GRANCNFSM42O6K6WA .

Harmonised7 commented 3 years ago

I can confirm that LootContext code of Project MMO is causing this, looking into it now. If someone finds something out, please let me know on Discord. (I will likely not be replying anywhere other than Discord)

Edit: drops = block.getDrops( event.getState(), builder ); is the method that destroys the items. I do not think that this should happen, so I am assuming it's not my side that's causing this. Keep in mind that I don't really understand LootContext related things, though I give all the arguments to my builder that the event provides, this includes: Player Position, Tool used, Player Entity, and Tile Entity

Harmonised7 commented 3 years ago

I've found an inefficient solution to this issue, which will be fixed in 3.52.5

P.S. Whoever left the Thumbs Down emoji, that's super inconsiderate... Your actions affect people, and in this case, rather negatively...

stfwi commented 3 years ago

Hi Harmonised, thanks for the quick check an keeping me in the loop. Let me take a look in the code of the Crate if I can find an problem there, too. Don't worry about the emoji, it's very likely mis-clicked, people are normally happy when modders care and maintain - and PMMO is a really nice mod. Cheers!

KoloNiko commented 3 years ago

Hi, thanks.Harmonised.

stfwi commented 3 years ago

Ok, quickly double checked. So the getDrops(...) of the crate depends on the loot context explosion radius (not null and >0) and that the tile entity is still available when getDrops(...) is called. Then it gathers the TE contents into item nbt and returns the crate item. The call is indirect via a super class (because I have more blocks keeping their inventory). Could we have a collision there?

In code, this would be these methods, getDrops() is here:

https://github.com/stfwi/engineers-decor/blob/4f07f82e0126cb10a06e2d492a361ce3dccc5312/src/main/java/wile/engineersdecor/libmc/blocks/StandardBlocks.java#L197-L207

... and the Crate drops here:

https://github.com/stfwi/engineers-decor/blob/4f07f82e0126cb10a06e2d492a361ce3dccc5312/src/main/java/wile/engineersdecor/blocks/EdLabeledCrate.java#L134-L153

You can also paste the permalink of the caller side in PMMO if you like an additional pair of eyes checking. Cheers!

Harmonised7 commented 3 years ago

Hi! I was told about your reply ^^ Here's the LootContext Builder

https://github.com/Harmonised7/project_mmo/blob/c31aa8d1c336b5185361096c80f7391d08e7a3eb/src/main/java/harmonised/pmmo/events/BlockBrokenHandler.java#L214

stfwi commented 3 years ago

Hi again. Alright, I'd say your code looks pretty much ok. One thing I noticed in the ED mod that can cause the problem, too: Querying the Crate drops will clear its inventory after gathering the TE data. That is a workaround for a drop dupe issue in the 1.14ish MC version range - which I did not take out yet. At least until now. So, if another event callback queries the Crate drops, but does not actually drop them, then your callback (probably last due to low prio) will see an empty crate. "Hence", there will be also a fix from the ED side ;). Cheers!

stfwi commented 3 years ago

Okay, the update is on curse, closing then, cheers guys!