Skip to content

Commit

Permalink
Review vehicle position resetting.
Browse files Browse the repository at this point in the history 
Main objective was to not reset the set-backs on setting back a vehicle
with a player, plus data resetting cleanup.

Specific changes:
* Prefer an existing set-back, in case the default one has been
invalidated.
* Do not cancel a vehicle set back task, in case no set-back is present.
* Avoid multiple resetting and redundant calls for resetting positions.
* Ensure the location set back to is set as a set-back location after
set back.
* Skip changing vehicle data with player moving resetting (e.g. player
teleport, player morepackets disabled).

Review and possibly correct/alter use of:
* MovingData.vehicleMoves.invalidate
* MovingData.vehicleSetBacks
* MovingData.clearMostMovingData
* AuxMoving.resetXYPositions
* MovingData.clearVehicleData
* MovingData.clearVehicleMorePacketsData
* MovingData.clearAllMorePacketsData
* MovingData|AuxMoving.resetVehiclePositions
  • Loading branch information
@asofold
asofold committed May 22, 2016
1 parent dfb65b2 commit a575a9b
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 50 deletions.
12 changes: 10 additions & 2 deletions NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/MovingData.java
View file
Expand Up @@ -302,8 +302,16 @@ public VehicleMoveData call() throws Exception {
public int lastSetBackHash = 0;

// Vehicles.
/** Inconsistency-flag. Set on moving inside of vehicles, reset on exiting properly. Workaround for VehicleLeaveEvent missing. */
/**
* Inconsistency-flag. Set on moving inside of vehicles, reset on exiting
* properly. Workaround for VehicleLeaveEvent missing.
*/
public boolean wasInVehicle = false; // Workaround
/**
* Set to indicate that events happen during a vehicle set back. Allows
* skipping some resetting.
*/
public boolean isVehicleSetBack = false; // Workaround
public MoveConsistency vehicleConsistency = MoveConsistency.INCONSISTENT; // Workaround
public final DefaultSetBackStorage vehicleSetBacks = new DefaultSetBackStorage();
// Data of the more packets vehicle check.
Expand Down Expand Up @@ -402,7 +410,7 @@ public void onSetBack(final PlayerLocation setBack) {
resetPlayerPositions(setBack);
adjustMediumProperties(setBack);
setSetBack(setBack); // Problematic with multiple set-back locations stored (currently the safe-medium one is preferred, but later...)
vehicleSetBacks.resetAllLazily(setBack);
// vehicleSetBacks.resetAllLazily(setBack); // Not good: Overrides older set-back locations.
}

/**
Expand Down
54 changes: 36 additions & 18 deletions NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/MovingListener.java
View file
Expand Up @@ -727,7 +727,7 @@ else if (checkCf) {
}
else {
// Otherwise we need to clear their data.
data.clearAllMorePacketsData();
data.clearPlayerMorePacketsData();
}

// Reset jump amplifier if needed.
Expand Down Expand Up @@ -1315,7 +1315,7 @@ else if (to == null) {
return;
}
final MovingConfig cc = MovingConfig.getConfig(player);
// Detect our own set-backs.
// Detect our own player set-backs.
if (data.hasTeleported()) {
if (data.isTeleported(to)) {
// Set-back.
Expand All @@ -1341,26 +1341,24 @@ else if (to == null) {
}
}

boolean skipExtras = false; // Skip extra data adjustments during special teleport, e.g. vehicle set back.
// Detect our own vehicle set-backs (...).
if (data.isVehicleSetBack) {
// Uncertain if this is vehicle leave or vehicle enter.
if (event.getCause() != PlayerTeleportEvent.TeleportCause.UNKNOWN) {
// TODO: Unexpected, what now?
NCPAPIProvider.getNoCheatPlusAPI().getLogManager().warning(Streams.STATUS, CheckUtils.getLogMessagePrefix(player, CheckType.MOVING_VEHICLE) + "Unexpected teleport cause on vehicle set back: " + event.getCause());
}
// TODO: Consider to verify, if this is somewhere near the vehicle as expected (might need storing more data for a set back).
skipExtras = true;
}

// Normal teleport
double fallDistance = data.noFallFallDistance;
// final LiftOffEnvelope oldEnv = data.liftOffEnvelope; // Remember for workarounds.
data.clearMostMovingCheckData();
data.clearFlyData();
data.clearPlayerMorePacketsData();
data.setSetBack(to);
// TODO: How to account for plugins that reset the fall distance here?
if (fallDistance > 1.0 && fallDistance - player.getFallDistance() > 0.0) {
// Reset fall distance if set so in the config.
if (!cc.noFallTpReset) {
// (Set fall distance if set to not reset.)
player.setFallDistance((float) fallDistance);
}
else if (fallDistance >= 3.0) {
data.noFallSkipAirCheck = true;
}
}
if (event.getCause() == TeleportCause.ENDER_PEARL) {
// Prevent NoFall violations for ender-pearls.
data.noFallSkipAirCheck = true;
}
data.sfHoverTicks = -1; // Important against concurrent modification exception.
aux.resetPositionsAndMediumProperties(player, to, data, cc);
// TODO: Decide to remove the LiftOffEnvelope thing completely.
Expand All @@ -1374,6 +1372,26 @@ else if (fallDistance >= 3.0) {
Combined.resetYawRate(player, to.getYaw(), System.currentTimeMillis(), true); // TODO: Not sure.
data.resetTeleported();

if (!skipExtras) {
// Adjust fall distance, if set so.
// TODO: How to account for plugins that reset the fall distance here?
// TODO: Detect transition from valid flying that needs resetting the fall distance.
if (fallDistance > 1.0 && fallDistance - player.getFallDistance() > 0.0) {
// Reset fall distance if set so in the config.
if (!cc.noFallTpReset) {
// (Set fall distance if set to not reset.)
player.setFallDistance((float) fallDistance);
}
else if (fallDistance >= 3.0) {
data.noFallSkipAirCheck = true;
}
}
if (event.getCause() == TeleportCause.ENDER_PEARL) {
// Prevent NoFall violations for ender-pearls.
data.noFallSkipAirCheck = true;
}
}

// Log.
if (data.debug) {
debug(player, "TP " + event.getCause() + " (normal): " + to);
Expand Down
18 changes: 18 additions & 0 deletions ...c/main/java/fr/neatmonster/nocheatplus/checks/moving/location/setback/SetBackStorage.java
View file
Expand Up @@ -5,6 +5,7 @@

import fr.neatmonster.nocheatplus.components.location.IGetLocationWithLook;
import fr.neatmonster.nocheatplus.time.monotonic.Monotonic;
import fr.neatmonster.nocheatplus.utilities.TrigUtil;

/**
* Represent a set-back location storage, allowing for more or less efficient
Expand Down Expand Up @@ -104,6 +105,23 @@ public SetBackEntry getValidEntry(final int index, final boolean fallBack) {
return null;
}

/**
* Get the first entry that is valid and matches the location (position +
* look, ignore world name).
*
* @param location
* @return
*/
public SetBackEntry getFirstValidEntry(final Location location) {
for (int i = 0; i < entries.length; i++) {
final SetBackEntry entry = entries[i];
if (entry.isValid() && TrigUtil.isSamePosAndLook(entry, location)) {
return entry;
}
}
return null;
}

public void invalidateAll() {
for (int i = 0; i < entries.length; i++) {
entries[i].setValid(false);
Expand Down
68 changes: 52 additions & 16 deletions NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/vehicle/VehicleChecks.java
View file
Expand Up @@ -37,6 +37,7 @@
import fr.neatmonster.nocheatplus.checks.moving.velocity.SimpleEntry;
import fr.neatmonster.nocheatplus.compat.MCAccess;
import fr.neatmonster.nocheatplus.components.location.IEntityAccessLastPositionAndLook;
import fr.neatmonster.nocheatplus.components.location.IGetLocationWithLook;
import fr.neatmonster.nocheatplus.components.location.SimplePositionWithLook;
import fr.neatmonster.nocheatplus.logging.StaticLog;
import fr.neatmonster.nocheatplus.logging.Streams;
Expand Down Expand Up @@ -434,15 +435,7 @@ private void checkVehicleMove(final Entity vehicle, final EntityType vehicleType

// Ensure a common set-back for now.
if (!data.vehicleSetBacks.isDefaultEntryValid()) {
// TODO: Check if other set-back is appropriate or if to set on other events.
data.vehicleSetBacks.setDefaultEntry(thisMove.from);
if (data.debug) {
debug(player, "Ensure vehicle set-back: " + thisMove.from);
}
if (data.vehicleSetBackTaskId != -1) {
// TODO: Set back outdated or not?
Bukkit.getScheduler().cancelTask(data.vehicleSetBackTaskId);
}
ensureSetBack(player, thisMove, data);
}

// Moving envelope check(s).
Expand Down Expand Up @@ -476,7 +469,7 @@ private void checkVehicleMove(final Entity vehicle, final EntityType vehicleType
}
else {
// Otherwise we need to clear their data.
// TODO: Should only if disabled.
// TODO: Make mid-term set-back resetting independent of more packets.
data.clearVehicleMorePacketsData();
}
}
Expand All @@ -494,9 +487,37 @@ private void checkVehicleMove(final Entity vehicle, final EntityType vehicleType
useLoc1.setWorld(null);
}

/**
* Called if the default set-back entry isn't valid.
*
* @param player
* @param thisMove
* @param data
*/
private void ensureSetBack(final Player player, final VehicleMoveData thisMove, final MovingData data) {
final IGetLocationWithLook ensureLoc;
if (!data.vehicleSetBacks.isAnyEntryValid()) {
ensureLoc = thisMove.from;
}
else {
ensureLoc = data.vehicleSetBacks.getOldestValidEntry();
}
data.vehicleSetBacks.setDefaultEntry(ensureLoc);
if (data.debug) {
debug(player, "Ensure vehicle set-back: " + ensureLoc);
}
// if (data.vehicleSetBackTaskId != -1) {
// // TODO: This is likely the wrong thing to do!
// Bukkit.getScheduler().cancelTask(data.vehicleSetBackTaskId);
// data.vehicleSetBackTaskId = -1;
// if (data.debug) {
// debug(player, "Cancel set-back task on ensureSetBack.");
// }
// }
}

private void setBack(final Player player, final Entity vehicle, final SetBackEntry newTo, final MovingData data, final MovingConfig cc) {
// TODO: Generic set-back manager, preventing all sorts of stuff that might be attempted or just happen before the task is running?
data.vehicleMoves.invalidate();
if (data.vehicleSetBackTaskId == -1) {
// Schedule a delayed task to teleport back the vehicle with the player.
// (Only schedule if not already scheduled.)
Expand All @@ -511,6 +532,7 @@ private void setBack(final Player player, final Entity vehicle, final SetBackEnt
boolean scheduleSetBack = cc.scheduleVehicleSetBacks;
// Schedule as task, if set so.
if (scheduleSetBack) {
aux.resetVehiclePositions(vehicle, LocUtil.set(useLoc2, vehicle.getWorld(), newTo), data, cc); // Heavy-ish, though.
data.vehicleSetBackTaskId = Bukkit.getScheduler().scheduleSyncDelayedTask(plugin, new VehicleSetBackTask(vehicle, player, newTo.getLocation(vehicle.getWorld()), data.debug));

if (data.vehicleSetBackTaskId == -1) {
Expand All @@ -523,7 +545,13 @@ else if (data.debug) {
}
// Attempt to set back directly if set so, or if needed.
if (!scheduleSetBack) {
// NOTE: This causes nested vehicle exit+enter and player teleport events, while the current event is still being processed (one of player move, vehicle update/move).
/*
* NOTE: This causes nested vehicle exit+enter and player
* teleport events, while the current event is still being
* processed (one of player move, vehicle update/move). Position
* resetting and updating the set-back (if needed) is done there
* (hack, subject to current review).
*/
if (data.debug) {
debug(player, "Attempt to set the player back directly.");
}
Expand All @@ -532,6 +560,8 @@ else if (data.debug) {

}
else if (data.debug) {
// TODO: Reset positions.
data.vehicleMoves.invalidate();
debug(player, "Vehicle set back task already scheduled, skip this time.");
}
}
Expand Down Expand Up @@ -595,13 +625,19 @@ private void dataOnVehicleEnter(final Player player, final Entity vehicle, fina
data.joinOrRespawn = false;
data.removeAllVelocity();
// Event should have a vehicle, in case check this last.

final Location vLoc = vehicle.getLocation(useLoc1);
data.vehicleConsistency = MoveConsistency.getConsistency(vLoc, null, player.getLocation(useLoc2));
// TODO: Check the set-back for consistency, verify if it is the same?
data.vehicleSetBacks.resetAll(vLoc); // TODO: Detect set-back and keep those then.
if (data.isVehicleSetBack) {
/*
* Currently checking for consistency is done in
* TeleportUtil.teleport, so we skip that here: if
* (data.vehicleSetBacks.getFirstValidEntry(vLoc) == null) {
*/
}
else {
data.vehicleSetBacks.resetAll(vLoc);
}
aux.resetVehiclePositions(vehicle, vLoc, data, cc);
// TODO: Get VehicleMoveInfo + data.resetVehiclePositions with this position for now.
if (data.debug) {
debug(player, "Vehicle enter: " + vehicle.getType() + " , player: " + useLoc2 + " c=" + data.vehicleConsistency);
}
Expand Down
28 changes: 18 additions & 10 deletions NCPCore/src/main/java/fr/neatmonster/nocheatplus/utilities/TeleportUtil.java
View file
Expand Up @@ -10,6 +10,7 @@
import fr.neatmonster.nocheatplus.checks.moving.MovingConfig;
import fr.neatmonster.nocheatplus.checks.moving.MovingData;
import fr.neatmonster.nocheatplus.checks.moving.location.LocUtil;
import fr.neatmonster.nocheatplus.checks.moving.model.VehicleMoveData;
import fr.neatmonster.nocheatplus.checks.moving.util.AuxMoving;
import fr.neatmonster.nocheatplus.checks.workaround.WRPT;

Expand All @@ -30,12 +31,15 @@ public static void teleport(final Entity vehicle, final Player player, final Loc
// TODO: Rubber band issue needs synchronizing with packet level and ignore certain incoming ones?
// TODO: This handling could conflict with WorldGuard region flags.
// TODO: Account for nested passengers and inconsistencies.
final MovingData data = MovingData.getData(player);
data.isVehicleSetBack = true;
final Entity passenger = vehicle.getPassenger();
boolean vehicleTeleported = false;
final boolean playerIsPassenger = player.equals(passenger);
boolean playerTeleported = false;
// TODO: TeleportCause needs some central configuration (plugin vs. unknown vs. future).
if (vehicle.isDead() || !vehicle.isValid()) {
// TODO: Still consider teleporting the player.
vehicleTeleported = false;
}
else if (playerIsPassenger) { // && vehicle.equals(player.getVehicle).
Expand All @@ -52,7 +56,7 @@ else if (playerIsPassenger) { // && vehicle.equals(player.getVehicle).
// }
// }
if (!playerTeleported){
vehicle.eject(); // NOTE: VehicleExit fires.
vehicle.eject(); // NOTE: VehicleExit fires, unknown TP fires.
// TODO: Confirm eject worked, handle if not.
vehicleTeleported = vehicle.teleport(LocUtil.clone(location), TeleportCause.PLUGIN);
}
Expand All @@ -61,8 +65,7 @@ else if (passenger == null) {
vehicleTeleported = vehicle.teleport(location, TeleportCause.PLUGIN);
}
if (!playerTeleported && player.isOnline() && !player.isDead()) {
// Mask teleport as set-back.
final MovingData data = MovingData.getData(player);
// Mask player teleport as a set-back.
data.prepareSetBack(location);
playerTeleported = player.teleport(LocUtil.clone(location));
data.resetTeleported(); // Just in case.
Expand All @@ -71,21 +74,26 @@ else if (passenger == null) {
// TODO: Magic 1.0, plus is this valid with horse, dragon...
if (playerIsPassenger && playerTeleported && vehicleTeleported && player.getLocation().distance(vehicle.getLocation(useLoc)) < 1.5) {
// Somewhat check against tp showing something wrong (< 1.0).
vehicle.setPassenger(player); // NOTE: VehicleEnter fires.
vehicle.setPassenger(player); // NOTE: VehicleEnter fires, unknown TP fires.
// TODO: What on failure of setPassenger?
// Ensure a set-back.
// TODO: Player teleportation leads to resetting the vehicle set-back, which is bad, because there are multiple possible set-back locations stored, then possibly interfering.
if (!data.vehicleSetBacks.isAnyEntryValid()) {
// TODO: Set-backs get invalidated somewhere, likely on an extra unknown TP. Use data.isVehicleSetBack in MovingListener/teleport.
if (data.vehicleSetBacks.getFirstValidEntry(location) == null) {
// At least ensure one of the entries has to match the location we teleported the vehicle to.
if (data.debug) {
CheckUtils.debug(player, CheckType.MOVING_VEHICLE, "No valid vehicle set-back present after setting back with vehicle. Set new Location as default: " + location);
CheckUtils.debug(player, CheckType.MOVING_VEHICLE, "No set-back is matching the vehicle location that it has just been set back to. Reset all lazily to: " + location);
}
data.vehicleSetBacks.setDefaultEntry(location);
data.vehicleSetBacks.resetAllLazily(location);
}
// Set this location as past move.
final MovingConfig cc = MovingConfig.getConfig(player);
NCPAPIProvider.getNoCheatPlusAPI().getGenericInstance(AuxMoving.class).resetVehiclePositions(vehicle, location, data, cc);
final VehicleMoveData firstPastMove = data.vehicleMoves.getFirstPastMove();
if (!firstPastMove.valid || firstPastMove.toIsValid || !TrigUtil.isSamePos(firstPastMove.from, location)) {
final MovingConfig cc = MovingConfig.getConfig(player);
NCPAPIProvider.getNoCheatPlusAPI().getGenericInstance(AuxMoving.class).resetVehiclePositions(vehicle, location, data, cc);
}
}
}
data.isVehicleSetBack = false;
if (debug) {
CheckUtils.debug(player, CheckType.MOVING_VEHICLE, "Vehicle set back resolution: " + location + " pt=" + playerTeleported + " vt=" + vehicleTeleported);
}
Expand Down

0 comments on commit a575a9b

Please sign in to comment.