SpongePowered / SpongeForge

A Forge mod that implements SpongeAPI
http://www.spongepowered.org/
MIT License
1.14k stars 306 forks source link

Fluid Dupe #2535

Closed SawFowl closed 5 years ago

SawFowl commented 5 years ago

I am currently running

Issue Description Dupe occurs when trying to use an empty bucket on a block of liquid in a foreign region. To reproduce the bug, you must have two empty buckets, as well as cancellation of the interaction event with the unit. Video https://youtu.be/hTgckhEa7tw

There is no information on this issue in the logs.

Shybella commented 5 years ago

The video and issue description is not good enough. You need more detailed steps.

I am assuming "foreign region" means GriefPrevention claim. I tried this 3 different ways using GP and was unsuccessful.

Do the buckets go away when you relog or move them?

gabizou commented 5 years ago

As mentioned above, bucket cancellations have been extensively tested for various conditions, we need very clear and explicit reproduction steps to verify this “dupe”. Likewise, the inventory should be refreshing regardless of circumstances either by logging out or changing slots trying to pick up one of the items from said inventory.

SawFowl commented 5 years ago

Tomorrow I will try to make a test plugin to check the result of canceling the desired event. I will report the result. If the problem persists, I will provide a test build of the server where you can check it, as well as of course the plugin code.

SawFowl commented 5 years ago

I found the cause of the problem. There is a conflict with the Thermal Foundation mod. Under the link below you can download a ready-made test build of the server and see for yourself. Be sure to hold 2 buckets in your hand.

https://drive.google.com/open?id=15H4JOoVi9zGuVgtxj_LZ4TWtEWn8bxiL

gabizou commented 5 years ago

I'm not able to download that file, google drive just gives me a blank page trying to download it.

SawFowl commented 5 years ago

It seems that Google has problems. I'll try to think of something. I myself can not download it for some strange reason.

SawFowl commented 5 years ago

Try this link.

https://yadi.sk/d/L8AH5igFd_CcjQ

SawFowl commented 5 years ago

I found out something else. This code does not work with liquids. @Listener public void onInteractBlockEvent(InteractBlockEvent event) { event.setCancelled(true); }

phit commented 5 years ago

can you retest this on latest? there was some changes to interaction events since this issue was made

SawFowl commented 5 years ago

Got better. The buckets are no longer duplicated, instead they are filled with liquid, which, in general, should also not occur. In addition, there was a permanent flood in the console. SpongeForge version: 1.12.2-2768-7.1.5-RC3535 logs.zip

phit commented 5 years ago

yeah that spam is caused by https://github.com/SpongePowered/SpongeForge/issues/2574

SawFowl commented 5 years ago

I think I will continue to use the following code in my plugin for now. `

@Listener
public void fixBucketDupe(InteractItemEvent event) {
    if(event.getSource() instanceof Player) {
        Player player = (Player) event.getSource();
        if(!getGriefPrevention().getClaimManager(player.getWorld()).getClaimAt(player.getLocation()).getUserTrusts().contains(player)) {
            if((player.getItemInHand(HandTypes.MAIN_HAND).get().getType().equals(ItemTypes.BUCKET) && player.getItemInHand(HandTypes.MAIN_HAND).get().getQuantity() != 1) ||
                    (player.getItemInHand(HandTypes.OFF_HAND).get().getType().equals(ItemTypes.BUCKET) && player.getItemInHand(HandTypes.OFF_HAND).get().getQuantity() != 1)) {
                event.setCancelled(true);
            }
        }
    }
}

`

phit commented 5 years ago

@ImMorpheus was InteractBlockEvent supposed to block bucket filling?

ImMorpheus commented 5 years ago

ok, so:

[...] trying to use an empty bucket on a block of liquid [...]

This is done by ItemBucket#onItemRightClick which is called by ItemStack#useItemRightClick here

https://github.com/SpongePowered/SpongeCommon/blob/8ea7c0d6c3b229fed32bd1f3ce0cf4b92521c380/src/main/java/org/spongepowered/common/mixin/core/server/management/MixinPlayerInteractionManager.java#L400

The action of picking up a fluid with a bucket is actually linked to CPacketPlayerTryUseItem (thus InteractItemEvent). Simply put: you're not picking up water because you right-clicked on a block with a bucket. You're picking up water because you right-clicked with a bucket in your hand (you can also do that while pointing at a block, but it's not required).

Proof:

Go at the bottom of an ocean, look up, right click with an empty bucket in your hand:

An InteractItemEvent is fired, but no InteractBlockEvent since you haven't right-clicked on a "block".

Simply put, you're using the wrong event. You need to cancel InteractItemEvent.

ImMorpheus commented 5 years ago

Closing this.

The dupe is fixed and the Illegal Async PhaseTracker Access was a different issue (fixed)

SawFowl commented 5 years ago

The fix is ​​not complete. Buckets are filled with liquid.

I guess I'll have to cancel the event myself.

ImMorpheus commented 5 years ago

The fix is complete. As I said, that's not an issue because you're cancelling the wrong event.

https://github.com/SpongePowered/SpongeForge/issues/2535#issuecomment-457851582

SawFowl commented 5 years ago

Then I'll have to use that code. `

@Listener
public void fixBucketDupe(InteractItemEvent.Secondary event) {
    if(event.getSource() instanceof Player) {
        Player player = (Player) event.getSource();
        if(!getGriefPrevention().getClaimManager(player.getWorld()).getClaimAt(player.getLocation()).isTrusted(player.getUniqueId())) {
            if(event.getItemStack().createStack().getType().equals(ItemTypes.BUCKET)) {
                event.setCancelled(true);
            }
        }
    }
}

`

ImMorpheus commented 5 years ago
  1. If you want to prevent a bucket from picking up water, you need to cancel InteractItemEvent.Secondary. There's no InteractBlockEvent when right-clicking on a liquid without pointing at a (solid) block.
  2. Your code is broken because is using the player position instead of the position the player interacted with.
SawFowl commented 5 years ago

Actually, it works. Without this code, the bucket will be filled with water.

SawFowl commented 5 years ago

I think the problem is connected with the Thermal Foundation mod. Perhaps I should write an issue to them.

phit commented 5 years ago

Honestly I don't even know what you think the issue is here, make a clean issue with what code you expect to work and how it doesn't. From reading the last few messages this sounds more like an issue with GP if anything.