EffVehicle + CondIsRiding Update

src/main/java/ch/njol/skript/conditions/CondIsRiding.java

@@ -1,5 +1,7 @@
 package ch.njol.skript.conditions;
 
+import ch.njol.skript.entity.EntityData;
+import ch.njol.skript.lang.util.SimpleExpression;
 import org.bukkit.entity.Entity;
 import org.bukkit.event.Event;
 import org.jetbrains.annotations.Nullable;
@@ -10,7 +12,6 @@
 import ch.njol.skript.doc.Examples;
 import ch.njol.skript.doc.Name;
 import ch.njol.skript.doc.Since;
-import ch.njol.skript.entity.EntityData;
 import ch.njol.skript.lang.Condition;
 import ch.njol.skript.lang.Expression;
 import ch.njol.skript.lang.SkriptParser.ParseResult;
@@ -20,41 +21,62 @@
  * @author Peter Güttinger
  */
 @Name("Is Riding")
-@Description("Tests whether an entity is riding another or is in a vehicle.")
-@Examples({"player is riding a saddled pig"})
-@Since("2.0")
+@Description("Tests whether an entity is riding any entity, a specific entity type, or a specific entity.")
+@Examples({
+	"if player is riding:",
+	"if player is riding any entity:",
+	"if player is riding a saddled pig:",
+	"if player is riding last spawned horse:"
+})
+@Since("2.0, INSERT VERSION (entities)")
 public class CondIsRiding extends Condition {
 
 	static {
-		PropertyCondition.register(CondIsRiding.class, "riding [%entitydatas%]", "entities");
+		PropertyCondition.register(CondIsRiding.class, "riding [any entity|%-entitydatas/entities%]", "entities");
 	}
-
-	@SuppressWarnings("null")
-	private Expression<Entity> entities;
-	@SuppressWarnings("null")
-	private Expression<EntityData<?>> types;
-
-	@SuppressWarnings({"unchecked", "null"})
+
+	private Expression<Entity> riders;
+	private @Nullable Expression<?> riding;
+
 	@Override
-	public boolean init(final Expression<?>[] exprs, final int matchedPattern, final Kleenean isDelayed, final ParseResult parseResult) {
-		entities = (Expression<Entity>) exprs[0];
-		types = (Expression<EntityData<?>>) exprs[1];
+	public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) {
+		//noinspection unchecked
+		riders = (Expression<Entity>) exprs[0];
+		riding = exprs[1];
 		setNegated(matchedPattern == 1);
 		return true;
 	}
-	
+
 	@Override
-	public boolean check(final Event e) {
-		return entities.check(e,
-				entity -> types.check(e,
-						data -> data.isInstance(entity.getVehicle())
-				), isNegated());
+	public boolean check(Event event) {
+		// Entities are riding in general
+		if (riding == null)
+			return riders.check(event, rider -> rider.getVehicle() != null, isNegated());
+		Object[] riding = this.riding.getArray(event);
+		// Entities are riding a specific type of entity or specific entity
+		return riders.check(event, rider -> {
+			Entity vehicle = rider.getVehicle();
+			// Entity is not riding anything
+			if (vehicle == null)
+				return false;
+			// An entity cannot be riding multiple entities/vehicles, will be treated as an 'or' list
+			return SimpleExpression.check(riding, object -> {
+				if (object instanceof EntityData<?> entityData) {
+					return entityData.isInstance(vehicle);
+				} else if (object instanceof Entity entity) {
+					return vehicle == entity;
+				}
+				return false;
+			}, false, false);
+		}, isNegated());
 	}
-	
+
 	@Override
-	public String toString(final @Nullable Event e, final boolean debug) {
-		return PropertyCondition.toString(this, PropertyType.BE, e, debug, entities,
-				"riding " + types.toString(e, debug));
+	public String toString(@Nullable Event event, boolean debug) {
+		String property = "riding";
+		if (riding != null)
+			property += " " + riding.toString(event, debug);
+		return PropertyCondition.toString(this, PropertyType.BE, event, debug, riders, property);
 	}
 
 }

src/main/java/ch/njol/skript/effects/EffVehicle.java

@@ -1,11 +1,6 @@
 package ch.njol.skript.effects;
 
-import org.bukkit.entity.Entity;
-import org.bukkit.event.Event;
-import org.jetbrains.annotations.Nullable;
-
 import ch.njol.skript.Skript;
-import ch.njol.skript.bukkitutil.PassengerUtils;
 import ch.njol.skript.doc.Description;
 import ch.njol.skript.doc.Examples;
 import ch.njol.skript.doc.Name;
@@ -15,93 +10,95 @@
 import ch.njol.skript.lang.Expression;
 import ch.njol.skript.lang.SkriptParser.ParseResult;
 import ch.njol.util.Kleenean;
+import org.bukkit.entity.Entity;
+import org.bukkit.event.Event;
+import org.jetbrains.annotations.Nullable;
 
 /**
  * @author Peter Güttinger
  */
 @Name("Vehicle")
-@Description({"Makes an entity ride another entity, e.g. a minecart, a saddled pig, an arrow, etc."})
-@Examples({"make the player ride a saddled pig",
-		"make the attacker ride the victim"})
+@Description("Makes an entity ride another entity, e.g. a minecart, a saddled pig, an arrow, etc.")
+@Examples({
+	"make the player ride a saddled pig",
+	"make the attacker ride the victim"
+})
 @Since("2.0")
 public class EffVehicle extends Effect {
+
 	static {
 		Skript.registerEffect(EffVehicle.class,
-				"(make|let|force) %entities% [to] (ride|mount) [(in|on)] %"+ (PassengerUtils.hasMultiplePassenger() ? "entities" : "entity") +"/entitydatas%",
+				"(make|let|force) %entities% [to] (ride|mount) [(in|on)] %entities/entitydatas%",
 				"(make|let|force) %entities% [to] (dismount|(dismount|leave) (from|of|) (any|the[ir]|his|her|) vehicle[s])",
 				"(eject|dismount) (any|the|) passenger[s] (of|from) %entities%");
 	}
-
-	@Nullable
-	private Expression<Entity> passengers;
-	@Nullable
-	private Expression<?> vehicles;
-
-	@SuppressWarnings({"unchecked", "null"})
+
+	private @Nullable Expression<Entity> passengers;
+	private @Nullable Expression<?> vehicles;
+
 	@Override
-	public boolean init(final Expression<?>[] exprs, final int matchedPattern, final Kleenean isDelayed, final ParseResult parseResult) {
+	public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) {
+		//noinspection unchecked
 		passengers = matchedPattern == 2 ? null : (Expression<Entity>) exprs[0];
 		vehicles = matchedPattern == 1 ? null : exprs[exprs.length - 1];
-		if (!PassengerUtils.hasMultiplePassenger() && passengers != null && vehicles != null && !passengers.isSingle() && vehicles.isSingle() && Entity.class.isAssignableFrom(vehicles.getReturnType()))
-			Skript.warning("An entity can only have one passenger");
 		return true;
 	}
 
 	@Override
-	protected void execute(final Event e) {
-		final Expression<?> vehicles = this.vehicles;
-		final Expression<Entity> passengers = this.passengers;
+	protected void execute(Event event) {
+		// matchedPattern = 1
 		if (vehicles == null) {
 			assert passengers != null;
-			for (final Entity p : passengers.getArray(e))
-				p.leaveVehicle();
+			for (Entity passenger : passengers.getArray(event))
+				passenger.leaveVehicle();
 			return;
 		}
+		// matchedPattern = 2
 		if (passengers == null) {
 			assert vehicles != null;
-			for (final Object v : vehicles.getArray(e))
-				((Entity) v).eject();
+			for (Object vehicle : vehicles.getArray(event))
+				((Entity) vehicle).eject();
 			return;
 		}
-		final Object[] vs = vehicles.getArray(e);
-		if (vs.length == 0)
+		// matchedPattern = 0
+		Object[] vehiclesArray = vehicles.getArray(event);
+		if (vehiclesArray.length == 0)
 			return;
-		final Entity[] ps = passengers.getArray(e);
-		if (ps.length == 0)
+		Entity[] passengersArray = passengers.getArray(event);
+		if (passengersArray.length == 0)
 			return;
-		for (final Object v : vs) {
-			if (v instanceof Entity) {
-				((Entity) v).eject();
-				for (Entity p : ps){
-					assert p != null;
-					p.leaveVehicle();
-					PassengerUtils.addPassenger((Entity)v, p); //For 1.9 and lower, it will only set the last one.
+		for (Object vehicle : vehiclesArray) {
+			if (vehicle instanceof Entity vehicleEntity) {
+				for (Entity passenger : passengersArray) {
+					assert passenger != null;
+					if (passenger == vehicleEntity)
+						continue;
+					passenger.leaveVehicle();
+					vehicleEntity.addPassenger(passenger);
 				}
 			} else {
-				for (final Entity p : ps) {
-					assert p != null : passengers;
-					final Entity en = ((EntityData<?>) v).spawn(p.getLocation());
-					if (en == null)
+				for (Entity passenger : passengersArray) {
+					assert passenger != null : passengers;
+					Entity entity = ((EntityData<?>) vehicle).spawn(passenger.getLocation());
+					if (entity == null)
 						return;
-					PassengerUtils.addPassenger(en, p);
+					entity.addPassenger(passenger);
 				}
 			}
 		}
 	}
 
 	@Override
-	public String toString(final @Nullable Event e, final boolean debug) {
-		final Expression<?> vehicles = this.vehicles;
-		final Expression<Entity> passengers = this.passengers;
+	public String toString(@Nullable Event event, boolean debug) {
 		if (vehicles == null) {
 			assert passengers != null;
-			return "make " + passengers.toString(e, debug) + " dismount";
+			return "make " + passengers.toString(event, debug) + " dismount";
 		}
 		if (passengers == null) {
 			assert vehicles != null;
-			return "eject passenger" + (vehicles.isSingle() ? "" : "s") + " of " + vehicles.toString(e, debug);
+			return "eject passenger" + (vehicles.isSingle() ? "" : "s") + " of " + vehicles.toString(event, debug);
 		}
-		return "make " + passengers.toString(e, debug) + " ride " + vehicles.toString(e, debug);
+		return "make " + passengers.toString(event, debug) + " ride " + vehicles.toString(event, debug);
 	}
 
 }

src/test/skript/tests/syntaxes/effects/EffVehicle.sk

@@ -0,0 +1,28 @@
+test "mounting vehicle":
+	spawn a villager at test-location:
+		set {_vehicle} to entity
+	spawn a cow, a pig and a sheep at test-location:
+		add entity to {_passengers::*}
+	make {_passengers::*} ride {_vehicle}
+	loop {_passengers::*}:
+		assert loop-value is riding with "Entity (%loop-value%) should pass riding in general"
+		assert loop-value is riding any entity with "Entity (%loop-value%) should pass riding in general with 'any entity' pattern"
+		assert loop-value is riding a villager with "Entity (%loop-value%) should pass riding a villager entity data"
+		assert loop-value is riding {_vehicle} with "Entity (%loop-value%) should pass riding specific entity"
+		assert loop-value is not riding a zombie villager with "Entity (%loop-value%) should pass riding the wrong entity data"
+
+	make {_passengers::*} dismount
+
+	loop {_passengers::*}:
+		assert loop-value is not riding with "Entity (%loop-value%) should pass not riding in general"
+		assert loop-value is not riding any entity with "Entity (%loop-value%) should pass not riding in general with 'any entity' pattern"
+		assert loop-value is not riding a villager with "Entity (%loop-value%) should pass not riding a villager entity data"
+		assert loop-value is not riding {_vehicle} with "Entity (%loop-value%) should pass not riding specific entity"
+
+	# Test exception on entity riding itself
+	make {_vehicle} ride {_vehicle}
+	assert {_vehicle} is not riding {_vehicle} with "Entity can not be riding itself"
+	assert passengers of {_vehicle} does not contain {_vehicle} with "Passengers of vehicle should not contain the vehicle itself"
+
+	clear entity within {_vehicle}
+	clear entities within {_passengers::*}