fix: synchronize entity removal when needed (#3325)

Co-authored-by: Matt <[email protected]>
Co-authored-by: Alexander Brandes <[email protected]>

Author: 3 people

worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/BukkitEntity.java

@@ -19,6 +19,7 @@
 
 package com.sk89q.worldedit.bukkit;
 
+import com.fastasyncworldedit.core.util.TaskManager;
 import com.sk89q.worldedit.bukkit.adapter.BukkitImplAdapter;
 import com.sk89q.worldedit.entity.BaseEntity;
 import com.sk89q.worldedit.entity.Entity;
@@ -110,17 +111,21 @@ public BaseEntity getState() {
 
     @Override
     public boolean remove() {
-        org.bukkit.entity.Entity entity = entityRef.get();
-        if (entity != null) {
-            try {
-                entity.remove();
-            } catch (UnsupportedOperationException e) {
-                return false;
+        // synchronize the whole method, not just the remove operation as we always need to synchronize and
+        // can make sure the entity reference was not invalidated in the few milliseconds between the next available tick (lol)
+        return TaskManager.taskManager().sync(() -> {
+            org.bukkit.entity.Entity entity = entityRef.get();
+            if (entity != null) {
+                try {
+                    entity.remove();
+                } catch (UnsupportedOperationException e) {
+                    return false;
+                }
+                return entity.isDead();
+            } else {
+                return true;
             }
-            return entity.isDead();
-        } else {
-            return true;
-        }
+        });
     }
 
     @SuppressWarnings("unchecked")

worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/BukkitWorld.java

@@ -167,6 +167,14 @@ public List<com.sk89q.worldedit.entity.Entity> getEntities() {
         return list;
     }
 
+    @Override
+    public int removeEntities(final Region region) {
+        List<com.sk89q.worldedit.entity.Entity> entities = getEntities(region);
+        return TaskManager.taskManager().sync(() -> entities.stream()
+                .mapToInt(entity -> entity.remove() ? 1 : 0).sum()
+        );
+    }
+
     //FAWE: createEntity was moved to IChunkExtent to prevent issues with Async Entity Add.
 
     /**

worldedit-core/src/main/java/com/sk89q/worldedit/command/ClipboardCommands.java

@@ -50,7 +50,6 @@
 import com.sk89q.worldedit.command.util.annotation.Confirm;
 import com.sk89q.worldedit.command.util.annotation.Preload;
 import com.sk89q.worldedit.command.util.annotation.SynchronousSettingExpected;
-import com.sk89q.worldedit.entity.Entity;
 import com.sk89q.worldedit.extension.platform.Actor;
 import com.sk89q.worldedit.extent.clipboard.BlockArrayClipboard;
 import com.sk89q.worldedit.extent.clipboard.Clipboard;
@@ -479,7 +478,7 @@ public void place(
                     .apply(region.getMaximumPoint().subtract(region.getMinimumPoint()).toVector3())
                     .toBlockPoint());
             if (removeEntities) {
-                editSession.getEntities(new CuboidRegion(realTo, max)).forEach(Entity::remove);
+                editSession.removeEntities(new CuboidRegion(realTo, max));
             }
             if (selectPasted || onlySelect) {
                 RegionSelector selector = new CuboidRegionSelector(world, realTo, max);
@@ -569,9 +568,9 @@ public void paste(
             Vector3 realTo = to.toVector3().add(transform.apply(clipboardOffset.toVector3()));
             Vector3 max = realTo.add(transform.apply(region.getMaximumPoint().subtract(region.getMinimumPoint()).toVector3()));
 
-            // FAWE start - entity remova;l
+            // FAWE start - entity removal
             if (removeEntities) {
-                editSession.getEntities(new CuboidRegion(realTo.toBlockPoint(), max.toBlockPoint())).forEach(Entity::remove);
+                editSession.removeEntities(new CuboidRegion(realTo.toBlockPoint(), max.toBlockPoint()));
             }
             if (selectPasted || onlySelect) {
                 //FAWE end

worldedit-core/src/main/java/com/sk89q/worldedit/extent/Extent.java

@@ -176,6 +176,18 @@ default Entity createEntity(Location location, BaseEntity entity, UUID uuid) {
     default void removeEntity(int x, int y, int z, UUID uuid) {
     }
 
+    /**
+     * Removes all entities in the given region.
+     *
+     * @param region the region
+     * @return the number of entities removed
+     */
+    default int removeEntities(Region region) {
+        return this.getEntities(region).stream()
+                .mapToInt(entity -> entity.remove() ? 1 : 0)
+                .sum();
+    }
+
     /*
     Queue based methods
     TODO NOT IMPLEMENTED: