Blocks in 'place' flag only placeable if clicked block is also in flag

[mindw0rm]

Debug paste link:
https://athion.net/ISPaster/paste/view/da7449b0efc14e5c9b20249b5171ae49
[REQUIRED] Spigot/Paper Version Number:
CraftBukkit version git-Spigot-94af569-c2d1201 (MC: 1.14.4) (Implementing API version 1.14.4-R0.1-SNAPSHOT) [latest version]

[REQUIRED] Minecraft Version Number: 1.14.4

[REQUIRED] Description of the problem:
If you try to place a block on a plot where you are not a member, but where the block type is in the 'place' flag, you can only place it if the block that you clicked at is also in the 'place' flag.

If you click on a block which is not in 'place', the message "Have an admin set the flag: place" is shown.
If you click on a block which is in 'place', but the block in your hand is not, the message "You are lacking the permission node: plots.admin.build.other" is shown.
Only if both the block in your hand and the one you click at is in 'place', it can be placed.

Note that while the above debugpaste shows the current release of PlotSquared (4.353), the bug also happens with the current github version - I normally use a custom version which branched at 4.386 (but the stuff I added - #2506, and some new flag which I might turn to a pull request later when it works as intended, see my fork if you are curious - does not concern block placement).

I tracked it to https://github.com/IntellectualSites/PlotSquared/blob/breaking/Bukkit/src/main/java/com/github/intellectualsites/plotsquared/bukkit/listeners/PlayerEvents.java#L2139: here lb is the block that was interacted with, i.e. the one clicked at, and EventUtil.manager.checkPlayerBlockEvent uses the type of lb when checking the 'place' flag.

Since 'place' is checked later anyway (in https://github.com/IntellectualSites/PlotSquared/blob/breaking/Bukkit/src/main/java/com/github/intellectualsites/plotsquared/bukkit/listeners/PlayerEvents.java#L3005), maybe the check in onInteract can be removed?

Plugins being used on the server:
see debugpaste above, none should be relevant for this bug.

[REQUIRED] How to replicate:

• add some block to the place flag of a given plot (e.g. /plot flag add stone)
• place some of that block on the plot (if not already present), make sure that there are other
  block types reachable (e.g. grass_block)
• switch to a user that is not member/trusted on the plot (you could also do the above steps as op on a plot that is not yours, and then deop yourself)
• try to
  ** place the block in place on blocks of different type, which is not in 'place' --> error message shown (but this should imho work)
  ** place it on a block with the same type --> can be placed
  ** place a different block (which is not in 'place') on the block in 'place' --> different error message shown (which is correct)

[REQUIRED] Checklist:

• I included all information required in the sections above
• I made sure there are no duplicates of this report (Use Search)
• I made sure I am using an up-to-date version of PlotSquared
• I made sure the bug/error is not caused by any other plugin

[MattBDev]

Removing any onInteract check tends to break a lot of other things unintentionally. I believe this is intended behavior though.

[mindw0rm]

So if you want to allow anyone to place flowers on your grass, you have to add grass to the 'place' flag, too?
And then anyone can place grass as well (well, not everywhere, but at least on top (or to the side) of the existing grass and flowers)?

If this is intended, then why? Isn't it sufficient to specify the blocks that you want to be able to be placed?
Wouldn't it be better to split it and add a flag 'place-on' which specifies on which blocks everyone is allowed to place stuff (the stuff that is allowed by 'place'). Maybe with "everything" being a valid value?

[MattBDev]

I misread the issue when I made that first comment. Ignore everything I said except for the onItemInteract part.

[AeSix]

To me, honestly, it sounds like something is broken - but not what @mindw0rm is thinking.
If a person is not added/trusted to a plot, they shouldn't be able to build at all, regardless of what the place flags say. Place flags should apply only to those who are added.

However, since I'm just a drop in the bucket of users in this, I think it would be up to the devs to decide which is actually broken.

[mindw0rm]

Someone who is trusted/added can place anything anyway, why should there be a flag required?

Or maybe they only should be able to place anything if the place flag is empty, and if it is not, they can place only the specified blocks?
In that case the flag would indeed be pretty broken, becaue atm it only affects nontrusted players ;)

[RedstoneFuture]

Same issue by myself:
#2483

[mindw0rm]

Why haven't I found that issue when I searched for 'place'? :(

[AeSix]

I'm derp. Sorry. Yes, the flags are for non-added players to be able to interact.
Anyways, closing this as it's a dupilicate of #2483

[mindw0rm]

@AeSix I don't agree that it is a duplicate, because the problematic here is not mentioned in #2483 at all (except shortly in one of my comments, but only as a reference to here).

I'm not even sure what exactly the issue of the other bug is (maybe that there should be a possibility to add all blocks to the blocklist flags?), but it has nothing to do with the problem that the block that you clicked on is also important for placing blocks.

Please reconsider you decision, or at least edit the other bug so that the problem described here is mentioned in the first post. Because else it will be forgotten, and I'll have to report it again in the future.
I could add a comment, but chances are high that this will be overlooked as well.

[AeSix]

they appear to be the same root cause to me.
reopened anyways.

[mindw0rm]

Well I think the root cause of this bug is that here,
the material of lb should be set to type.

IF that is done, the check called here gets a block with the material in the hand you clicked with (i.e. the one you want to place) instead of the the material of the block that you clicked at.

But that won't help with the other issue.

---

I just added the line
lb = new BukkitLazyBlock (new StringPlotBlock(type.toString()));
(seems there is no more elegant way to do it?) and as expected the bug is gone.

[mindw0rm]

Fun fact: all I had to do move a line up a bit. Took me a while to realize that the thing that I want to happen is already done directly after the if where it is required :)