[BREAKING] Simplify Check.executeActions, remove getParameterMap.

* The primary thread is now detected in Check.executeActions.
* ViolationData.chainParameter allows for one-statement action-execution
with chaining.
This commit is contained in:
asofold 2015-01-30 16:33:11 +01:00
parent 86732764e3
commit 586013a907
15 changed files with 117 additions and 170 deletions

View File

@ -85,7 +85,7 @@ public class FlyingFrequency extends PacketAdapter {
// TODO: Consider using the NetStatic check.
data.flyingFrequencyAll.add(time, 1f);
final float allScore = data.flyingFrequencyAll.score(1f);
if (allScore / cc.flyingFrequencySeconds > cc.flyingFrequencyPPS && !frequency.hasBypass(player) && frequency.executeActions(player, allScore / cc.flyingFrequencySeconds - cc.flyingFrequencyPPS, 1.0 / cc.flyingFrequencySeconds, cc.flyingFrequencyActions, true)) {
if (allScore / cc.flyingFrequencySeconds > cc.flyingFrequencyPPS && !frequency.hasBypass(player) && frequency.executeActions(player, allScore / cc.flyingFrequencySeconds - cc.flyingFrequencyPPS, 1.0 / cc.flyingFrequencySeconds, cc.flyingFrequencyActions)) {
event.setCancelled(true);
return;
}
@ -179,7 +179,7 @@ public class FlyingFrequency extends PacketAdapter {
if (allScore / cc.flyingFrequencySeconds > 20f && !frequency.hasBypass(player)) {
// (Must re-check bypass here.)
data.flyingFrequencyRedundantFreq.add(time, 1f);
if (frequency.executeActions(player, data.flyingFrequencyRedundantFreq.score(1f) / cc.flyingFrequencyRedundantSeconds, 1.0 / cc.flyingFrequencyRedundantSeconds, cc.flyingFrequencyRedundantActions, true)) {
if (frequency.executeActions(player, data.flyingFrequencyRedundantFreq.score(1f) / cc.flyingFrequencyRedundantSeconds, 1.0 / cc.flyingFrequencyRedundantSeconds, cc.flyingFrequencyRedundantActions)) {
return true;
}
}

View File

@ -51,6 +51,10 @@ public abstract class Action <D extends ActionData, L extends AbstractActionList
/**
* Check if parameters are needed at all for faster processing.
* <hr>
* This method must be thread-safe, if an execution of this might happen
* outside of the primary thread.
*
* @return
*/
public boolean needsParameters() {

View File

@ -5,28 +5,28 @@ package fr.neatmonster.nocheatplus.actions;
* @author mc_dev
*
*/
public interface ParameterHolder extends ActionData{
/**
*
* @param parameterName
* @return Will always return some string, if not set: "<?PARAMETERNAME>".
*/
public String getParameter(final ParameterName parameterName);
/**
* This will set the parameter, even if needsParameters() returns false.
* @param parameterName
* @param value
*/
public void setParameter(final ParameterName parameterName, String value);
public interface ParameterHolder extends ActionData {
/**
*
* @param parameterName
* @return Will always return some string, if not set: "<?PARAMETERNAME>".
*/
public String getParameter(final ParameterName parameterName);
/**
* This will set the parameter, even if needsParameters() returns false.
* @param parameterName
* @param value
*/
public void setParameter(final ParameterName parameterName, String value);
/**
* Check if any of the actions needs parameters.
* @return If true, actions are likely to contain command or logging actions.
*/
public boolean needsParameters();
/**
* Check if any parameters are set (in case of special settings NCP might add parameters for debugging purposes.).
* @return

View File

@ -8,7 +8,6 @@ import org.bukkit.entity.Player;
import fr.neatmonster.nocheatplus.NCPAPIProvider;
import fr.neatmonster.nocheatplus.actions.ActionList;
import fr.neatmonster.nocheatplus.actions.ParameterName;
import fr.neatmonster.nocheatplus.compat.MCAccess;
import fr.neatmonster.nocheatplus.components.MCAccessHolder;
import fr.neatmonster.nocheatplus.hooks.NCPExemptionManager;
@ -20,7 +19,7 @@ import fr.neatmonster.nocheatplus.utilities.TickTask;
/**
* The parent class of all checks. Don't let this implement Listener without knowing that this might be registered as component with NCP before the check-listeners.
*/
public abstract class Check implements MCAccessHolder{
public abstract class Check implements MCAccessHolder {
// TODO: Do these get cleaned up ?
/** The execution histories of each check. */
@ -59,8 +58,7 @@ public abstract class Check implements MCAccessHolder{
}
/**
* Execute actions, possibly thread safe according to the isMainThread flag.<br>
* This does not use extra synchronization.
* Execute actions, thread-safe.<br>
*
* @param player
* the player
@ -72,50 +70,19 @@ public abstract class Check implements MCAccessHolder{
* if the thread the main thread
* @return true, if successful
*/
public boolean executeActions(final Player player, final double vL, final double addedVL,
final ActionList actions, boolean isMainThread) {
public boolean executeActions(final Player player, final double vL, final double addedVL, final ActionList actions) {
// Sync it into the main thread by using an event.
return executeActions(new ViolationData(this, player, vL, addedVL, actions), isMainThread);
return executeActions(new ViolationData(this, player, vL, addedVL, actions));
}
/**
* Convenience method: Execute actions from the main thread only.
*
* @param player
* the player
* @param vL
* the violation level
* @param addedVL
* the violation level added
* @param actions
* the actions
* @return true, if the event should be cancelled
*/
protected boolean executeActions(final Player player, final double vL, final double addedVL,
final ActionList actions) {
return executeActions(new ViolationData(this, player, vL, addedVL, actions), true);
}
/**
* Execute some actions for the specified player, only for the main thread.
* Execute actions, thread safe.
*
* @param violationData
* the violation data
* @return true, if the event should be cancelled
*/
protected boolean executeActions(final ViolationData violationData) {
return executeActions(violationData, true);
}
/**
* Execute some actions for the specified player.
*
* @param violationData
* the violation data
* @param isMainThread If this is called from within the main thread. If true, this must really be the main thread and not from synchronized code coming form another thread.
* @return true, if the event should be cancelled
*/
protected boolean executeActions(final ViolationData violationData, final boolean isMainThread) {
public boolean executeActions(final ViolationData violationData) {
// Dispatch the VL processing to the hook manager (now thread safe).
if (NCPHookManager.shouldCancelVLProcessing(violationData)) {
@ -125,7 +92,7 @@ public abstract class Check implements MCAccessHolder{
final boolean hasCancel = violationData.hasCancel();
if (isMainThread) {
if (Bukkit.isPrimaryThread()) {
return violationData.executeActions();
}
else {
@ -137,19 +104,6 @@ public abstract class Check implements MCAccessHolder{
return hasCancel;
}
/**
* Fill in parameters for creating violation data.
* Individual checks should override this to fill in check specific parameters,
* which then are fetched by the violation data instance.
* @param player
* @return
*/
protected Map<ParameterName, String> getParameterMap(final ViolationData violationData) {
final Map<ParameterName, String> params = new HashMap<ParameterName, String>();
// (Standard parameters like player, vl, check name are filled in in ViolationData.getParameter!)
return params;
}
/**
* Gets the type of the check.
*

View File

@ -20,7 +20,7 @@ import fr.neatmonster.nocheatplus.logging.StaticLog;
* TODO: Re-think visibility questions.
* @author asofold
*/
public class ViolationData implements IViolationInfo, ActionData{
public class ViolationData implements IViolationInfo, ActionData {
/** The actions to be executed. */
public final ActionList actions;
@ -41,12 +41,15 @@ public class ViolationData implements IViolationInfo, ActionData{
public final double vL;
/** Filled in parameters. */
private Map<ParameterName, String> parameters;
private Map<ParameterName, String> parameters = null;
private boolean needsParameters = false;
/**
* Instantiates a new violation data.
* <hr>
* This constructor must be thread-safe for checks that might be executed
* outside of the primary thread.
*
* @param check
* the check
@ -59,8 +62,7 @@ public class ViolationData implements IViolationInfo, ActionData{
* @param actions
* the actions
*/
public ViolationData(final Check check, final Player player, final double vL, final double addedVL,
final ActionList actions) {
public ViolationData(final Check check, final Player player, final double vL, final double addedVL, final ActionList actions) {
this.check = check;
this.player = player;
this.vL = vL;
@ -74,7 +76,6 @@ public class ViolationData implements IViolationInfo, ActionData{
break;
}
}
parameters = needsParameters ? check.getParameterMap(this) : null;
this.needsParameters = needsParameters;
}
@ -133,21 +134,21 @@ public class ViolationData implements IViolationInfo, ActionData{
return "<???>";
}
switch (parameterName) {
case CHECK:
return check.getClass().getSimpleName();
case IP:
return player.getAddress().toString().substring(1).split(":")[0];
case PLAYER:
case PLAYER_NAME:
return player.getName();
case PLAYER_DISPLAY_NAME:
return player.getDisplayName();
case UUID:
return player.getUniqueId().toString();
case VIOLATIONS:
return String.valueOf((long) Math.round(vL));
default:
break;
case CHECK:
return check.getClass().getSimpleName();
case IP:
return player.getAddress().toString().substring(1).split(":")[0];
case PLAYER:
case PLAYER_NAME:
return player.getName();
case PLAYER_DISPLAY_NAME:
return player.getDisplayName();
case UUID:
return player.getUniqueId().toString();
case VIOLATIONS:
return String.valueOf((long) Math.round(vL));
default:
break;
}
if (parameters == null) {
// Return what would have been parsed to get the parameter.
@ -158,13 +159,25 @@ public class ViolationData implements IViolationInfo, ActionData{
}
@Override
public void setParameter(final ParameterName parameterName, String value) {
public void setParameter(final ParameterName parameterName, final String value) {
if (parameters == null) {
parameters = new HashMap<ParameterName, String>();
}
parameters.put(parameterName, value);
}
/**
* Convenience method: Delegates to setParameter, return this for chaining.
*
* @param parameterName
* @param value
* @return This ViolationData instance.
*/
public ViolationData chainParameter(final ParameterName parameterName, final String value) {
setParameter(parameterName, value);
return this;
}
@Override
public boolean needsParameters() {
return needsParameters;

View File

@ -7,7 +7,7 @@ import fr.neatmonster.nocheatplus.actions.ParameterHolder;
* @author mc_dev
*
*/
public interface IViolationInfo extends ParameterHolder{
public interface IViolationInfo extends ParameterHolder {
/**
* Get the violation level just added by this violation.
* @return

View File

@ -1,7 +1,5 @@
package fr.neatmonster.nocheatplus.checks.blockbreak;
import java.util.Map;
import org.bukkit.GameMode;
import org.bukkit.Location;
import org.bukkit.block.Block;
@ -17,9 +15,9 @@ import fr.neatmonster.nocheatplus.utilities.TrigUtil;
* The Reach check will find out if a player interacts with something that's too far away.
*/
public class Reach extends Check {
/** For temporary use: LocUtil.clone before passing deeply, call setWorld(null) after use. */
private final Location useLoc = new Location(null, 0, 0, 0);
/** For temporary use: LocUtil.clone before passing deeply, call setWorld(null) after use. */
private final Location useLoc = new Location(null, 0, 0, 0);
/** The maximum distance allowed to interact with a block in creative mode. */
public static final double CREATIVE_DISTANCE = 5.6D;
@ -65,7 +63,9 @@ public class Reach extends Check {
// Execute whatever actions are associated with this check and the violation level and find out if we should
// cancel the event.
cancel = executeActions(player, data.reachVL, distance, BlockBreakConfig.getConfig(player).reachActions);
final ViolationData vd = new ViolationData(this, player, data.reachVL, distance, BlockBreakConfig.getConfig(player).reachActions);
vd.setParameter(ParameterName.REACH_DISTANCE, String.valueOf(Math.round(data.reachDistance)));
cancel = executeActions(vd);
} else{
// Player passed the check, reward them.
data.reachVL *= 0.9D;
@ -73,11 +73,5 @@ public class Reach extends Check {
return cancel;
}
@Override
protected Map<ParameterName, String> getParameterMap(final ViolationData violationData) {
final Map<ParameterName, String> parameters = super.getParameterMap(violationData);
parameters.put(ParameterName.REACH_DISTANCE, String.valueOf(Math.round(BlockBreakData.getData(violationData.player).reachDistance)));
return parameters;
}
}

View File

@ -1,7 +1,5 @@
package fr.neatmonster.nocheatplus.checks.blockplace;
import java.util.Map;
import org.bukkit.GameMode;
import org.bukkit.Location;
import org.bukkit.block.Block;
@ -23,9 +21,9 @@ public class Reach extends Check {
/** The maximum distance allowed to interact with a block in survival mode. */
public static final double SURVIVAL_DISTANCE = 5.2D;
/** For temporary use: LocUtil.clone before passing deeply, call setWorld(null) after use. */
private final Location useLoc = new Location(null, 0, 0, 0);
private final Location useLoc = new Location(null, 0, 0, 0);
/**
* Instantiates a new reach check.
@ -67,7 +65,9 @@ public class Reach extends Check {
// Execute whatever actions are associated with this check and the violation level and find out if we should
// cancel the event.
cancel = executeActions(player, data.reachVL, distance, cc.reachActions);
final ViolationData vd = new ViolationData(this, player, data.reachVL, distance, cc.reachActions);
vd.setParameter(ParameterName.REACH_DISTANCE, String.valueOf(data.reachDistance));
cancel = executeActions(vd);
} else{
// Player passed the check, reward them.
data.reachVL *= 0.9D;
@ -75,15 +75,8 @@ public class Reach extends Check {
// Cleanup.
useLoc.setWorld(null);
return cancel;
}
@Override
protected Map<ParameterName, String> getParameterMap(final ViolationData violationData) {
final Map<ParameterName, String> parameters = super.getParameterMap(violationData);
parameters.put(ParameterName.REACH_DISTANCE, "" +Math.round(BlockPlaceData.getData(violationData.player).reachDistance));
return parameters;
}
}

View File

@ -39,7 +39,7 @@ public class Captcha extends Check implements ICaptcha{
// Have they failed too man times?
if (data.captchTries > cc.captchaTries) {
// Find out if we need to kick the player or not.
executeActions(player, data.captchaVL, 1, cc.captchaActions, isMainThread);
executeActions(player, data.captchaVL, 1, cc.captchaActions);
// (Resetting captcha tries is done on quit/kick).
}

View File

@ -41,7 +41,7 @@ public class Color extends Check {
data.colorVL++;
// Find out if we need to remove the colors or not.
if (executeActions(player, data.colorVL, 1D, cc.colorActions, isMainThread))
if (executeActions(player, data.colorVL, 1D, cc.colorActions))
// Remove color codes.
return message.replaceAll("\302\247.", "").replaceAll("\247.", "");
}

View File

@ -42,7 +42,7 @@ public class Relog extends Check {
} else{
// Find out if we need to ban the player or not.
data.relogVL += 1D;
cancel = executeActions(player, (double) data.relogVL, 1D, cc.relogActions, true);
cancel = executeActions(player, (double) data.relogVL, 1D, cc.relogActions);
}
}
// TODO: decrease relog vl ?

View File

@ -268,12 +268,12 @@ public class Text extends Check implements INotifyReload {
}
else{
if (shortTermViolation) {
if (executeActions(player, data.textVL, added, cc.textFreqShortTermActions, isMainThread)) {
if (executeActions(player, data.textVL, added, cc.textFreqShortTermActions)) {
cancel = true;
}
}
else if (normalViolation) {
if (executeActions(player, data.textVL, added, cc.textFreqNormActions, isMainThread)) {
if (executeActions(player, data.textVL, added, cc.textFreqNormActions)) {
cancel = true;
}
}

View File

@ -1,7 +1,5 @@
package fr.neatmonster.nocheatplus.checks.fight;
import java.util.Map;
import org.bukkit.entity.Player;
import fr.neatmonster.nocheatplus.actions.ParameterName;
@ -35,41 +33,41 @@ public class Speed extends Check {
final FightData data = FightData.getData(player);
boolean cancel = false;
// Add to frequency.
data.speedBuckets.add(now, 1f);
// Medium term (normalized to one second), account for server side lag.
final long fullTime = cc.speedBucketDur * cc.speedBuckets;
final float fullLag = cc.lag ? TickTask.getLag(fullTime, true) : 1f;
final float total = data.speedBuckets.score(cc.speedBucketFactor) * 1000f / (fullLag * fullTime);
// Short term.
final int tick = TickTask.getTick();
if (tick < data.speedShortTermTick){
// Tick task got reset.
data.speedShortTermTick = tick;
data.speedShortTermCount = 1;
// Tick task got reset.
data.speedShortTermTick = tick;
data.speedShortTermCount = 1;
}
else if (tick - data.speedShortTermTick < cc.speedShortTermTicks){
// Account for server side lag.
if (!cc.lag || TickTask.getLag(50L * (tick - data.speedShortTermTick), true) < 1.5f){
// Within range, add.
data.speedShortTermCount ++;
}
else{
// Too much lag, reset.
data.speedShortTermTick = tick;
data.speedShortTermCount = 1;
}
// Account for server side lag.
if (!cc.lag || TickTask.getLag(50L * (tick - data.speedShortTermTick), true) < 1.5f){
// Within range, add.
data.speedShortTermCount ++;
}
else{
// Too much lag, reset.
data.speedShortTermTick = tick;
data.speedShortTermCount = 1;
}
}
else{
data.speedShortTermTick = tick;
data.speedShortTermCount = 1;
data.speedShortTermTick = tick;
data.speedShortTermCount = 1;
}
final float shortTerm = (float ) data.speedShortTermCount * 1000f / (50f * cc.speedShortTermTicks);
final float max = Math.max(shortTerm, total);
// Too many attacks?
@ -79,17 +77,13 @@ public class Speed extends Check {
// Execute whatever actions are associated with this check and the violation level and find out if we should
// cancel the event.
cancel = executeActions(player, data.speedVL, max - cc.speedLimit, cc.speedActions);
final ViolationData vd = new ViolationData(this, player, data.speedVL, max - cc.speedLimit, cc.speedActions);
vd.setParameter(ParameterName.LIMIT, String.valueOf(Math.round(cc.speedLimit)));
cancel = executeActions(vd);
}
else data.speedVL *= 0.96;
return cancel;
}
@Override
protected Map<ParameterName, String> getParameterMap(final ViolationData violationData) {
final Map<ParameterName, String> parameters = super.getParameterMap(violationData);
parameters.put(ParameterName.LIMIT, String.valueOf(Math.round(FightConfig.getConfig(violationData.player).speedLimit)));
return parameters;
}
}

View File

@ -1,7 +1,5 @@
package fr.neatmonster.nocheatplus.checks.inventory;
import java.util.Map;
import org.bukkit.entity.Player;
import fr.neatmonster.nocheatplus.actions.ParameterName;
@ -31,9 +29,9 @@ public class InstantEat extends Check {
* @return true, if successful
*/
public boolean check(final Player player, final int level) {
// Take time once.
final long time = System.currentTimeMillis();
// Take time once.
final long time = System.currentTimeMillis();
final InventoryData data = InventoryData.getData(player);
boolean cancel = false;
@ -60,19 +58,16 @@ public class InstantEat extends Check {
// Execute whatever actions are associated with this check and the violation level and find out if we should
// cancel the event.
cancel = executeActions(player, data.instantEatVL, difference,
InventoryConfig.getConfig(player).instantEatActions);
final ViolationData vd = new ViolationData(this, player, data.instantEatVL, difference, InventoryConfig.getConfig(player).instantEatActions);
if (data.instantEatFood != null) {
vd.setParameter(ParameterName.FOOD, data.instantEatFood.toString());
}
cancel = executeActions(vd);
}
data.instantEatInteract = 0;
data.instantEatFood = null;
return cancel;
}
@Override
protected Map<ParameterName, String> getParameterMap(final ViolationData violationData) {
final Map<ParameterName, String> parameters = super.getParameterMap(violationData);
parameters.put(ParameterName.FOOD, InventoryData.getData(violationData.player).instantEatFood.toString());
return parameters;
}
}

View File

@ -1170,7 +1170,7 @@ public class MovingListener extends CheckListener implements TickListener, IRemo
if (!pLoc.isOnGround(1.0, 0.3, 0.1) && !pLoc.isResetCond() && !pLoc.isAboveLadder() && !pLoc.isAboveStairs()) {
// Likely a new style no-fall bypass (damage in mid-air).
data.noFallVL += 1.0;
if (noFall.executeActions(player, data.noFallVL, 1.0, cc.noFallActions, true) && data.hasSetBack()) {
if (noFall.executeActions(player, data.noFallVL, 1.0, cc.noFallActions) && data.hasSetBack()) {
// Cancel the event and restore fall distance.
// NoFall data will not be reset
allowReset = false;