Cannot click to activate message blocks if user does not have permission to delete block

[Goodlyay]

This issue applies to portals as well.

• [ ] Use PlayerClick on MBs you do not have permission to delete
• [ ] If PlayerClick is not supported, allow the MB to be activated by deletion. Currently this is intercepted by code which checks permission of block and code which checks if level deletable is enabled.

[Goodlyay]

Tentative fix for task 1 could look something like

internal static void HandlePlayerClick(Player p, MouseButton button, MouseAction action, ushort yaw, ushort pitch,
                                       byte entity, ushort x, ushort y, ushort z, TargetBlockFace face) {
    if (action != MouseAction.Pressed) return;           
    if (entity != Entities.SelfID && ClickOnBot(p, entity)) return;
    if (!p.level.IsValidPos(x, y, z)) return;

    BlockID block = p.level.GetBlock(x, y, z);

    // if block is deletable, handle MBs/Portals in Player.HandleManualChange instead
    // TODO: fix that handling MBs/Portals in Player.HandleManualChange is blocked by CheckManualChange and parent caller method ProcessBlockchange
    if (CanDeleteBlockClientSide(p, block)) return;

    bool isMB     = p.level.Props[block].IsMessageBlock;
    bool isPortal = p.level.Props[block].IsPortal;

    if (isMB) { MessageBlock.Handle(p, x, y, z, true); }
    if (isPortal) { Portal.Handle(p, x, y, z); }
}

static bool CanDeleteBlockClientSide(Player p, BlockID block) {
    // Clients that do not support BlockPermissios can always do client-side deletions
    // TODO: This might need a tweak to take bedrock into account
    if (!p.Supports(CpeExt.BlockPermissions)) return true;

    if (!p.level.Config.Deletable) return false;

    return p.group.Blocks[block];
}

[rdebath]

Like #714 I'd suggest that using PlayerClick seems to be a bit of an overkill, especially as you have to do the other method anyway.

[Goodlyay]

Why is it overkill? It makes sense that you should be able to trigger the block without literally deleting it clientside.

[rdebath]

That would indeed make sense. But the classic client isn't going to stop deleting a block just because the server is listening to the PlayerClick packet. Now if the client knows the difference between a White block and a White Message Block it can choose not to delete the latter. But it can't see any difference between them.

[Goodlyay]

That's why the PlayerClick wouldn't trigger the block if the client is able to delete it. It would trigger it only once in either case

[rdebath]

Ah, oops, my bad. For "user" I read "player" you wrote "client".

Okay for part one that looks nice ... except ... selection (eg: /mark like) too pretty please :smiley: Oh look, you and @UnknownShadow200 were right about #714 PlayerClick is the better choice because it solves the same problem for MB and portals.

Note: There is a race here, but it should look like the user just fumbled the click.

As for the second part. ... reordering those checks probably needs to be done for pure classic mode, however, I think support for BlockPermissions without PlayerClick should be considered a client side issue as the only viable fix I see is "don't use BlockPermissions if PlayerClick isn't supported".