Fix riptide-equipped trident release not being detected; fix elytra + riptide; attempt to fix illegalsprint false positives.

* Order of operations for gliding with an elytra while riptiding was wrong: when gliding, riptide motion is added before. Only question that remains is: before stuck speed or after?

* For the tridentRelease flag, the moving trace was resetting the flag too early: as a fix, simply reset the flag manually instead of in the resetBase.

* Some notes/comments

Author: Lysandr0

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/MovingData.java

@@ -23,7 +23,6 @@
 import org.bukkit.entity.Entity;
 import org.bukkit.entity.EntityType;
 import org.bukkit.entity.Player;
-import org.bukkit.event.player.PlayerInputEvent;
 import org.bukkit.event.player.PlayerMoveEvent;
 
 import fr.neatmonster.nocheatplus.NCPAPIProvider;
@@ -176,7 +175,7 @@ public VehicleMoveData call() throws Exception {
     }, 2);
     /**
      * Track the inputs of the player (WASD, space bar, sprinting and jumping). <br> 
-     * The field is updated on {@link org.bukkit.event.player.PlayerMoveEvent} under split moves processor.<p>
+     * The field is updated on {@link org.bukkit.event.player.PlayerMoveEvent}s under split moves processor.<p>
      * This field is the one you should use to read input information during a PlayerMoveEvent instead of {@link Player#getCurrentInput()} alone, as it is kept synchronized with the correct movement, in case Bukkit happens to skip PlayerMoveEvents, 
      * causing a de-synchronization between inputs and movements (see comment in {@link MovingListener#onPlayerMove(PlayerMoveEvent)} and {@link PlayerMoveData#multiMoveCount}).<p>
      * This data is stored in MovingData instead of the Moving trace, as the latter may be invalidated, overridden or otherwise wiped out, while the input state is still valid and needed for the next move(s); it is not suitable for long-term storage.

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/model/PlayerMoveData.java

@@ -46,6 +46,7 @@ public class PlayerMoveData extends MoveData {
     /** Represents how far the player is submerged in water. Set with {@link fr.neatmonster.nocheatplus.utilities.map.BlockProperties#getVerticalFrictionFactor(LivingEntity, Location, double, PlayerMoveData)} */
     public double submergedWaterHeight;
 
+    /** Sets whether the player has just released a trident with riptide. Set on {@link org.bukkit.event.player.PlayerRiptideEvent}. */
     public boolean tridentRelease;
 
     /**
@@ -57,7 +58,8 @@ public class PlayerMoveData extends MoveData {
 
     /**
      * Indicates that this movement has/should have been slowed down due to the player hitting an entity (sprinting will be reset as well).<br>
-     * Mostly intended to be used for the h-speed prediction.
+     * Mostly intended to be used for the h-speed prediction.<br>
+     * Set in the {@link fr.neatmonster.nocheatplus.checks.fight.FightListener}
      */
     public boolean hasAttackSlowDown;
 
@@ -70,7 +72,7 @@ public class PlayerMoveData extends MoveData {
     public double xAllowedDistance;
 
     /**
-     *  The collision oon the X axis that has been set in this movement by SurvivalFly.
+     *  The collision on the X axis that has been set in this movement by SurvivalFly.
      *  Prior to version 1.21.2, this was set only if the theoretical speed prediction to set in this movement was found; or in other words, only if the player isn't cheating.
      *  On 1.21.2 and above, this is always set 
      *  @see PlayerMoveData#hasImpulse
@@ -206,7 +208,6 @@ protected void resetBase() {
         submergedLavaHeight = 0.0;
         submergedWaterHeight = 0.0;
         isGliding = false;
-        tridentRelease = false;
         forwardImpulse = PlayerKeyboardInput.ForwardDirection.NONE;
         strafeImpulse = PlayerKeyboardInput.StrafeDirection.NONE;
         // Properties involving the environment.

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/player/SurvivalFly.java

@@ -124,6 +124,7 @@ public Location check(final Player player, final PlayerLocation from, final Play
         final boolean debug = pData.isDebugActive(type);
         final PlayerMoveData thisMove = data.playerMoves.getCurrentMove();
         final PlayerMoveData lastMove = data.playerMoves.getFirstPastMove();
+        final CombinedData cData = pData.getGenericInstance(CombinedData.class);
         /* Regular and past fromOnGround */
         final boolean fromOnGround = from.isOnGround() || useBlockChangeTracker && from.isOnGroundOpportune(cc.yOnGround, 0L, blockChangeTracker, data.blockChangeRef, tick);
         /* Regular and past toOnGround */
@@ -328,11 +329,13 @@ else if (resetFrom) {
         data.lastLevitationLevel = !Double.isInfinite(Bridge1_9.getLevitationAmplifier(player)) ? Bridge1_9.getLevitationAmplifier(player) + 1 : 0.0;
         data.lastGravity = data.nextGravity;
         data.lastCollidingEntitiesLocations = CollisionUtil.getCollidingEntitiesLocations(player);
-        final CombinedData cData = pData.getGenericInstance(CombinedData.class);
         cData.wasSprinting = pData.isSprinting();
         cData.wasPressingShift = pData.isShiftKeyPressed();
         cData.wasSlowFalling = !Double.isInfinite(Bridge1_13.getSlowfallingAmplifier(player));
         cData.wasLevitating = !Double.isInfinite(Bridge1_9.getLevitationAmplifier(player));
+        if (thisMove.tridentRelease) {
+            thisMove.tridentRelease = false;
+        }
         // Log tags added after violation handling.
         if (debug && tags.size() > tagsLength) {
             logPostViolationTags(player);
@@ -474,6 +477,16 @@ private double[] processGliding(final PlayerLocation from, final PlayerLocation
         // Reset speed if judged to be negligible.
         checkNegligibleMomentum(pData, thisMove);
         checkNegligibleMomentumVertical(pData, thisMove);
+        // Yes, players can glide and riptide at the same time, increasing speed at a faster rate than chunks can load...
+        // Surely a questionable decision on Mojang's part.
+        // NOTE: For the elytra, this has to be done before applying gravity and other motion changes.
+        if (thisMove.tridentRelease) {
+            Vector riptideVelocity = to.getRiptideVelocity(false); // Cannot glide while on ground, so no need to check for it.
+            // Fortunately, we do not have to account for onGround push here, as gliding does not work on ground.
+            thisMove.xAllowedDistance += riptideVelocity.getX();
+            thisMove.yAllowedDistance += riptideVelocity.getY();
+            thisMove.zAllowedDistance += riptideVelocity.getZ();
+        }
         // TODO: Reduce verbosity (at least, make it easier to look at)
         Vector viewVector = TrigUtil.getLookingDirection(to, player);
         float radianPitch = to.getPitch() * TrigUtil.toRadians;
@@ -522,22 +535,12 @@ private double[] processGliding(final PlayerLocation from, final PlayerLocation
         thisMove.xAllowedDistance *= 0.99;
         thisMove.yAllowedDistance *= data.lastFrictionVertical;
         thisMove.zAllowedDistance *= 0.99;
-
         // Stuck-speed with the updated multiplier (both at the end)
         if (TrigUtil.lengthSquared(data.nextStuckInBlockHorizontal, data.nextStuckInBlockVertical, data.nextStuckInBlockHorizontal) > 1.0E-7) {
             thisMove.xAllowedDistance *= (double) data.nextStuckInBlockHorizontal;
             thisMove.yAllowedDistance *= (double) data.nextStuckInBlockVertical;
             thisMove.zAllowedDistance *= (double) data.nextStuckInBlockHorizontal;
         }
-        // Yes, players can glide and riptide at the same time, increasing speed at a faster rate than chunks can load...
-        // Surely a questionable decision on Mojang's part.
-        if (thisMove.tridentRelease) {
-            Vector riptideVelocity = to.getRiptideVelocity(false); // Cannot glide while on ground, so no need to check for it.
-            // Fortunately, we do not have to account for onGround push here, as gliding does not work on ground.
-            thisMove.xAllowedDistance += riptideVelocity.getX();
-            thisMove.yAllowedDistance += riptideVelocity.getY();
-            thisMove.zAllowedDistance += riptideVelocity.getZ();
-        }
         // Collisions last.
         Vector collisionVector = from.collide(new Vector(thisMove.xAllowedDistance, thisMove.yAllowedDistance, thisMove.zAllowedDistance), fromOnGround || thisMove.touchedGroundWorkaround, from.getBoundingBox());
         thisMove.collideX = collisionVector.getX() != thisMove.xAllowedDistance;
@@ -676,7 +679,9 @@ private double[] prepareSpeedEstimation(final PlayerLocation from, final PlayerL
             return new double[]{thisMove.hAllowedDistance, hDistanceAboveLimit};
         }
 
-        boolean onGround = !forceSetOffGround && (from.isOnGround() || lastMove.toIsValid && lastMove.yDistance <= 0.0 && lastMove.from.onGround || lastMove.yDistance < 0.0 && thisMove.fromLostGround || forceSetOnGround);
+        boolean onGround = from.isOnGround() || lastMove.toIsValid && lastMove.yDistance <= 0.0 && lastMove.from.onGround || lastMove.yDistance < 0.0 && thisMove.fromLostGround || forceSetOnGround;
+        // Override ground status if needed.
+        if (forceSetOffGround) onGround = false;
         /* All moves are assumed to be predictable, unless there are technical limitations / bugs / glitches that we cannot solve */
         boolean isPredictable;
         //////////////////////////////////////////////////////////////
@@ -737,7 +742,7 @@ else if (from.isInLava()) {
         else hDistanceAboveLimit = handleUnpredictableMove(thisMove, cc.survivalFlyStrictHorizontal);
         if (hDistanceAboveLimit > 0.0) {
              tags.add("hdistrel");
-            //if (debug) player.sendMessage("c/e: " + StringUtil.fdec6.format(thisMove.hDistance) + " / " + StringUtil.fdec6.format(thisMove.hAllowedDistance));
+            if (debug) player.sendMessage("c/e: " + StringUtil.fdec6.format(thisMove.hDistance) + " / " + StringUtil.fdec6.format(thisMove.hAllowedDistance));
         }
         return new double[]{thisMove.hAllowedDistance, hDistanceAboveLimit};
     }
@@ -856,7 +861,7 @@ private boolean estimateNextSpeed(final Player player, float movementSpeed, fina
         }
         // If impulses don't need to be inferred from the prediction, illegal sprinting checks can be performed here.
         if (BridgeMisc.isWASDImpulseKnown(player) && pData.isSprinting()
-            && (data.input.getForwardDir() != ForwardDirection.FORWARD 
+            && (data.input.getForwardDir() != ForwardDirection.FORWARD && data.input.getStrafeDir() != StrafeDirection.NONE && data.input.getForwardDir() != ForwardDirection.NONE
                 || player.getFoodLevel() <= 5) // must be checked here as well (besides on toggle sprinting) because players will immediately lose the ability to sprint if food level drops below 5
             ) { 
             // || inputs[i].getForward() < 0.8 // hasEnoughImpulseToStartSprinting, in LocalPlayer,java -> aiStep()
@@ -1039,9 +1044,6 @@ private boolean estimateNextSpeed(final Player player, float movementSpeed, fina
             thisMove.xAllowedDistance += (double) (-TrigUtil.sin(to.getYaw() * TrigUtil.toRadians) * Magic.BUNNYHOP_BOOST);
             thisMove.zAllowedDistance += (double) (TrigUtil.cos(to.getYaw() * TrigUtil.toRadians) * Magic.BUNNYHOP_BOOST);
             thisMove.bunnyHop = true;
-            if (!BridgeMisc.isWASDImpulseKnown(player) && PhysicsEnvelope.isVerticallyConstricted(from, to, pData)) {
-                isPredictable = false;
-            }
             tags.add("bunnyhop");
         }
 
@@ -1162,9 +1164,9 @@ else if (lastMove.from.inLava) {
             thisMove.hasImpulse = AlmostBoolean.match(input.getForwardDir() != ForwardDirection.NONE || input.getStrafeDir() != StrafeDirection.NONE);
             thisMove.strafeImpulse = input.getStrafeDir();
             thisMove.forwardImpulse = input.getForwardDir();
-            /*if (debug) {
+            if (debug) {
                 player.sendMessage("[SurvivalFly] (postPredict) Direction: " + input.getForwardDir() +" | "+ input.getStrafeDir());
-            }*/
+            }
             // If-else instead of an early return... Matter of preference. This makes code slightly easier to look at, as it avoids yet another indentation
             return isPredictable;
         }
@@ -1305,7 +1307,8 @@ True, if the offset between predicted and actual speed is smaller than the accur
             if (found) {
                 // These checks must be performed ex-post because they rely on data that is set after the prediction.
                 if (pData.isSprinting() 
-                    && (theorInputs[i].getForwardDir() != ForwardDirection.FORWARD || player.getFoodLevel() <= 5)) { 
+                    && (theorInputs[i].getForwardDir() != ForwardDirection.FORWARD && theorInputs[i].getStrafeDir() != StrafeDirection.NONE && theorInputs[i].getForwardDir() != ForwardDirection.NONE
+                        || player.getFoodLevel() <= 5)) { 
                     tags.add("illegalsprint");
                     Improbable.check(player, (float) thisMove.hDistance, System.currentTimeMillis(), "moving.survivalfly.illegalsprint", pData);
                     // Keep looping

NCPCore/src/main/java/fr/neatmonster/nocheatplus/utilities/location/PlayerLocation.java

@@ -66,7 +66,7 @@ public Player getPlayer() {
 
     /**
      * Straw-man method to account for this specific bug: <a href="https://bugs.mojang.com/browse/MC-2404">...</a>
-     * Should not be used outside its intended context (sneaking on edges), or if vanilla uses it.
+     * Should not be used outside its intended context (sneaking on edges), or if vanilla uses it elsewhere.
      */
     public boolean isAboveGround() {
         final IPlayerData pData = DataManager.getPlayerData(player);