From 36d4483353c5ddcd071509d1e21533f125240d44 Mon Sep 17 00:00:00 2001 From: stonar96 <minecraft.stonar96@gmail.com> Date: Thu, 13 Oct 2022 18:16:20 +0200 Subject: [PATCH 1/2] Fix, simplify and optimize AbstractRegionOverlapAssociation --- .../AbstractRegionOverlapAssociation.java | 79 ++++++++++--------- .../worldguard/protection/flags/Flags.java | 2 +- .../flags/registry/SimpleFlagRegistry.java | 7 +- 3 files changed, 47 insertions(+), 41 deletions(-) diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/association/AbstractRegionOverlapAssociation.java b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/association/AbstractRegionOverlapAssociation.java index 0cbd78fe0..3ca155581 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/association/AbstractRegionOverlapAssociation.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/association/AbstractRegionOverlapAssociation.java @@ -24,12 +24,12 @@ import com.sk89q.worldguard.domains.Association; import com.sk89q.worldguard.protection.FlagValueCalculator; import com.sk89q.worldguard.protection.flags.Flags; +import com.sk89q.worldguard.protection.flags.StateFlag.State; import com.sk89q.worldguard.protection.regions.ProtectedRegion; import javax.annotation.Nullable; import java.util.Collection; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Set; @@ -37,9 +37,9 @@ public abstract class AbstractRegionOverlapAssociation implements RegionAssociab @Nullable protected Set<ProtectedRegion> source; - private boolean useMaxPriorityAssociation; + private boolean effectivelyEmpty; + private final boolean useMaxPriorityAssociation; private int maxPriority; - private Set<ProtectedRegion> maxPriorityRegions; protected AbstractRegionOverlapAssociation(@Nullable Set<ProtectedRegion> source, boolean useMaxPriorityAssociation) { this.source = source; @@ -48,36 +48,53 @@ protected AbstractRegionOverlapAssociation(@Nullable Set<ProtectedRegion> source protected void calcMaxPriority() { checkNotNull(source); - int best = 0; - Set<ProtectedRegion> bestRegions = new HashSet<>(); + boolean effectivelyEmpty = true; + int maxPriority = Integer.MIN_VALUE; + for (ProtectedRegion region : source) { int priority = region.getPriority(); - if (priority > best) { - best = priority; - bestRegions.clear(); - bestRegions.add(region); - } else if (priority == best) { - bestRegions.add(region); + + // Potential endless recurrence? No, because there is no region group flag. + if ((effectivelyEmpty || priority > maxPriority) && FlagValueCalculator.getEffectiveFlagOf(region, Flags.PASSTHROUGH, this) != State.ALLOW) { + effectivelyEmpty = false; + + if (useMaxPriorityAssociation) { + maxPriority = priority; + } else { + break; + } } } - this.maxPriority = best; - this.maxPriorityRegions = bestRegions; + + this.effectivelyEmpty = effectivelyEmpty; + this.maxPriority = maxPriority; } - private boolean checkNonplayerProtectionDomains(Iterable<? extends ProtectedRegion> source, Collection<?> domains) { - if (source == null || domains == null || domains.isEmpty()) { + private boolean checkNonplayerProtectionDomains(ProtectedRegion region) { + if (source.isEmpty()) { return false; } - for (ProtectedRegion region : source) { + // Potential endless recurrence? No, because there is no region group flag. + Set<String> domains = FlagValueCalculator.getEffectiveFlagOf(region, Flags.NONPLAYER_PROTECTION_DOMAINS, this); + + if (domains == null || domains.isEmpty()) { + return false; + } + + for (ProtectedRegion sourceRegion : source) { + if (sourceRegion.getPriority() < maxPriority) { + continue; + } + // Potential endless recurrence? No, because there is no region group flag. - Set<String> regionDomains = FlagValueCalculator.getEffectiveFlagOf(region, Flags.NONPLAYER_PROTECTION_DOMAINS, this); + Set<String> sourceDomains = FlagValueCalculator.getEffectiveFlagOf(sourceRegion, Flags.NONPLAYER_PROTECTION_DOMAINS, this); - if (regionDomains == null || regionDomains.isEmpty()) { + if (sourceDomains == null || sourceDomains.isEmpty()) { continue; } - if (!Collections.disjoint(regionDomains, domains)) { + if (!Collections.disjoint(sourceDomains, domains)) { return true; } } @@ -90,31 +107,15 @@ public Association getAssociation(List<ProtectedRegion> regions) { checkNotNull(source); for (ProtectedRegion region : regions) { while (region != null) { - if ((region.getId().equals(ProtectedRegion.GLOBAL_REGION) && source.isEmpty())) { + if (region.getId().equals(ProtectedRegion.GLOBAL_REGION) && effectivelyEmpty) { return Association.OWNER; } - if (source.contains(region)) { - if (useMaxPriorityAssociation) { - int priority = region.getPriority(); - if (priority == maxPriority) { - return Association.OWNER; - } - } else { - return Association.OWNER; - } - } - - Set<ProtectedRegion> source; - - if (useMaxPriorityAssociation) { - source = maxPriorityRegions; - } else { - source = this.source; + if (source.contains(region) && region.getPriority() >= maxPriority) { + return Association.OWNER; } - // Potential endless recurrence? No, because there is no region group flag. - if (checkNonplayerProtectionDomains(source, FlagValueCalculator.getEffectiveFlagOf(region, Flags.NONPLAYER_PROTECTION_DOMAINS, this))) { + if (checkNonplayerProtectionDomains(region)) { return Association.OWNER; } diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/Flags.java b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/Flags.java index b8ccfb3fc..467666283 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/Flags.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/Flags.java @@ -45,7 +45,7 @@ public final class Flags { public static final List<String> INBUILT_FLAGS = Collections.unmodifiableList(INBUILT_FLAGS_LIST); // Overrides membership check - public static final StateFlag PASSTHROUGH = register(new StateFlag("passthrough", false)); + public static final StateFlag PASSTHROUGH = register(new StateFlag("passthrough", false, null)); public static final SetFlag<String> NONPLAYER_PROTECTION_DOMAINS = register(new SetFlag<>("nonplayer-protection-domains", null, new StringFlag(null))); // This flag is unlike the others. It forces the checking of region membership diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/registry/SimpleFlagRegistry.java b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/registry/SimpleFlagRegistry.java index 741d17491..e7326e864 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/registry/SimpleFlagRegistry.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/registry/SimpleFlagRegistry.java @@ -24,6 +24,7 @@ import com.google.common.collect.Maps; import com.sk89q.worldguard.protection.flags.Flag; import com.sk89q.worldguard.protection.flags.Flags; +import com.sk89q.worldguard.protection.flags.RegionGroupFlag; import javax.annotation.Nullable; import java.util.Collection; @@ -162,7 +163,11 @@ public Map<Flag<?>, Object> unmarshal(Map<String, Object> rawValues, boolean cre values.put(unk, entry.getValue()); } } else { - values.put(parent.getRegionGroupFlag(), parent.getRegionGroupFlag().unmarshal(entry.getValue())); + RegionGroupFlag regionGroupFlag = parent.getRegionGroupFlag(); + + if (regionGroupFlag != null) { + values.put(regionGroupFlag, regionGroupFlag.unmarshal(entry.getValue())); + } } } From 19016921785c4396c38d87c28f344c79a09c3ad9 Mon Sep 17 00:00:00 2001 From: stonar96 <minecraft.stonar96@gmail.com> Date: Wed, 19 Oct 2022 03:05:46 +0200 Subject: [PATCH 2/2] Handle non-existent region group flags smarter --- .../flags/registry/SimpleFlagRegistry.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/registry/SimpleFlagRegistry.java b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/registry/SimpleFlagRegistry.java index e7326e864..6b6ed0155 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/registry/SimpleFlagRegistry.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/registry/SimpleFlagRegistry.java @@ -24,7 +24,10 @@ import com.google.common.collect.Maps; import com.sk89q.worldguard.protection.flags.Flag; import com.sk89q.worldguard.protection.flags.Flags; +import com.sk89q.worldguard.protection.flags.RegionGroup; import com.sk89q.worldguard.protection.flags.RegionGroupFlag; +import com.sk89q.worldguard.protection.flags.StateFlag; +import com.sk89q.worldguard.protection.flags.StateFlag.State; import javax.annotation.Nullable; import java.util.Collection; @@ -167,6 +170,18 @@ public Map<Flag<?>, Object> unmarshal(Map<String, Object> rawValues, boolean cre if (regionGroupFlag != null) { values.put(regionGroupFlag, regionGroupFlag.unmarshal(entry.getValue())); + } else { + log.warning("Found non-existent region group flag '" + entry.getKey() + "' with value '" + entry.getValue() + "' for flag '" + parent.getName() + "'"); + log.warning("The region group flag '" + entry.getKey() + "' with value '" + entry.getValue() + "' will be removed"); + regionGroupFlag = new RegionGroupFlag("dummy", RegionGroup.ALL); + RegionGroup regionGroup = regionGroupFlag.unmarshal(entry.getValue()); + + if (regionGroup != null && regionGroup != RegionGroup.ALL) { + if (!(parent instanceof StateFlag) || values.get(parent) == State.ALLOW) { + log.warning("For safety reasons the flag '" + parent.getName() + "' with value '" + values.get(parent) + "' will also be removed"); + values.remove(parent); + } + } } } }