Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix potential NPE visualizing conflict zones #1854

Merged
merged 4 commits into from May 25, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -21,7 +21,7 @@
public class BoundaryVisualizationEvent extends PlayerEvent
{

private static final VisualizationProvider DEFAULT_PROVIDER = (world, visualizeFrom, height) ->
public static final VisualizationProvider DEFAULT_PROVIDER = (world, visualizeFrom, height) ->
{
if (GriefPrevention.instance.config_visualizationAntiCheatCompat)
{
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/griefprevention/util/IntVector.java
Expand Up @@ -46,7 +46,7 @@ public IntVector(@NotNull Vector vector)
}

/**
* Get ae {@link Block} representing the {@code IntVector} in the specified {@link World}.
* Get a {@link Block} representing the {@code IntVector} in the specified {@link World}.
*
* @param world the {@code World}
* @return the corresponding {@code Block}
Expand Down
@@ -1,6 +1,7 @@
package com.griefprevention.visualization;

import me.ryanhamshire.GriefPrevention.Claim;
import me.ryanhamshire.GriefPrevention.CustomLogEntryTypes;
import me.ryanhamshire.GriefPrevention.GriefPrevention;
import me.ryanhamshire.GriefPrevention.PlayerData;
import com.griefprevention.events.BoundaryVisualizationEvent;
Expand All @@ -18,6 +19,7 @@
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;
import java.util.logging.Level;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -206,6 +208,8 @@ private static void visualizeClaim(
*/
private static Collection<Boundary> defineBoundaries(Claim claim, VisualizationType type)
{
if (claim == null) return Set.of();

// For single claims, always visualize parent and children.
if (claim.parent != null) claim = claim.parent;

Expand Down Expand Up @@ -264,9 +268,55 @@ public static void callAndVisualize(@NotNull BoundaryVisualizationEvent event) {
{
GriefPrevention.instance.getServer().getScheduler().scheduleSyncDelayedTask(
GriefPrevention.instance,
() -> visualization.apply(player, playerData),
new DelayedVisualizationTask(visualization, playerData, event),
1L);
}
}

private record DelayedVisualizationTask(
@NotNull BoundaryVisualization visualization,
@NotNull PlayerData playerData,
@NotNull BoundaryVisualizationEvent event)
implements Runnable
{

@Override
public void run()
{
try
{
visualization.apply(event.getPlayer(), playerData);
}
catch (Exception exception)
{
if (event.getProvider() == BoundaryVisualizationEvent.DEFAULT_PROVIDER)
{
// If the provider is our own, log normally.
GriefPrevention.instance.getLogger().log(Level.WARNING, "Exception visualizing claim", exception);
return;
}

// Otherwise, add an extra hint that the problem is not with GP.
GriefPrevention.AddLogEntry(
String.format(
"External visualization provider %s caused %s: %s",
event.getProvider().getClass().getName(),
exception.getClass().getName(),
exception.getCause()),
CustomLogEntryTypes.Exception);
GriefPrevention.instance.getLogger().log(
Level.WARNING,
"Exception visualizing claim using external provider",
exception);

// Fall through to default provider.
BoundaryVisualization fallback = BoundaryVisualizationEvent.DEFAULT_PROVIDER
.create(event.getPlayer().getWorld(), event.getCenter(), event.getHeight());
event.getBoundaries().stream().filter(Objects::nonNull).forEach(fallback.elements::add);
fallback.apply(event.getPlayer(), playerData);
}
}

}

}
Expand Up @@ -18,12 +18,14 @@

package me.ryanhamshire.GriefPrevention;

import org.jetbrains.annotations.Nullable;

public class CreateClaimResult
{
//whether or not the creation succeeded (it would fail if the new claim overlapped another existing claim)
public boolean succeeded;

//when succeeded, this is a reference to the new claim
//when failed, this is a reference to the pre-existing, conflicting claim
public Claim claim;
public @Nullable Claim claim;
}
Expand Up @@ -1474,7 +1474,7 @@ void resizeClaimWithChecks(Player player, PlayerData playerData, int newx1, int
newClaim.getGreaterBoundaryCorner().getBlockZ(),
player);

if (result.succeeded)
if (result.succeeded && result.claim != null)
{
//decide how many claim blocks are available for more resizing
int claimBlocksRemaining = 0;
Expand Down
Expand Up @@ -1079,7 +1079,7 @@ public boolean onCommand(CommandSender sender, Command cmd, String commandLabel,
gc.getWorld().getHighestBlockYAt(gc) - GriefPrevention.instance.config_claims_claimsExtendIntoGroundDistance - 1,
lc.getBlockZ(), gc.getBlockZ(),
player.getUniqueId(), null, null, player);
if (!result.succeeded)
if (!result.succeeded || result.claim == null)
{
if (result.claim != null)
{
Expand Down
Expand Up @@ -2428,10 +2428,17 @@ else if (playerData.shovelMode == ShovelMode.Subdivide)
null, player);

//if it didn't succeed, tell the player why
if (!result.succeeded)
if (!result.succeeded || result.claim == null)
{
GriefPrevention.sendMessage(player, TextMode.Err, Messages.CreateSubdivisionOverlap);
BoundaryVisualization.visualizeClaim(player, result.claim, VisualizationType.CONFLICT_ZONE, clickedBlock);
if (result.claim != null)
{
GriefPrevention.sendMessage(player, TextMode.Err, Messages.CreateSubdivisionOverlap);
BoundaryVisualization.visualizeClaim(player, result.claim, VisualizationType.CONFLICT_ZONE, clickedBlock);
}
else
{
GriefPrevention.sendMessage(player, TextMode.Err, Messages.CreateClaimFailOverlapRegion);
}

return;
}
Expand Down Expand Up @@ -2570,7 +2577,7 @@ else if (playerData.shovelMode == ShovelMode.Subdivide)
player);

//if it didn't succeed, tell the player why
if (!result.succeeded)
if (!result.succeeded || result.claim == null)
{
if (result.claim != null)
{
Expand Down