Correctly fire VehicleExitEvent. Fixes BUKKIT-3761

This change makes it so that EntityHuman#setPassengerOf(Entity) invokes
its parent method when leaving vehicles so that VehicleExitEvent is fired
for players leaving vehicles.

This change also fixes BUKKIT-2110, making it so VehicleExitEvent
correctly handles cancellation. The implementation of VehicleExitEvent
completely ignored the cancellation state of the event, making it so that
cancelling the event had no effect.  Cancelling a VehicleExitEvent now
causes the entity to remain inside of the vehicle, with no visual stutter.

4 files changed, 52 insertions, 6 deletions

diff --git a/src/main/java/net/minecraft/server/Entity.java b/src/main/java/net/minecraft/server/Entity.java
index 671384d5..50edbdb5 100644
--- a/src/main/java/net/minecraft/server/Entity.java
+++ b/src/main/java/net/minecraft/server/Entity.java
@@ -1391,6 +1391,8 @@ public abstract class Entity {
         // b(null) doesn't really fly for overloaded methods,
         // so this method is needed
 
+        Entity originalVehicle = this.vehicle;
+        Entity originalPassenger = this.vehicle == null ? null : this.vehicle.passenger;
         PluginManager pluginManager = Bukkit.getPluginManager();
         this.getBukkitEntity(); // make sure bukkitEntity is initialised
         // CraftBukkit end
@@ -1402,6 +1404,10 @@ public abstract class Entity {
                 if ((this.bukkitEntity instanceof LivingEntity) && (this.vehicle.getBukkitEntity() instanceof Vehicle)) {
                     VehicleExitEvent event = new VehicleExitEvent((Vehicle) this.vehicle.getBukkitEntity(), (LivingEntity) this.bukkitEntity);
                     pluginManager.callEvent(event);
+
+                    if (event.isCancelled() || this.vehicle != originalVehicle) {
+                        return;
+                    }
                 }
                 // CraftBukkit end
 
@@ -1413,10 +1419,28 @@ public abstract class Entity {
         } else {
             // CraftBukkit start
             if ((this.bukkitEntity instanceof LivingEntity) && (entity.getBukkitEntity() instanceof Vehicle) && entity.world.isChunkLoaded((int) entity.locX >> 4, (int) entity.locZ >> 4)) {
+                // It's possible to move from one vehicle to another.  We need to check if they're already in a vehicle, and fire an exit event if they are.
+                VehicleExitEvent exitEvent = null;
+                if (this.vehicle != null) {
+                    exitEvent = new VehicleExitEvent((Vehicle) this.vehicle.getBukkitEntity(), (LivingEntity) this.bukkitEntity);
+                    pluginManager.callEvent(exitEvent);
+
+                    if (exitEvent.isCancelled() || this.vehicle != originalVehicle || (this.vehicle != null && this.vehicle.passenger != originalPassenger)) {
+                        return;
+                    }
+                }
+
                 VehicleEnterEvent event = new VehicleEnterEvent((Vehicle) entity.getBukkitEntity(), this.bukkitEntity);
                 pluginManager.callEvent(event);
 
-                if (event.isCancelled()) {
+                // If a plugin messes with the vehicle or the vehicle's passenger
+                if (event.isCancelled() || this.vehicle != originalVehicle || (this.vehicle != null && this.vehicle.passenger != originalPassenger)) {
+                    // If we only cancelled the enterevent then we need to put the player in a decent position.
+                    if (exitEvent != null && this.vehicle == originalVehicle && this.vehicle != null && this.vehicle.passenger == originalPassenger) {
+                        this.setPositionRotation(this.vehicle.locX, this.vehicle.boundingBox.b + (double) this.vehicle.length, this.vehicle.locZ, this.yaw, this.pitch);
+                        this.vehicle.passenger = null;
+                        this.vehicle = null;
+                    }
                     return;
                 }
             }
diff --git a/src/main/java/net/minecraft/server/EntityHuman.java b/src/main/java/net/minecraft/server/EntityHuman.java
index 963bc55c..e1a3ca11 100644
--- a/src/main/java/net/minecraft/server/EntityHuman.java
+++ b/src/main/java/net/minecraft/server/EntityHuman.java
@@ -304,6 +304,10 @@ public abstract class EntityHuman extends EntityLiving implements ICommandListen
     public void setPassengerOf(Entity entity) {
         // CraftBukkit end
         if (this.vehicle != null && entity == null) {
+            // CraftBukkit start - use parent method instead to correctly fire VehicleExitEvent
+            Entity originalVehicle = this.vehicle;
+            // First statement moved down, second statement handled in parent method.
+            /*
             if (!this.world.isStatic) {
                 this.l(this.vehicle);
             }
@@ -313,6 +317,12 @@ public abstract class EntityHuman extends EntityLiving implements ICommandListen
             }
 
             this.vehicle = null;
+            */
+            super.setPassengerOf(entity);
+            if (!this.world.isStatic && this.vehicle == null) {
+                this.l(originalVehicle);
+            }
+            // CraftBukkit end
         } else {
             super.setPassengerOf(entity); // CraftBukkit - call new parent
         }
diff --git a/src/main/java/net/minecraft/server/EntityPlayer.java b/src/main/java/net/minecraft/server/EntityPlayer.java
index 2191b075..955b75c3 100644
--- a/src/main/java/net/minecraft/server/EntityPlayer.java
+++ b/src/main/java/net/minecraft/server/EntityPlayer.java
@@ -461,12 +461,16 @@ public class EntityPlayer extends EntityHuman implements ICrafting {
     public void setPassengerOf(Entity entity) {
         // mount(null) doesn't really fly for overloaded methods,
         // so this method is needed
+        Entity currentVehicle = this.vehicle;
 
         super.setPassengerOf(entity);
-        // CraftBukkit end
 
-        this.playerConnection.sendPacket(new Packet39AttachEntity(0, this, this.vehicle));
-        this.playerConnection.a(this.locX, this.locY, this.locZ, this.yaw, this.pitch);
+        // Check if the vehicle actually changed.
+        if (currentVehicle != this.vehicle) {
+            this.playerConnection.sendPacket(new Packet39AttachEntity(0, this, this.vehicle));
+            this.playerConnection.a(this.locX, this.locY, this.locZ, this.yaw, this.pitch);
+        }
+        // CraftBukkit end
     }
 
     protected void a(double d0, boolean flag) {}
diff --git a/src/main/java/net/minecraft/server/PathfinderGoalTame.java b/src/main/java/net/minecraft/server/PathfinderGoalTame.java
index 8fde2e11..4b0d484c 100644
--- a/src/main/java/net/minecraft/server/PathfinderGoalTame.java
+++ b/src/main/java/net/minecraft/server/PathfinderGoalTame.java
@@ -55,8 +55,16 @@ public class PathfinderGoalTame extends PathfinderGoal {
                 this.entity.t(5);
             }
 
-            if (this.entity.passenger != null) this.entity.passenger.mount((Entity) null); // CraftBukkit - Check for null
-            this.entity.passenger = null;
+            // CraftBukkit start - Handle dismounting to account for VehicleExitEvent being fired.
+            if (this.entity.passenger != null) {
+                this.entity.passenger.mount((Entity) null);
+                // If the entity still has a passenger, then a plugin cancelled the event.
+                if (this.entity.passenger != null) {
+                    return;
+                }
+            }
+            // this.entity.passenger = null;
+            // CraftBukkit end
             this.entity.cD();
             this.entity.world.broadcastEntityEffect(this.entity, (byte) 6);
         }