Fix ExprRemainingAir Negative Value Handling (#6947)

src/main/java/ch/njol/skript/expressions/ExprRemainingAir.java

@@ -1,108 +1,81 @@
-/**
- *   This file is part of Skript.
- *
- *  Skript is free software: you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation, either version 3 of the License, or
- *  (at your option) any later version.
- *
- *  Skript is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with Skript.  If not, see <http://www.gnu.org/licenses/>.
- *
- * Copyright Peter Güttinger, SkriptLang team and contributors
- */
 package ch.njol.skript.expressions;
 
-import org.bukkit.entity.LivingEntity;
-import org.bukkit.event.Event;
-import org.eclipse.jdt.annotation.Nullable;
-
-import ch.njol.skript.classes.Changer;
 import ch.njol.skript.classes.Changer.ChangeMode;
 import ch.njol.skript.doc.Description;
 import ch.njol.skript.doc.Examples;
 import ch.njol.skript.doc.Name;
 import ch.njol.skript.doc.Since;
 import ch.njol.skript.expressions.base.SimplePropertyExpression;
 import ch.njol.skript.util.Timespan;
+import ch.njol.skript.util.Timespan.TimePeriod;
 import ch.njol.util.coll.CollectionUtils;
+import org.bukkit.entity.LivingEntity;
+import org.bukkit.event.Event;
+import org.jetbrains.annotations.Nullable;
 
-/**
- * @author Peter Güttinger
- */
 @Name("Remaining Air")
 @Description("How much time a player has left underwater before starting to drown.")
-@Examples({"player's remaining air is less than 3 seconds:",
-		"	send \"hurry, get to the surface!\" to the player"})
-@Since("<i>unknown</i> (before 2.1)")
+@Examples({
+	"if the player's remaining air is less than 3 seconds:",
+		"\tsend \"hurry, get to the surface!\" to the player"
+})
+@Since("2.0")
 public class ExprRemainingAir extends SimplePropertyExpression<LivingEntity, Timespan> {
 
 	static {
 		register(ExprRemainingAir.class, Timespan.class, "remaining air", "livingentities");
 	}
-
-	@Override
-	public Class<Timespan> getReturnType() {
-		return Timespan.class;
-	}
-
-	@Override
-	protected String getPropertyName() {
-		return "remaining air";
-	}
-
-	@Override
-	public Timespan convert(final LivingEntity entity) {
-		return Timespan.fromTicks(entity.getRemainingAir());
-	}
-
-	@Nullable
+
 	@Override
-	public Class<?>[] acceptChange(Changer.ChangeMode mode) {
-		return (mode != ChangeMode.REMOVE_ALL) ? CollectionUtils.array(Timespan.class) : null;
+	public Timespan convert(LivingEntity entity) {
+		/*
+		 * negative values are allowed, and Minecraft itself may return a negative value from -1 to -20
+		 * these negative values seem to control when the entity actually takes damage
+		 * that is, when it hits -20, the entity takes damage, and it goes back to 0
+		 * for simplicity, we cap it at 0 seconds (as it is still the case that the entity has no air)
+		 */
+		return new Timespan(TimePeriod.TICK, Math.max(0, entity.getRemainingAir()));
 	}
-
-	@SuppressWarnings("null")
+
 	@Override
-	public void change(Event event, @Nullable Object[] delta, Changer.ChangeMode mode) {
+	public Class<?> @Nullable [] acceptChange(ChangeMode mode) {
 		switch (mode) {
 			case ADD:
-				long ticks = ((Timespan)delta[0]).getTicks();
-				for (LivingEntity entity : getExpr().getArray(event)) {
-					int newTicks = entity.getRemainingAir() + (int) ticks;
-
-					// Sanitize remaining air to avoid client hangs/crashes
-					if (newTicks > 20000) // 1000 seconds
-						newTicks = 20000;
-					entity.setRemainingAir(newTicks);
-				}
-				break;
-			case REMOVE:
-				ticks = ((Timespan)delta[0]).getTicks();
-				for (LivingEntity entity : getExpr().getArray(event))
-					entity.setRemainingAir(entity.getRemainingAir() - (int) ticks);
-				break;
 			case SET:
-				ticks = ((Timespan)delta[0]).getTicks();
-				// Sanitize remaining air to avoid client hangs/crashes
-				if (ticks > 20000) // 1000 seconds
-					ticks = 20000;
-
-				for (LivingEntity entity : getExpr().getArray(event))
-					entity.setRemainingAir((int) ticks);
-				break;
+			case REMOVE:
 			case DELETE:
-			case REMOVE_ALL:
 			case RESET:
-				for (LivingEntity entity : getExpr().getArray(event))
-					entity.setRemainingAir(20 * 15); // 15 seconds of air
-				break;
+				return CollectionUtils.array(Timespan.class);
+			default:
+				return null;
 		}
 	}
-
+
+	@Override
+	public void change(Event event, Object @Nullable [] delta, ChangeMode mode) {
+		// default is 15 seconds of air
+		long changeValue = delta != null ? ((Timespan) delta[0]).getAs(TimePeriod.TICK) : 20 * 15;
+		if (mode == ChangeMode.REMOVE) // subtract the change value
+			changeValue *= -1;
+		for (LivingEntity entity : getExpr().getArray(event)) {
+			long newRemainingAir = 0;
+			if (mode == ChangeMode.ADD || mode == ChangeMode.REMOVE)
+				newRemainingAir = entity.getRemainingAir();
+			// while entities have a "maximum air", the value is allowed to go past it
+			// while negative values are permitted, the behavior is strange
+			newRemainingAir = Math.max(Math.min(newRemainingAir + changeValue, Integer.MAX_VALUE), 0);
+			entity.setRemainingAir((int) newRemainingAir);
+		}
+	}
+
+	@Override
+	public Class<Timespan> getReturnType() {
+		return Timespan.class;
+	}
+
+	@Override
+	protected String getPropertyName() {
+		return "remaining air";
+	}
+
 }

src/test/skript/tests/syntaxes/expressions/ExprRemainingAir.sk

@@ -0,0 +1,18 @@
+test "remaining air":
+
+	spawn a pig at location of spawn of world "world":
+		set {_e} to event-entity
+
+	reset remaining air of {_e}
+	assert remaining air of {_e} is 15 seconds with "resetting did not set to 15 seconds"
+
+	set remaining air of {_e} to 3 years
+	assert remaining air of {_e} is 3 years with "setting to 3 years did not work"
+
+	remove 5 years from remaining air of {_e}
+	assert remaining air of {_e} is 0 seconds with "removing did not limit to 0 seconds"
+
+	add 10 years to remaining air of {_e}
+	assert ticks of remaining air of {_e} is 2147483647 with "adding did not limit to max int value"
+
+	delete the entity within {_e}