[Critical bug] WGEF breaks WG's protection

[stonar96]

In my opinion this is a very critical bug, because WGEF breaks WG's protection by just installing the plugin (not even using any features).

The setup: Install WG and add the following regions:

regions:
    inner:
        min: {x: -16, y: 0, z: -16}
        max: {x: 15, y: 255, z: 15}
        members: {}
        flags: {}
        owners:
            unique-ids: [your-uuid]
        type: cuboid
        priority: 1
    outer:
        min: {x: -32, y: 0, z: -32}
        max: {x: 31, y: 255, z: 31}
        members: {}
        flags: {}
        owners: {}
        type: cuboid
        priority: 0

Make sure you are not op or have any special permissions. You can build in the inner region. Place a sapling at the edge inside of the inner region and use bone meal to grow the tree. Since you can't build in the outer region, the tree won't grow out of the inner region, as expected.

The issue: Now install WGEF and do the exact same thing in-game. The tree will grow outside although it shoudn't.

Versions:
WGEF: 4.1.4
WG: 7.0.4
Server: git-Paper-318 (MC: 1.16.4) (Implementing API version 1.16.4-R0.1-SNAPSHOT)

[TheoRetisch1]

I have the same problem. Take four spruce saplings and let them grow on a region border using bonemeal. The leaves and podzol will spread across the region boundary to plots where you don't have any rights. When you uninstall Worldguardextraflags, the Worldguard protection mechanism works correctly and leaves + podzol are cut off at the boundary of the region.

WorldGuardExtraFlags version 4.1.5-SNAPSHOT
WorldGuard version 7.0.5-SNAPSHOT+2084-035a645
Paper 1.16.4 build 318

[aromaa]

Seems to only effect WG 7, looking in to it

[stonar96]

Thanks! As I said, I removed my review on Spigot. Do you plan to properly reimplement the removed flags for WG 7?

[aromaa]

No, it would require going out of my way to get this to work properly again. This was working by hanging string and the changes made due to 1.13 finally broke it all

[andrewkm]

This should absolutely be a config option, not just stripped from the plugin as many of us rely on these flags and these minor issues are of no concern to us.

Sure disable it by default, but do not simply remove such a major flag from the plugin out right.
Absolutely must be a config. @aromaa

[aromaa]

I would rather remove a broken feature than keep it as broken. Use older version if you rely on these flags

[Phoenix616]

What exactly was causing this issue? As far as I can tell from the flag code it should've never influenced any block placements other than player placements?

EDIT: Oh I think I know what the issue here was: WorldGuard uses the AbstractBlockEvent#filter method to remove blocks in the case that multiple are placed (e.g. on structure grow) outside of the allowed region. But WGEF would listen on that event before that filter was run (lowest priority) while WorldGuard only applied that on normal priority. And WGEF explicitly allowed the event if its flags where right and the WorldGuard filter wouldn't even be applied.

Changing the priority of the WGEF listener above normal (and also not resetting the result to the original as that seems pointless imo?) should be able to solve this in WG7.

[aromaa]

The underlying problem is more complex and is due to the introduction of BlockState in 1.13. The StructureGrowEvent returns List<BlockState> which the AbstractBlockEvent consumes. When the #getBlocks method is called, a lazy loaded List<Block> is generated and causing side-effects in the #filter method code. When the blocks list is generated, the #filter method loops over the lazy loaded list instead of the list provided by the event, causing all the changes to be discarded and thus allowing the placement of blocks inside a region.

I did discuss about this a bit with the EngineHub folks but ended up deciding to not do anything about it and instead removed it altogether. There are few ways to "fix" this but did not end up going that round.

[Phoenix616]

As far as I can tell it will loop over the correct list, the lazy list is only created from the block states if no blocks list exists?

I also don't see how simply changing the priority doesn't solve this already.

[aromaa]

If the priority is changed to be after WorldGuard, it has removed all the blocks from the list that were denied, making the list in most cases empty and no way to know what there were originally

[aromaa]

These flags might be soon available as part of WorldGuard and this is finally resolved. EngineHub/WorldGuard#1778