Skip redundant checks within BlockBreak and BlockPlace.

Reach and Direction are near-identical to the variants in BlockInteract
and will be
replaced by implementing an abstract class.
This commit is contained in:
asofold 2017-05-02 20:05:03 +02:00
parent 3ff207d487
commit bd370ba633
5 changed files with 129 additions and 41 deletions

View File

@ -33,6 +33,7 @@ import fr.neatmonster.nocheatplus.NCPAPIProvider;
import fr.neatmonster.nocheatplus.checks.CheckListener;
import fr.neatmonster.nocheatplus.checks.CheckType;
import fr.neatmonster.nocheatplus.checks.blockinteract.BlockInteractData;
import fr.neatmonster.nocheatplus.checks.blockinteract.BlockInteractListener;
import fr.neatmonster.nocheatplus.checks.inventory.Items;
import fr.neatmonster.nocheatplus.checks.moving.util.MovingUtil;
import fr.neatmonster.nocheatplus.compat.AlmostBoolean;
@ -85,7 +86,7 @@ public class BlockBreakListener extends CheckListener {
*/
@EventHandler(ignoreCancelled = false, priority = EventPriority.LOWEST)
public void onBlockBreak(final BlockBreakEvent event) {
final long now = System.currentTimeMillis();
final Player player = event.getPlayer();
// Illegal enchantments hotfix check.
@ -108,7 +109,6 @@ public class BlockBreakListener extends CheckListener {
final Block block = event.getBlock();
boolean cancelled = false;
// Do the actual checks, if still needed. It's a good idea to make computationally cheap checks first, because
@ -116,7 +116,14 @@ public class BlockBreakListener extends CheckListener {
final BlockBreakConfig cc = BlockBreakConfig.getConfig(player);
final BlockBreakData data = BlockBreakData.getData(player);
final long now = System.currentTimeMillis();
final BlockInteractData bdata = BlockInteractData.getData(player);
/*
* Re-check if this is a block interacted with before. With instantly
* broken blocks, this may be off by one orthogonally.
*/
final boolean isInteractBlock = !bdata.getLastIsCancelled() && bdata.matchesLastBlock(block);
int skippedRedundantChecks = 0;
final GameMode gameMode = player.getGameMode();
@ -132,7 +139,8 @@ public class BlockBreakListener extends CheckListener {
}
// Has the player broken blocks faster than possible?
if (!cancelled && gameMode != GameMode.CREATIVE && fastBreak.isEnabled(player) && fastBreak.check(player, block, isInstaBreak, cc, data)) {
if (!cancelled && gameMode != GameMode.CREATIVE
&& fastBreak.isEnabled(player) && fastBreak.check(player, block, isInstaBreak, cc, data)) {
cancelled = true;
}
@ -142,14 +150,25 @@ public class BlockBreakListener extends CheckListener {
}
// Is the block really in reach distance?
if (!cancelled && reach.isEnabled(player) && reach.check(player, block, data)) {
cancelled = true;
if (!cancelled) {
if (isInteractBlock && bdata.isPassedCheck(CheckType.BLOCKINTERACT_REACH)) {
skippedRedundantChecks ++;
}
else if (reach.isEnabled(player) && reach.check(player, block, data)) {
cancelled = true;
}
}
// Did the player look at the block at all?
// TODO: Skip if checks were run on this block (all sorts of hashes/conditions).
if (!cancelled && direction.isEnabled(player) && direction.check(player, block, data)) {
cancelled = true;
if (!cancelled) {
if (isInteractBlock && (bdata.isPassedCheck(CheckType.BLOCKINTERACT_DIRECTION)
|| bdata.isPassedCheck(CheckType.BLOCKINTERACT_VISIBLE))) {
skippedRedundantChecks ++;
}
else if (direction.isEnabled(player) && direction.check(player, block, data)) {
cancelled = true;
}
}
// Destroying liquid blocks.
@ -168,13 +187,12 @@ public class BlockBreakListener extends CheckListener {
data.clickedY = block.getY();
data.clickedZ = block.getZ();
}
else{
else {
// Invalidate last damage position:
// data.clickedX = Integer.MAX_VALUE;
// Debug log (only if not cancelled, to avoid spam).
if (data.debug) {
debug(player, "Block break(" + block.getType() + "): " + block.getX() + ", " + block.getY() + ", " + block.getZ());
debugBlockVSBlockInteract(player, block, "onBlockBreak");
debugBlockBreakResult(player, block, skippedRedundantChecks);
}
}
@ -191,6 +209,15 @@ public class BlockBreakListener extends CheckListener {
isInstaBreak = AlmostBoolean.NO;
}
private void debugBlockBreakResult(final Player player, final Block block, final int skippedRedundantChecks) {
debug(player, "Block break(" + block.getType() + "): " + block.getX() + ", " + block.getY() + ", " + block.getZ());
BlockInteractListener.debugBlockVSBlockInteract(player, checkType, block, "onBlockBreak",
Action.LEFT_CLICK_BLOCK);
if (skippedRedundantChecks > 0) {
debug(player, "Skipped redundant checks: " + skippedRedundantChecks);
}
}
/**
* We listen to PlayerAnimation events because it is (currently) equivalent to "player swings arm" and we want to
* check if they did that between block breaks.
@ -230,11 +257,16 @@ public class BlockBreakListener extends CheckListener {
@EventHandler(ignoreCancelled = false, priority = EventPriority.LOWEST)
public void onBlockDamageLowest(final BlockDamageEvent event) {
/*
* TODO: Add a check type BLOCKDAMAGE_CONFIRM (no permission):
* Cancel if the block doesn't match (MC 1.11.2, other ...)?
*/
if (MovingUtil.hasScheduledPlayerSetBack(event.getPlayer())) {
event.setCancelled(true);
}
else if (event.getInstaBreak()) {
// Indicate that this might have been set by CB/MC.
// TODO: Set in BlockInteractListener !!
isInstaBreak = AlmostBoolean.MAYBE;
}
}
@ -273,6 +305,7 @@ public class BlockBreakListener extends CheckListener {
// Skip if already set to the same block without breaking within one tick difference.
final ItemStack stack = Bridge1_9.getItemInMainHand(player);
final Material tool = stack == null ? null: stack.getType();
if (data.toolChanged(tool)) {
// Update.
} else if (tick < data.clickedTick || now < data.fastBreakfirstDamage || now < data.fastBreakBreakTime) {
@ -289,30 +322,11 @@ public class BlockBreakListener extends CheckListener {
data.setClickedBlock(block, tick, now, tool);
// Compare with BlockInteract data (debug first).
if (data.debug) {
debugBlockVSBlockInteract(player, block, "checkBlockDamage");
BlockInteractListener.debugBlockVSBlockInteract(player, this.checkType, block, "checkBlockDamage",
Action.LEFT_CLICK_BLOCK);
}
}
private void debugBlockVSBlockInteract(final Player player, final Block block, final String prefix) {
final BlockInteractData bdata = BlockInteractData.getData(player);
final int manhattan = bdata.manhattanLastBlock(block);
String msg;
if (manhattan == Integer.MAX_VALUE) {
msg = "no last block set!";
}
else {
msg = manhattan == 0 ? "same as last block."
: ("last block differs, Manhattan: " + manhattan);
if (bdata.getLastIsCancelled()) {
msg += " / cancelled";
}
if (bdata.getLastAction() != Action.LEFT_CLICK_BLOCK) {
msg += " / action=" + bdata.getLastAction();
}
}
debug(player, prefix + " BlockInteract: " + msg);
}
@EventHandler(ignoreCancelled = false, priority = EventPriority.MONITOR)
public void onItemHeld(final PlayerItemHeldEvent event) {
// Reset clicked block.

View File

@ -60,7 +60,11 @@ public class WrongBlock extends Check {
}
else if (dist == 1) {
// One might to a concession in case of instant breaking.
// TODO: WHY ?
if (now - data.wasInstaBreak < 60) {
if (data.debug) {
debug(player, "Skip on Manhattan 1 and wasInstaBreak within 60 ms.");
}
wrongBlock = false;
}
else {

View File

@ -14,6 +14,8 @@
*/
package fr.neatmonster.nocheatplus.checks.blockinteract;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
@ -310,6 +312,14 @@ public class BlockInteractData extends ACheckData {
return passedChecks.contains(checkType);
}
/**
*
* @return Unmodifiable collection.
*/
public Collection<CheckType> getPassedChecks() {
return Collections.unmodifiableCollection(passedChecks);
}
/**
* Check if the last block was set to be consumed by this check type.
*
@ -337,6 +347,7 @@ public class BlockInteractData extends ACheckData {
*/
public void setPlayerInteractEventResolution(final PlayerInteractEvent event) {
if (event.isCancelled()) {
// TODO: resetPassedChecks() ?
lastIsCancelled = true;
lastAllowUseItem = event.useItemInHand() == Result.ALLOW;
lastAllowUseBlock = event.useInteractedBlock() == Result.ALLOW;

View File

@ -38,6 +38,7 @@ import fr.neatmonster.nocheatplus.compat.Bridge1_9;
import fr.neatmonster.nocheatplus.compat.BridgeHealth;
import fr.neatmonster.nocheatplus.compat.BridgeMisc;
import fr.neatmonster.nocheatplus.stats.Counters;
import fr.neatmonster.nocheatplus.utilities.CheckUtils;
import fr.neatmonster.nocheatplus.utilities.InventoryUtil;
import fr.neatmonster.nocheatplus.utilities.TickTask;
import fr.neatmonster.nocheatplus.utilities.location.LocUtil;
@ -50,6 +51,29 @@ import fr.neatmonster.nocheatplus.utilities.map.BlockProperties;
*/
public class BlockInteractListener extends CheckListener {
public static void debugBlockVSBlockInteract(final Player player, final CheckType checkType,
final Block block, final String prefix, final Action expectedAction) {
final BlockInteractData bdata = BlockInteractData.getData(player);
final int manhattan = bdata.manhattanLastBlock(block);
String msg;
if (manhattan == Integer.MAX_VALUE) {
msg = "no last block set!";
}
else {
msg = manhattan == 0 ? "same as last block."
: ("last block differs, Manhattan: " + manhattan);
if (bdata.getLastIsCancelled()) {
msg += " / cancelled";
}
if (bdata.getLastAction() != expectedAction) {
msg += " / action=" + bdata.getLastAction();
}
}
CheckUtils.debug(player, checkType, prefix + " BlockInteract: " + msg);
}
// INSTANCE ----
/** The looking-direction check. */
private final Direction direction = addCheck(new Direction());
@ -82,10 +106,10 @@ public class BlockInteractListener extends CheckListener {
* the event
*/
@EventHandler(ignoreCancelled = false, priority = EventPriority.LOWEST)
protected void onPlayerInteract(final PlayerInteractEvent event) {
public void onPlayerInteract(final PlayerInteractEvent event) {
final Player player = event.getPlayer();
final BlockInteractData data = BlockInteractData.getData(player);
data.resetPassedChecks();
data.resetLastBlock();
// Early cancel for interact events with dead players and other.
final int cancelId;
if (player.isDead() && BridgeHealth.getHealth(player) <= 0.0) { // TODO: Should be dead !?.
@ -107,7 +131,7 @@ public class BlockInteractListener extends CheckListener {
event.setUseInteractedBlock(Result.DENY);
event.setUseItemInHand(Result.DENY);
event.setCancelled(true);
data.resetLastBlock();
data.setPlayerInteractEventResolution(event);
if (cancelId >= 0) {
counters.addPrimaryThread(cancelId, 1);
}
@ -144,6 +168,7 @@ public class BlockInteractListener extends CheckListener {
}
break;
default:
data.setPlayerInteractEventResolution(event);
return;
}
@ -155,6 +180,7 @@ public class BlockInteractListener extends CheckListener {
}
else {
// Can't do more than prevent all (could: set to prevent on highest, if desired).
data.setPlayerInteractEventResolution(event);
return;
}
}
@ -218,6 +244,8 @@ public class BlockInteractListener extends CheckListener {
counters.addPrimaryThread(idInteractLookCurrent, 1);
}
}
// Set resolution here already:
data.setPlayerInteractEventResolution(event);
useLoc.setWorld(null);
}
@ -271,6 +299,11 @@ public class BlockInteractListener extends CheckListener {
final Player player = event.getPlayer();
final BlockInteractData data = BlockInteractData.getData(player);
data.setPlayerInteractEventResolution(event);
/*
* TODO: BlockDamageEvent fires before MONITOR level, BlockBreak after
* (!). Thus resolution is set on LOWEST already, probably should be
* HIGHEST to account for other plugins.
*/
// Elytra boost.
if (event.getAction() == Action.RIGHT_CLICK_AIR
&& event.isCancelled() && event.useItemInHand() != Result.DENY) {

View File

@ -35,6 +35,8 @@ import org.bukkit.inventory.ItemStack;
import fr.neatmonster.nocheatplus.NCPAPIProvider;
import fr.neatmonster.nocheatplus.checks.CheckListener;
import fr.neatmonster.nocheatplus.checks.CheckType;
import fr.neatmonster.nocheatplus.checks.blockinteract.BlockInteractData;
import fr.neatmonster.nocheatplus.checks.blockinteract.BlockInteractListener;
import fr.neatmonster.nocheatplus.checks.combined.Combined;
import fr.neatmonster.nocheatplus.checks.combined.CombinedConfig;
import fr.neatmonster.nocheatplus.checks.combined.Improbable;
@ -146,6 +148,9 @@ public class BlockPlaceListener extends CheckListener {
final BlockPlaceData data = BlockPlaceData.getData(player);
final BlockPlaceConfig cc = BlockPlaceConfig.getConfig(player);
final BlockInteractData bdata = BlockInteractData.getData(player);
final boolean isInteractBlock = !bdata.getLastIsCancelled() && bdata.matchesLastBlock(blockAgainst);
int skippedRedundantChecks = 0;
final boolean shouldSkipSome;
if (blockMultiPlaceEvent != null && event.getClass() == blockMultiPlaceEvent) {
@ -169,7 +174,7 @@ public class BlockPlaceListener extends CheckListener {
// Always hash as sign post for improved compatibility with Lockette etc.
data.autoSignPlacedHash = getBlockPlaceHash(block, Material.SIGN);
}
// Don't run checks, if a set back is scheduled.
if (!cancelled && MovingUtil.hasScheduledPlayerSetBack(player)) {
cancelled = true;
@ -193,13 +198,23 @@ public class BlockPlaceListener extends CheckListener {
}
// Reach check (distance).
if (!cancelled && !shouldSkipSome && reach.isEnabled(player) && reach.check(player, block, data, cc)) {
cancelled = true;
if (!cancelled && !shouldSkipSome) {
if (isInteractBlock && bdata.isPassedCheck(CheckType.BLOCKINTERACT_REACH)) {
skippedRedundantChecks ++;
}
else if (reach.isEnabled(player) && reach.check(player, block, data, cc)) {
cancelled = true;
}
}
// Direction check.
if (!cancelled && !shouldSkipSome && direction.isEnabled(player) && direction.check(player, block, blockAgainst, data, cc)) {
cancelled = true;
if (!cancelled && !shouldSkipSome) {
if (isInteractBlock && bdata.isPassedCheck(CheckType.BLOCKINTERACT_DIRECTION)) {
skippedRedundantChecks ++;
}
else if (direction.isEnabled(player) && direction.check(player, block, blockAgainst, data, cc)) {
cancelled = true;
}
}
// Surrounding material.
@ -214,13 +229,24 @@ public class BlockPlaceListener extends CheckListener {
else {
// Debug log (only if not cancelled, to avoid spam).
if (data.debug) {
debug(player, "Block place(" + placedMat + "): " + block.getX() + ", " + block.getY() + ", " + block.getZ());
debugBlockPlace(player, placedMat, block, blockAgainst, skippedRedundantChecks);
}
}
// Cleanup
// Reminder(currently unused): useLoc.setWorld(null);
}
private void debugBlockPlace(final Player player, final Material placedMat,
final Block block, final Block blockAgainst,
final int skippedRedundantChecks) {
debug(player, "Block place(" + placedMat + "): " + block.getX() + ", " + block.getY() + ", " + block.getZ());
BlockInteractListener.debugBlockVSBlockInteract(player, checkType, blockAgainst, "onBlockInteract(blockAgainst)",
Action.RIGHT_CLICK_BLOCK);
if (skippedRedundantChecks > 0) {
debug(player, "Skipped redundant checks: " + skippedRedundantChecks);
}
}
@EventHandler(priority = EventPriority.LOWEST, ignoreCancelled = true)
public void onSignChange(final SignChangeEvent event) {
if (event.getClass() != SignChangeEvent.class) {