Fix region equality across multiple region managers. (#395)

It is possible, for example when teleporting between two worlds, that
regions in two different managers are compared. If the regions in each
world have the same name, they would return equal. Removing the equals
override will prevent two different regions from seeming equal even if
they are not.
This commit is contained in:
wizjany 2019-03-01 21:42:07 -05:00 committed by GitHub
parent 43d8cf8cba
commit af93530dbe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 24 additions and 16 deletions

View File

@ -6,6 +6,10 @@
<!-- Tabs are strictly banned -->
<module name="FileTabCharacter"/>
<module name="SuppressionFilter">
<property name="file" value="${basedir}/config/checkstyle/suppressions.xml"/>
</module>
<module name="TreeWalker">
<!-- Important basics -->
<!-- <module name="PackageDeclaration"/> Unlikely that we would miss this in a PR -->

View File

@ -0,0 +1,7 @@
<?xml version="1.0"?>
<!DOCTYPE suppressions PUBLIC
"-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN"
"https://checkstyle.org/dtds/suppressions_1_2.dtd">
<suppressions>
<suppress files="ProtectedRegion.java" checks="EqualsHashCode"/>
</suppressions>

View File

@ -299,7 +299,7 @@ public abstract class ProtectedRegion implements ChangeTracked, Comparable<Prote
*
* @param playerName player name to check
* @return whether an owner
* @deprecated Names are deprecated
* @deprecated Names are deprecated, this will not return owners added by UUID (LocalPlayer)
*/
@Deprecated
public boolean isOwner(String playerName) {
@ -335,20 +335,7 @@ public abstract class ProtectedRegion implements ChangeTracked, Comparable<Prote
return true;
}
if (members.contains(player)) {
return true;
}
ProtectedRegion curParent = getParent();
while (curParent != null) {
if (curParent.getMembers().contains(player)) {
return true;
}
curParent = curParent.getParent();
}
return false;
return isMemberOnly(player);
}
/**
@ -357,7 +344,7 @@ public abstract class ProtectedRegion implements ChangeTracked, Comparable<Prote
*
* @param playerName player name to check
* @return whether an owner or member
* @deprecated Names are deprecated
* @deprecated Names are deprecated, this will not return players added by UUID (LocalPlayer)
*/
@Deprecated
public boolean isMember(String playerName) {
@ -713,6 +700,15 @@ public abstract class ProtectedRegion implements ChangeTracked, Comparable<Prote
return id.hashCode();
}
// ** This equals doesn't take the region manager into account (since it's not available anyway)
// ** and thus will return equality for two regions with the same name in different worlds (or even
// ** no world at all, such as when testing intersection with a dummy region). Thus, we keep the
// ** hashCode method to improve hashset operation (which is fine within one manager - ids are unique)
// ** and will only leave a collision when regions in different managers with the same id are tested.
// ** In that case, they are checked for reference equality. Note that is it possible to programmatically
// ** add the same region object to two different managers. If someone uses the API in this way, it is expected
// ** that the regions be the same reference in both worlds. (Though that might lead to other odd behavior)
/*
@Override
public boolean equals(Object obj) {
if (!(obj instanceof ProtectedRegion)) {
@ -722,6 +718,7 @@ public abstract class ProtectedRegion implements ChangeTracked, Comparable<Prote
ProtectedRegion other = (ProtectedRegion) obj;
return other.getId().equals(getId());
}
*/
@Override
public String toString() {