diff --git a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/actions/Action.java b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/actions/Action.java index 12ec4141..54c66248 100644 --- a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/actions/Action.java +++ b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/actions/Action.java @@ -45,36 +45,36 @@ public abstract class Action - * TODO: "Copy" does not match this. - * @param config - * @param threshold - * @return Can return this (unchanged), null (not to be executed ever) or a new instance (changed, optimized). - */ - public Action getOptimizedCopy(final ConfigFileWithActions config, final Integer threshold) { - return this; - } + /** + * Get an optimized copy, given the config in use. The default implementation returns this instance.
+ * TODO: "Copy" does not match this. + * @param config + * @param threshold + * @return Can return this (unchanged), null (not to be executed ever) or a new instance (changed, optimized). + */ + public Action getOptimizedCopy(final ConfigFileWithActions config, final Integer threshold) { + return this; + } } diff --git a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/Check.java b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/Check.java index 20bc7b06..36b63276 100644 --- a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/Check.java +++ b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/Check.java @@ -22,6 +22,7 @@ import fr.neatmonster.nocheatplus.utilities.TickTask; */ public abstract class Check implements MCAccessHolder{ + // TODO: Do these get cleaned up ? /** The execution histories of each check. */ protected static Map histories = new HashMap(); @@ -33,8 +34,9 @@ public abstract class Check implements MCAccessHolder{ * @return the history */ protected static ExecutionHistory getHistory(final Player player) { - if (!histories.containsKey(player.getName())) + if (!histories.containsKey(player.getName())) { histories.put(player.getName(), new ExecutionHistory()); + } return histories.get(player.getName()); } @@ -101,7 +103,7 @@ public abstract class Check implements MCAccessHolder{ * the violation data * @return true, if the event should be cancelled */ - protected boolean executeActions(final ViolationData violationData){ + protected boolean executeActions(final ViolationData violationData) { return executeActions(violationData, true); } @@ -116,17 +118,20 @@ public abstract class Check implements MCAccessHolder{ protected boolean executeActions(final ViolationData violationData, final boolean isMainThread) { // Dispatch the VL processing to the hook manager (now thread safe). - if (NCPHookManager.shouldCancelVLProcessing(violationData)) + if (NCPHookManager.shouldCancelVLProcessing(violationData)) { // One of the hooks has decided to cancel the VL processing, return false. return false; + } final boolean hasCancel = violationData.hasCancel(); - if (isMainThread) + if (isMainThread) { return violationData.executeActions(); - else + } + else { // Always schedule to add to ViolationHistory. TickTask.requestActionsExecution(violationData); + } // (Design change: Permission checks are moved to cached permissions, lazily updated.) return hasCancel; @@ -139,7 +144,7 @@ public abstract class Check implements MCAccessHolder{ * @param player * @return */ - protected Map getParameterMap(final ViolationData violationData){ + protected Map getParameterMap(final ViolationData violationData) { final Map params = new HashMap(); // (Standard parameters like player, vl, check name are filled in in ViolationData.getParameter!) return params; diff --git a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/ViolationData.java b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/ViolationData.java index d1c818cd..5f134d07 100644 --- a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/ViolationData.java +++ b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/ViolationData.java @@ -24,7 +24,7 @@ public class ViolationData implements IViolationInfo, ActionData{ /** The actions to be executed. */ public final ActionList actions; - + /** The actions applicable for the violation level. */ public final Action[] applicableActions; @@ -39,10 +39,10 @@ public class ViolationData implements IViolationInfo, ActionData{ /** The violation level. */ public final double vL; - + /** Filled in parameters. */ private Map parameters; - + private boolean needsParameters = false; /** @@ -68,11 +68,11 @@ public class ViolationData implements IViolationInfo, ActionData{ this.actions = actions; this.applicableActions = actions.getActions(vL); boolean needsParameters = false; - for (int i = 0; i < applicableActions.length; i++){ - if (applicableActions[i].needsParameters()){ - needsParameters = true; - break; - } + for (int i = 0; i < applicableActions.length; i++) { + if (applicableActions[i].needsParameters()) { + needsParameters = true; + break; + } } parameters = needsParameters ? check.getParameterMap(this) : null; this.needsParameters = needsParameters; @@ -86,84 +86,90 @@ public class ViolationData implements IViolationInfo, ActionData{ public Action [] getActions() { return applicableActions; } - - /** - * Execute actions and return if cancel. Does add it to history. - * @return - */ - public boolean executeActions(){ - try { - ViolationHistory.getHistory(player).log(check.getClass().getName(), addedVL); + /** + * Execute actions and return if cancel. Does add it to history. + * @return + */ + public boolean executeActions() { + try { + // Statistics. + ViolationHistory.getHistory(player).log(check.getClass().getName(), addedVL); + // TODO: the time is taken here, which makes sense for delay, but otherwise ? + final long time = System.currentTimeMillis() / 1000L; + boolean cancel = false; + for (final Action action : getActions()) { + if (Check.getHistory(player).executeAction(this, action, time)) { + // The execution history said it really is time to execute the action, find out what it is and do + // what is needed. + if (action.execute(this)) { + cancel = true; + } + } + } + return cancel; + } catch (final Exception e) { + LogUtil.logSevere(e); + // On exceptions cancel events. + return true; + } + } - // TODO: the time is taken here, which makes sense for delay, but otherwise ? - final long time = System.currentTimeMillis() / 1000L; - boolean cancel = false; - for (final Action action : getActions()) - if (Check.getHistory(player).executeAction(this, action, time)) - // The execution history said it really is time to execute the action, find out what it is and do - // what is needed. - if (action.execute(this)) cancel = true; + /** + * Check if the actions contain a cancel. + * @return + */ + @Override + public boolean hasCancel() { + for (final Action action : applicableActions) { + if (action instanceof CancelAction) return true; + } + return false; + } - return cancel; - } catch (final Exception e) { - LogUtil.logSevere(e); - // On exceptions cancel events. - return true; - } - } - - /** - * Check if the actions contain a cancel. - * @return - */ - @Override - public boolean hasCancel(){ - for (final Action action : applicableActions){ - if (action instanceof CancelAction) return true; - } - return false; - } - - @Override - public String getParameter(final ParameterName parameterName){ - if (parameterName == null) return ""; - switch (parameterName) { - case CHECK: - return check.getClass().getSimpleName(); - case PLAYER: - return player.getName(); - case VIOLATIONS: - return String.valueOf((long) Math.round(vL)); - case UUID: - return player.getUniqueId().toString(); - default: - break; - } - if (parameters == null) return ""; - final String value = parameters.get(parameterName); - return(value == null) ? ("") : value; - } - - @Override - public void setParameter(final ParameterName parameterName, String value){ - if (parameters == null) { - parameters = new HashMap(); - } - parameters.put(parameterName, value); - } + @Override + public String getParameter(final ParameterName parameterName) { + if (parameterName == null) { + return ""; + } + switch (parameterName) { + case CHECK: + return check.getClass().getSimpleName(); + case PLAYER: + return player.getName(); + case VIOLATIONS: + return String.valueOf((long) Math.round(vL)); + case UUID: + return player.getUniqueId().toString(); + default: + break; + } + if (parameters == null) { + return ""; + } + final String value = parameters.get(parameterName); + return(value == null) ? ("") : value; + } - @Override + @Override + public void setParameter(final ParameterName parameterName, String value) { + if (parameters == null) { + parameters = new HashMap(); + } + parameters.put(parameterName, value); + } + + @Override public boolean needsParameters() { return needsParameters; } - @Override - public boolean hasParameters() { - return parameters != null && !parameters.isEmpty(); - } + @Override + public boolean hasParameters() { + return parameters != null && !parameters.isEmpty(); + } - @Override + @Override public double getAddedVl() { return addedVL; } @@ -173,11 +179,11 @@ public class ViolationData implements IViolationInfo, ActionData{ return vL; } - public String getPermissionSilent() { - return actions.permissionSilent; - } - - public ActionList getActionList(){ - return actions; - } + public String getPermissionSilent() { + return actions.permissionSilent; + } + + public ActionList getActionList() { + return actions; + } }