Arena signs cannot be easily removed

[EpicKnarvik97]

Bug report

Short description

After creating an arena sign, there is no apparent method for removing it if it was mis-placed by accident, or it needs to be moved in the future. I tried looking at the arena signs documentation. I tried looking through the commands. I tried disabling the arena. I tried enabling edit mode. I tried breaking the block behind the sign, hoping it might be de-registered after a reload (it was not). The only way I found to remove an arena sign was to manually edit signs.csv, and then reload (a bit of a chore as I've already got almost 40 arena signs to look through).

Reproduction steps

1. Create an arena
2. Create an arena sign for the arena, of any type
3. Try to remove the sign in-game

Details

• MobArena version: 0.108
• Server version: git-Paper-478 (MC: 1.20.4)

Additional info

As clearly stated in the short description, enabling edit mode with the /ma editarena command does not allow me to destroy an arena sign. Neither does disabling an arena. Neither does disabling an arena and enabling edit mode at the same time.

[Nesseley]

Hi! I'm pretty sure you need to do /ma editarena to remove the signs! They are protected by the arena region :)

Please join our Discord if you need any more help! ^^

[EpicKnarvik97]

I tried enabling edit mode.

I'm pretty sure you need to do /ma editarena to remove the signs!

As I clearly stated, even when I enable edit mode, I cannot remove any arena sign. Neither arena signs in the arena or signs outside the area. Please don't close issues without reading them.

[garbagemule]

Just gave this a whirl, and it turns out the sign destruction is being blocked, not due to the regular protection mechanism, but as a side effect of the recent attempt at blocking sign editing. It seems like cancelling the interact event also cancels what would become a block break event, which means the usual "sign was broken, so let's deregister it" logic is never actually invoked.

That means this issue is closely related to #787. The "best" option is to bump the Spigot API version so we can handle sign breaking and sign editing independently, but this could (c.f. my question in #787) result in breaking compatibility for people on older server versions (currently the majority of servers running MobArena), unless listening for non-existent events is not a breaking change.

Happy to review potential workarounds, but I don't have time to look into any myself right now. One workaround would be to downgrade to 0.107 and break the signs there, then hop back to 0.108 (or stay on 0.107 for the time being).

[EpicKnarvik97]

Happy to review potential workarounds, but I don't have time to look into any myself right now. One workaround would be to downgrade to 0.107 and break the signs there, then hop back to 0.108 (or stay on 0.107 for the time being).

I was actually able to just now (after being told the problem was strictly related to sign blocks) find a single way to do it in-game.

1. Destroy the sign by breaking the block beneath it/below it, or remove it using WorldEdit.
2. Place a non-sign block where the sign was.
3. Destroy the newly placed block.

[EpicKnarvik97]

The easiest way code-wise I can think of to allow the signs to be destroyed while changing as little code as possible, and without breaking backwards compatibility would be to add additional conditions to HandleSignClicks.on(), like ignoring normal behavior if the player is sneaking (I tested it, and it works):

@EventHandler(priority = EventPriority.HIGH) // Changed from MONITOR because an event is cancelled
public void on(PlayerInteractEvent event) {
    Block block = event.getClickedBlock();
    if (block == null) {
        return;
    }
    if (!(block.getState() instanceof Sign)) {
        return;
    }

    ArenaSign sign = signStore.findByLocation(block.getLocation());
    if (sign != null && (!event.getPlayer().isSneaking() || event.getAction() != Action.LEFT_CLICK_BLOCK)) { // Added sneaking and left-click conditions
        event.setCancelled(true);
        purgeAndInvoke(sign, event.getPlayer());
    }
}

It would probably be better to replace !event.getPlayer().isSneaking() || event.getAction() != Action.LEFT_CLICK_BLOCK with a check for if the arena is in edit mode and the action is left click block, though I'm not familiar enough with the code to know how I'd get the arena from the ArenaSign's arena id.

[garbagemule]

@EpicKnarvik97 I've implemented something similar to what you suggested in the 791-sneaky-signs branch. You can grab a build off GitHub Actions or hit me up on Discord if you want to test it out. I gave it a quick whirl and it seems to work fine, but I'd love to get some more hands on it before I commit to releasing it. I'd love it if you could give it a spin.

[EpicKnarvik97]

@EpicKnarvik97 I've implemented something similar to what you suggested in the 791-sneaky-signs branch. You can grab a build off GitHub Actions or hit me up on Discord if you want to test it out. I gave it a quick whirl and it seems to work fine, but I'd love to get some more hands on it before I commit to releasing it. I'd love it if you could give it a spin.

It seems to work fine, though it's not currently limited to left-clicks only which allows users to edit arena signs while sneaking, not only destroy them. Editing the sign (adding text for a valid join sign on an info sign for example) does not change the sign to another type of MobArena sign, so being able to edit it is definitely a bug. The sign cannot be edited or destroyed when the arena is not in edit mode, which is as it should be.

If you just turn if (event.getPlayer().isSneaking()) { (from commit ca195eb) into if (event.getPlayer().isSneaking() && event.getAction() == Action.LEFT_CLICK_BLOCK) { it should be perfect (until you decide to use the new sign API).

[garbagemule]

Right, I actually wrote a TODO about right-clicking in the WIP commit, but I completely forgot about it, hah. I'll work it in there, thanks :)

The sign cannot be edited or destroyed when the arena is not in edit mode, which is as it should be.

I just want to point out that this is only partially true. Arena Signs inside the arena region are protected the same way other blocks are as part of the normal protection code, but Arena Signs outside the region are not protected. This is by design, but not necessarily something I have any strong feelings for or against. There are no permissions associated with Arena Signs, so technically anyone can create (or destroy) an Arena Sign anywhere.

[EpicKnarvik97]

I just want to point out that this is only partially true. Arena Signs inside the arena region are protected the same way other blocks are as part of the normal protection code, but Arena Signs outside the region are not protected. This is by design, but not necessarily something I have any strong feelings for or against. There are no permissions associated with Arena Signs, so technically anyone can create (or destroy) an Arena Sign anywhere.

I forgot to specify that I only tried signs inside the arena's lobby the last time I tested. When outside of the arena, the signs behave the same as within arenas in edit mode.

With the change, it works just as it should. The sign behaves normally (join signs make players join and stuff) except while sneaking and hitting the sign. The signs cannot be edited no matter what.

The one case I've found that can be improved is when the sign is protected. For example, if a sign is within an arena's area and edit mode is disabled, the normal join action could still be triggered on sneak + left click (it's currently cancelled). For signs outside of arenas, I don't really know a good way of keeping the join action when sneaking, except perhaps checking for some MobArena admin permission so normal players would join arenas when left-clicking a sign outside an arena while sneaking. It's not that big of a deal, but a normal player might find it odd that the sign doesn't work while sneaking.

[garbagemule]

Thanks so much for testing it out! I'll get it merged into the main branch and included in the upcoming release :)

The one case I've found that can be improved is when the sign is protected. For example, if a sign is within an arena's area and edit mode is disabled, the normal join action could still be triggered on sneak + left click (it's currently cancelled). For signs outside of arenas, I don't really know a good way of keeping the join action when sneaking, except perhaps checking for some MobArena admin permission so normal players would join arenas when left-clicking a sign outside an arena while sneaking. It's not that big of a deal, but a normal player might find it odd that the sign doesn't work while sneaking.

This is a valid point, and I'll keep it in mind for when we eventually adopt the sign change event, which might require a bit of a rethink about some of these workarounds anyway. For now, I think it's a subtle enough "bug" that we can ignore it in favor of actually being able to break arena signs 😁