CB5: StoredBlockMechanic drops regardless of the block's actual removal

[BlvckBytes]

CraftBook Version

5.0.0-SNAPSHOT

Platform Version

Paper version 1.21.10-101-main@893ea74

Confirmations

• I am using the most recent Minecraft release.
• I am using a version of WorldEdit compatible with my Minecraft version.
• I am using a version of CraftBook compatible with my Minecraft version.
• I am using the latest or recommended version of my platform software.
• I am NOT using a hybrid server, e.g. a server that combines Bukkit and Forge. Examples include Arclight, Mohist, and Cardboard.
• I am NOT using a fork of WorldEdit, such as FastAsyncWorldEdit (FAWE) or AsyncWorldEdit (AWE)

Bug Description

When using external sign-editors of any kind, be that the one of CraftBook 3 (for the reasons of not wanting to alter the command-structure), or any third-party plugin really, they may fire a fake BlockBreakEvent as to check for build-permissions, which CraftBook 5 will not ignore, seeing how it only accounts for internal events using its built-in ignore-list.

Check out the handler here:

CraftBook/craftbook-bukkit/src/main/java/org/enginehub/craftbook/mechanics/area/StoredBlockMechanic.java

Lines 180 to 213 in 7c891bc

	public void onBlockBreak(BlockBreakEvent event) {
	if (!EventUtil.passesFilter(event)) {
	return;
	}
	if (!SignUtil.isSign(event.getBlock())) {
	return;
	}
	Sign sign = (Sign) event.getBlock().getState(false);
	if (!isApplicableSign(sign)) {
	return;
	}
	CraftBookPlayer player = CraftBookPlugin.inst().wrapPlayer(event.getPlayer());
	int amount = getStoredBlockCounts(sign);
	if (amount > 0) {
	try {
	Material base = getOrSetStoredType(event.getBlock());
	while (amount > 0) {
	ItemStack toDrop = new ItemStack(base, Math.min(amount, 64));
	event.getBlock().getWorld().dropItemNaturally(event.getBlock().getLocation(), toDrop);
	amount -= 64;
	}
	} catch (InvalidMechanismException e) {
	if (e.getMessage() != null) {
	player.printError(TextComponent.of(e.getMessage()));
	}
	}
	}
	}

It drops the items regardless of whether the block is actually gone. In the case that it is, it makes sense why one wouldn't try to set the stored amount back to zero, due to the PDC having been destroyed already anyway. Due to the aforementioned reason, this does not suffice in real-world scenarios; one cannot simply assume the absence of other plugins which use standard-practise checks.

I would suggest making this feature bullet-proof against various special cases, by ensuring the absence of the sign before handing out the stored items, as follows:

    @EventHandler(priority = EventPriority.HIGH)
    public void onBlockBreak(BlockBreakEvent event) {
        if (!EventUtil.passesFilter(event)) {
            return;
        }

        if (!SignUtil.isSign(event.getBlock())) {
            return;
        }

        Sign sign = (Sign) event.getBlock().getState(false);


        if (!isApplicableSign(sign)) {
            return;
        }

        CraftBookPlayer player = CraftBookPlugin.inst().wrapPlayer(event.getPlayer());
        int amount = getStoredBlockCounts(sign);

        Material base;

        try {
            base = getOrSetStoredType(event.getBlock());
        } catch (InvalidMechanismException e) {
            if (e.getMessage() != null) {
                player.printError(TextComponent.of(e.getMessage()));
            }
            return;
        }

        Bukkit.getScheduler().runTaskLater(CraftBookPlugin.inst(), () -> {
            // The sign has not actually been destroyed, but this was rather a fake event used
            // to check whether the player has permission to build in this area.
            if (SignUtil.isSign(event.getBlock()))
                return;

            int remainingAmount = amount;

            if (remainingAmount > 0) {
                while (remainingAmount > 0) {
                    ItemStack toDrop = new ItemStack(base, Math.min(remainingAmount, 64));
                    event.getBlock().getWorld().dropItemNaturally(event.getBlock().getLocation(), toDrop);
                    remainingAmount -= 64;
                }
            }
        }, 1L);
    }

Expected Behavior

Check for the block's destruction next tick and only then drop the items.

Reproduction Steps

1. Use the SignCopier of CraftBook 3, which performs this check
2. Use the Bridge-Mechanic of CraftBook 5
3. Try to copy the bridge-sign - the items will drop, due to the internal protection-check of CraftBook 3

Anything Else?

No response