Reworking protection system (test-event & move-location check) #545
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
With this PR I would like to suggest a different approach to checking building rights. Querying all rights plugins is currently inperformant and not completely correct due to the different scenarios (e.g. misc flags like
block-build
on WorldGuard; custom flags for WorldGuard & PlotSquared via add-ons; or custom features via add-ons for BentoBox). At the end, we are interested in whether the player is allowed to build at the corresponding location or not, right? So, I used a test event to check whether the player is allowed to build at the current Armorstand position and whether he is allowed to build at the target location of a Move event (if performed).I can see you had a lot of work to support the large number of protection plugins, but the current test seems to work well. The new check may be more correct for some cases, as it simply reflects the test result and does not refer to the theoretical processes of the individual restriction plugins.
Some other notes
By movements in the Y-achse the "lower block" is checked here. This is the same behavior as normal building protections (e.g. with WorldGuard).
The only weak point of this concept is that logging systems log the test-event. For example, it's logged by CoreProtect on location without AIR, by default. I don't see any good way to get around this.
Once the restriction has been tested for a while, we can minimize the
/protections
folder. Only the permission check ofasedit.ignoreProtection.<plugin>
would then be necessary. (An OP check is not necessary, as the permission is already queried)./src/main/java/io/github/rypofalem/armorstandeditor/protections/WorldGuardProtection.java#L48-L49
I would like to point out that I am primarily looking for the best and safest option. In this case, it also seems to be the simplest solution. Provided it works equally well with all supported plugins.
[CORE] Changes
Changes to the core of the plugin - Performance Fixes, Bug Fixes, New Features, New Permission Nodes, New Config Options etc.
BlockPlaceEvent
test event (performance and restriction improvements)canEdit
check for Move and Reverse-Move to prevent armorstands from leaving the region[CI] Changes
Changes relating to the Continuous Integration of other Plugin APIs, Github Workflows, Issue Templates etc.
[DOC] Changes
Changes relating to plugin Documentation - See the Wiki for more info
[MISC] Changes
Changes that does not fit in the above list
Dev-Test
Tested with:
1.20.6
ArmorStandEditor-Reborn v. 1.20.6-46.2
PlotSquared v. 7.3.8
andWorldGuard v. 7.0.9
forX
,Y
andZ
movements of armorstandsI am not familiar with all of the supported protection systems. Please feel free to report any errors you find with other plugins.
By making this pull request, I represent that I have the right to waive copyright and related rights to my contribution, and agree that all copyright and related rights in my contributions are waived, and I acknowledge that the ArmorStandEditor Project Owners have the copyright to use and modify my contribution under the ArmorStandEditor License for perpetuity.