Fix, simplify and optimize AbstractRegionOverlapAssociation

[stonar96]

This pr fixes two issues:

1. This variable should be initialized with Integer.MIN_VALUE instead of 0.
2. An issue that Theosis#0001 talked about in this message on Discord.
   
   
   In general this can also happen with normal regions when the config setting use-max-priority-association is set to true (e.g. by overlapping a protected region and a higher priority region with passthrough set to allow). As noted in the docs, the global region always behaves as if use-max-priority-association is set to true (due to legacy reasons I guess). That's why this also happens for the global region, even if use-max-priority-association is set to false. My solution to this problem is to ignore regions with passthrough set to allow, when calculating the maximum priority here and later checking for >= here. Equivalently the global region is fixed by treating nonplayers as members not only when the source of regions is empty, but also when the source only contains regions with passthrough set to allow (which I call effectivelyEmpty).

I was also able to simplify and optimize some parts while fixing those issues and I have cleaned up a bit. E.g. #getAssociation(List<ProtectedRegion>) now uses the same logic, regardless of whether or not use-max-priority-association is enabled (it's just controlled by the calculated maximum priority). Also calculating the maximum priority returns fast if use-max-priority-association is disabled and it no longer allocates a new HashSet.

Unfortunately, checking flags (like passthrough) can itself depend on membership (#getAssociation), if there's an associated region group flag. This would lead to endless recursions when doing this while calculating the membership. That's why I have removed the passthrough region group flag, which explains the required changes in Flags and SimpleFlagRegistry in case someone has set it. However, I think setting a region group for the passthrough flag doesn't make much sense anyway.

[wizjany]

However, I think setting a region group for the passthrough flag doesn't make much sense anyway.

Unfortunately people who run minecraft servers aren't known for having much sense. I've seen a lot of people setting the group flag on passthrough and I'm unsure what implications this may have.
(Haven't really looked at the rest of the pr yet, just wanted to point that out early)

[stonar96]

I think, if this issue should be fixed, there isn't really a way around removing the region group flag for the passthrough flag. At least with the logic that is introduced with this pr (membership depends on passthrough), I'd say that region groups just don't make sense anymore here (neither was there really a use case in the past).

However, you are right that there could currently be (probably dumb and overly complex) region setups that will lose protection when a build including the changes of this pr is used. I also agree that this is unacceptable (especially considering that you've "seen a lot of people setting the group flag on passthrough"). The only solution I see to avoid this is to be smarter in the way handling the case where the region group flag has been set.

As a general rule for parsing state flags without a region group flag, the following logic could work:

• If the (non existing) region group flag of the state flag state-x has been set to "all":
  • Remove the region group flag (and keep state-x).
• If the (non existing) region group flag of the state flag state-x has been set to a value != "all":
  • If the set value of state-x is "deny":
    • Remove the region group flag (and keep state-x).
  • If the set value of state-x is "allow":
    • Remove state-x (and of course remove the region group flag too).

I think with this logic we can avoid that protection is lost. It could lead to stronger protection in case someone has set the region group flag, which must then be fixed manually if not wanted. I think that would be acceptable behavior.

[stonar96]

I've implemented the logic described in my comment above. This is ready for review.

[wizjany]

in the months since this was opened i've seen 2 more people using the group flag on passthrough. as weird as it really is, it does currently work as intended and is "guaranteed behavior" by the general documentation on how flags work. while the most recent commit makes the transition "safer", i still think this is a breaking change that probably deserves to go into WG8 instead of a 7.x release.

re-reading the original issue that spurred this (and the current docs on non-players in global), couldn't the normal regions and the global region just share a nonplayer protection domain and then the pistons would work again (inside the "normal region"), or am i misunderstanding something?

[stonar96]

in the months since this was opened i've seen 2 more people using the group flag on passthrough. as weird as it really is, it does currently work as intended and is "guaranteed behavior" by the general documentation on how flags work. while the most recent commit makes the transition "safer", i still think this is a breaking change that probably deserves to go into WG8 instead of a 7.x release.

Ofc this must be documented, for example here in the blue tip box and maybe here too. WG8 is fine for me.

re-reading the original issue that spurred this (and the current docs on non-players in global), couldn't the normal regions and the global region just share a nonplayer protection domain and then the pistons would work again (inside the "normal region"), or am i misunderstanding something?

Yes, that's what I told them as a workaround, see https://discord.com/channels/80853743897149440/470410381597343753/869586730817298503.