Better handle exceptions in ticking spells and try to prevent spells from being destroyed more than once

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/AbstractDelegatingSpell.java

@@ -20,8 +20,9 @@ import net.minecraft.util.hit.EntityHitResult;
 public abstract class AbstractDelegatingSpell implements Spell,
     ProjectileDelegate.ConfigurationListener, ProjectileDelegate.BlockHitListener, ProjectileDelegate.EntityHitListener {
 
-    private boolean isDirty;
+    private boolean dirty;
     private boolean hidden;
+    private boolean destroyed;
 
     private UUID uuid = UUID.randomUUID();
 
@@ -59,7 +60,7 @@ public abstract class AbstractDelegatingSpell implements Spell,
     }
 
     @Override
-    public UUID getUuid() {
+    public final UUID getUuid() {
         return uuid;
     }
 
@@ -75,12 +76,12 @@ public abstract class AbstractDelegatingSpell implements Spell,
 
     @Override
     public boolean isDirty() {
-        return isDirty || getDelegates().stream().anyMatch(Spell::isDirty);
+        return dirty || getDelegates().stream().anyMatch(Spell::isDirty);
     }
 
     @Override
     public void setDirty() {
-        isDirty = true;
+        dirty = true;
     }
 
     @Override
@@ -94,8 +95,17 @@ public abstract class AbstractDelegatingSpell implements Spell,
     }
 
     @Override
-    public void onDestroyed(Caster<?> caster) {
+    public final void destroy(Caster<?> caster) {
-        getDelegates().forEach(a -> a.onDestroyed(caster));
+        if (destroyed) {
+            return;
+        }
+        destroyed = true;
+        setDead();
+        onDestroyed(caster);
+    }
+
+    protected void onDestroyed(Caster<?> caster) {
+        getDelegates().forEach(a -> a.destroy(caster));
     }
 
     @Override
@@ -127,7 +137,7 @@ public abstract class AbstractDelegatingSpell implements Spell,
 
     @Override
     public void fromNBT(NbtCompound compound) {
-        isDirty = false;
+        dirty = false;
         hidden = compound.getBoolean("hidden");
         if (compound.contains("uuid")) {
             uuid = compound.getUuid("uuid");

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/AbstractDisguiseSpell.java

@@ -27,7 +27,7 @@ public abstract class AbstractDisguiseSpell extends AbstractSpell implements Dis
     }
 
     @Override
-    public void onDestroyed(Caster<?> caster) {
+    protected void onDestroyed(Caster<?> caster) {
         caster.asEntity().calculateDimensions();
         caster.asEntity().setInvisible(false);
         if (caster instanceof Pony) {
@@ -51,12 +51,6 @@ public abstract class AbstractDisguiseSpell extends AbstractSpell implements Dis
         return situation == Situation.BODY && update(source, true);
     }
 
-    @Override
-    public void setDead() {
-        super.setDead();
-        disguise.remove();
-    }
-
     @Override
     public void toNBT(NbtCompound compound) {
         super.toNBT(compound);

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/DispersableDisguiseSpell.java

@@ -24,6 +24,7 @@ public class DispersableDisguiseSpell extends AbstractDisguiseSpell implements I
 
     public DispersableDisguiseSpell(CustomisedSpellType<?> type) {
         super(type);
+        setHidden(true);
     }
 
     @Override
@@ -92,13 +93,4 @@ public class DispersableDisguiseSpell extends AbstractDisguiseSpell implements I
     public Optional<EntityAppearance> getAppearance() {
         return isSuppressed() ? Optional.empty() : super.getAppearance();
     }
-
-    @Override
-    public boolean isHidden() {
-        return true;
-    }
-
-    @Override
-    public void setHidden(boolean hidden) {
-    }
 }

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/PlaceableSpell.java

@@ -164,7 +164,7 @@ public class PlaceableSpell extends AbstractDelegatingSpell implements OrientedS
     }
 
     @Override
-    public void onDestroyed(Caster<?> source) {
+    protected void onDestroyed(Caster<?> source) {
         if (!source.isClient()) {
             castEntity.getTarget().ifPresent(target -> {
                 getWorld(source).map(Ether::get)

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/RainboomAbilitySpell.java

@@ -32,11 +32,11 @@ public class RainboomAbilitySpell extends AbstractSpell {
 
     public RainboomAbilitySpell(CustomisedSpellType<?> type) {
         super(type);
+        setHidden(true);
     }
 
     @Override
-    public void setDead() {
+    protected void onDestroyed(Caster<?> source) {
-        super.setDead();
         particlEffect.destroy();
     }
 
@@ -93,13 +93,4 @@ public class RainboomAbilitySpell extends AbstractSpell {
         super.fromNBT(compound);
         age = compound.getInt("age");
     }
-
-    @Override
-    public boolean isHidden() {
-        return false;
-    }
-
-    @Override
-    public void setHidden(boolean hidden) {
-    }
 }

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/Spell.java

@@ -101,7 +101,7 @@ public interface Spell extends NbtSerialisable, Affine {
     /**
      * Called when a gem is destroyed.
      */
-    void onDestroyed(Caster<?> caster);
+    void destroy(Caster<?> caster);
 
     /**
      * Converts this spell into a placeable spell.

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/effect/AbstractSpell.java

@@ -14,6 +14,7 @@ public abstract class AbstractSpell implements Spell {
     private boolean dead;
     private boolean dirty;
     private boolean hidden;
+    private boolean destroyed;
 
     private CustomisedSpellType<?> type;
 
@@ -43,33 +44,33 @@ public abstract class AbstractSpell implements Spell {
     }
 
     @Override
-    public void setDead() {
+    public final void setDead() {
         dead = true;
         setDirty();
     }
 
     @Override
-    public boolean isDead() {
+    public final boolean isDead() {
         return dead;
     }
 
     @Override
-    public boolean isDirty() {
+    public final boolean isDirty() {
         return dirty;
     }
 
     @Override
-    public void setDirty() {
+    public final void setDirty() {
         dirty = true;
     }
 
     @Override
-    public boolean isHidden() {
+    public final boolean isHidden() {
         return hidden;
     }
 
     @Override
-    public void setHidden(boolean hidden) {
+    public final void setHidden(boolean hidden) {
         this.hidden = hidden;
     }
 
@@ -78,8 +79,17 @@ public abstract class AbstractSpell implements Spell {
         return getType().getAffinity();
     }
 
+    protected void onDestroyed(Caster<?> caster) {
+    }
+
     @Override
-    public void onDestroyed(Caster<?> caster) {
+    public final void destroy(Caster<?> caster) {
+        if (destroyed) {
+            return;
+        }
+        destroyed = true;
+        setDead();
+        onDestroyed(caster);
     }
 
     @Override

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/effect/AttractiveSpell.java

@@ -138,7 +138,8 @@ public class AttractiveSpell extends ShieldSpell implements HomingSpell, TimedSp
     }
 
     @Override
-    public void onDestroyed(Caster<?> caster) {
+    protected void onDestroyed(Caster<?> caster) {
+        super.onDestroyed(caster);
         target.getOrEmpty(caster.asWorld()).ifPresent(target -> target.setGlowing(false));
     }
 

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/effect/BubbleSpell.java

@@ -138,7 +138,7 @@ public class BubbleSpell extends AbstractSpell implements TimedSpell,
     }
 
     @Override
-    public void onDestroyed(Caster<?> source) {
+    protected void onDestroyed(Caster<?> source) {
         particlEffect.destroy();
         if (source.asEntity() instanceof LivingEntity l) {
             MODIFIERS.forEach((attribute, modifier) -> {

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/effect/DisplacementSpell.java

@@ -104,7 +104,7 @@ public class DisplacementSpell extends AbstractSpell implements HomingSpell, Pla
     }
 
     @Override
-    public void onDestroyed(Caster<?> caster) {
+    protected void onDestroyed(Caster<?> caster) {
         caster.getOriginatingCaster().asEntity().setGlowing(false);
         target.ifPresent(caster.asWorld(), e -> e.setGlowing(false));
     }

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/effect/HydrophobicSpell.java

@@ -106,7 +106,7 @@ public class HydrophobicSpell extends AbstractSpell {
     }
 
     @Override
-    public void onDestroyed(Caster<?> caster) {
+    protected void onDestroyed(Caster<?> caster) {
         storedFluidPositions.removeIf(entry -> {
             entry.restore(caster.asWorld());
             return true;

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/effect/LightSpell.java

@@ -90,7 +90,7 @@ public class LightSpell extends AbstractSpell implements TimedSpell, ProjectileD
     }
 
     @Override
-    public void onDestroyed(Caster<?> caster) {
+    protected void onDestroyed(Caster<?> caster) {
         if (caster.isClient()) {
             return;
         }

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/effect/MindSwapSpell.java

@@ -44,7 +44,7 @@ public class MindSwapSpell extends MimicSpell implements ProjectileDelegate.Enti
     }
 
     @Override
-    public void onDestroyed(Caster<?> caster) {
+    protected void onDestroyed(Caster<?> caster) {
         super.onDestroyed(caster);
         if (initialized && !caster.isClient()) {
             counterpart.ifPresent(caster.asWorld(), e -> {

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/effect/NecromancySpell.java

@@ -151,7 +151,7 @@ public class NecromancySpell extends AbstractAreaEffectSpell implements Projecti
     }
 
     @Override
-    public void onDestroyed(Caster<?> caster) {
+    protected void onDestroyed(Caster<?> caster) {
         if (caster.isClient()) {
             return;
         }

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/effect/PortalSpell.java

@@ -186,7 +186,8 @@ public class PortalSpell extends AbstractSpell implements PlaceableSpell.Placeme
     }
 
     @Override
-    public void onDestroyed(Caster<?> caster) {
+    protected void onDestroyed(Caster<?> caster) {
+        particleEffect.destroy();
         Ether ether = Ether.get(caster.asWorld());
         ether.remove(getType(), caster.asEntity().getUuid());
         getTarget(caster).ifPresent(e -> e.setTaken(false));
@@ -213,10 +214,4 @@ public class PortalSpell extends AbstractSpell implements PlaceableSpell.Placeme
             (180 - yaw) * MathHelper.RADIANS_PER_DEGREE
         );
     }
-
-    @Override
-    public void setDead() {
-        super.setDead();
-        particleEffect.destroy();
-    }
 }

src/main/java/com/minelittlepony/unicopia/ability/magic/spell/effect/ShieldSpell.java

@@ -54,8 +54,7 @@ public class ShieldSpell extends AbstractSpell {
     }
 
     @Override
-    public void setDead() {
+    protected void onDestroyed(Caster<?> caster) {
-        super.setDead();
         particlEffect.destroy();
     }
 

src/main/java/com/minelittlepony/unicopia/entity/Living.java

@@ -14,7 +14,6 @@ import com.minelittlepony.unicopia.ability.Abilities;
 import com.minelittlepony.unicopia.ability.magic.Caster;
 import com.minelittlepony.unicopia.ability.magic.SpellContainer;
 import com.minelittlepony.unicopia.ability.magic.SpellPredicate;
-import com.minelittlepony.unicopia.ability.magic.SpellContainer.Operation;
 import com.minelittlepony.unicopia.ability.magic.spell.AbstractDisguiseSpell;
 import com.minelittlepony.unicopia.ability.magic.spell.Situation;
 import com.minelittlepony.unicopia.advancement.UCriteria;
@@ -291,12 +290,7 @@ public abstract class Living<T extends LivingEntity> implements Equine<T>, Caste
     @Override
     public void tick() {
         tickers.forEach(Tickable::tick);
-
+        effectDelegate.tick(Situation.BODY);
-        try {
-            getSpellSlot().forEach(spell -> Operation.ofBoolean(spell.tick(this, Situation.BODY)), entity.getWorld().isClient);
-        } catch (Exception e) {
-            Unicopia.LOGGER.error("Error whilst ticking spell on entity {}", entity, e);
-        }
 
         if (!(entity instanceof PlayerEntity)) {
             if (!entity.hasVehicle() && getCarrierId().isPresent() && !asWorld().isClient && entity.age % 10 == 0) {

src/main/java/com/minelittlepony/unicopia/entity/mob/CastSpellEntity.java

@@ -4,7 +4,6 @@ import com.minelittlepony.unicopia.*;
 import com.minelittlepony.unicopia.ability.magic.Caster;
 import com.minelittlepony.unicopia.ability.magic.Levelled;
 import com.minelittlepony.unicopia.ability.magic.SpellContainer;
-import com.minelittlepony.unicopia.ability.magic.SpellContainer.Operation;
 import com.minelittlepony.unicopia.ability.magic.spell.Situation;
 import com.minelittlepony.unicopia.ability.magic.spell.Spell;
 import com.minelittlepony.unicopia.entity.EntityPhysics;
@@ -67,7 +66,7 @@ public class CastSpellEntity extends LightEmittingEntity implements Caster<CastS
             return;
         }
 
-        if (!getSpellSlot().forEach(spell -> Operation.ofBoolean(spell.tick(this, Situation.GROUND_ENTITY)), getWorld().isClient)) {
+        if (!effectDelegate.tick(Situation.GROUND_ENTITY)) {
             discard();
         }
     }

src/main/java/com/minelittlepony/unicopia/network/datasync/EffectSync.java

@@ -8,9 +8,11 @@ import java.util.stream.Stream;
 
 import org.jetbrains.annotations.Nullable;
 
+import com.minelittlepony.unicopia.Unicopia;
 import com.minelittlepony.unicopia.ability.magic.Caster;
 import com.minelittlepony.unicopia.ability.magic.SpellContainer;
 import com.minelittlepony.unicopia.ability.magic.SpellPredicate;
+import com.minelittlepony.unicopia.ability.magic.spell.Situation;
 import com.minelittlepony.unicopia.ability.magic.spell.Spell;
 import com.minelittlepony.unicopia.util.NbtSerialisable;
 
@@ -41,6 +43,26 @@ public class EffectSync implements SpellContainer, NbtSerialisable {
         this.param = param;
     }
 
+    public boolean tick(Situation situation) {
+        return tick(spell -> Operation.ofBoolean(spell.tick(owner, situation)));
+    }
+
+    public boolean tick(Function<Spell, Operation> tickAction) {
+        try {
+            return forEach(spell -> {
+                try {
+                    return tickAction.apply(spell);
+                } catch (Throwable t) {
+                    Unicopia.LOGGER.error("Error whilst ticking spell on entity {}", owner, t);
+                }
+                return Operation.REMOVE;
+            }, owner.isClient());
+        } catch (Exception e) {
+            Unicopia.LOGGER.error("Error whilst ticking spell on entity {}", owner.asEntity(), e);
+        }
+        return false;
+    }
+
     @Override
     public boolean contains(UUID id) {
         return spells.containsReference(id) || spells.getReferences().anyMatch(s -> s.equalsOrContains(id));
@@ -60,8 +82,8 @@ public class EffectSync implements SpellContainer, NbtSerialisable {
     public void put(@Nullable Spell effect) {
         spells.addReference(effect);
         write();
-        if (owner instanceof UpdateCallback) {
+        if (owner instanceof UpdateCallback callback) {
-            ((UpdateCallback)owner).onSpellSet(effect);
+            callback.onSpellSet(effect);
         }
     }
 

src/main/java/com/minelittlepony/unicopia/network/datasync/SpellNetworkedReference.java

@@ -60,8 +60,7 @@ public class SpellNetworkedReference<T extends Spell> implements NetworkedRefere
             currentValue = Optional.ofNullable(newValue);
 
             if (oldValue != null && (newValue == null || !oldValue.getUuid().equals(newValue.getUuid()))) {
-                oldValue.setDead();
+                oldValue.destroy(owner);
-                oldValue.onDestroyed(owner);
             }
         }
     }

src/main/java/com/minelittlepony/unicopia/projectile/MagicProjectileEntity.java

@@ -8,6 +8,7 @@ import org.jetbrains.annotations.Nullable;
 
 import com.minelittlepony.unicopia.Affinity;
 import com.minelittlepony.unicopia.EquinePredicates;
+import com.minelittlepony.unicopia.Unicopia;
 import com.minelittlepony.unicopia.WeaklyOwned;
 import com.minelittlepony.unicopia.ability.magic.Affine;
 import com.minelittlepony.unicopia.ability.magic.Caster;
@@ -198,7 +199,7 @@ public class MagicProjectileEntity extends ThrownItemEntity implements Caster<Ma
             return;
         }
 
-        getSpellSlot().get(true).filter(spell -> spell.tick(this, Situation.PROJECTILE));
+        effectDelegate.tick(Situation.PROJECTILE);
 
         if (getHydrophobic()) {
             if (StatePredicate.isFluid(getWorld().getBlockState(getBlockPos()))) {
@@ -317,11 +318,15 @@ public class MagicProjectileEntity extends ThrownItemEntity implements Caster<Ma
     }
 
     protected <T extends ProjectileDelegate> void forEachDelegates(Consumer<T> consumer, Function<Object, T> predicate) {
-        getSpellSlot().forEach(spell -> {
+        effectDelegate.tick(spell -> {
             Optional.ofNullable(predicate.apply(spell)).ifPresent(consumer);
             return Operation.SKIP;
-        }, getWorld().isClient);
+        });
-        Optional.ofNullable(predicate.apply(getItem().getItem())).ifPresent(consumer);
+        try {
+            Optional.ofNullable(predicate.apply(getItem().getItem())).ifPresent(consumer);
+        } catch (Throwable t) {
+            Unicopia.LOGGER.error("Error whilst ticking spell on entity {}", owner, t);
+        }
     }
 
     @Override