Merge pull request #328 from vincent4vx/fix-327-fighter-not-on-cell-o…

…n-cast

fix(fight): Fighter#cell() called when fighter is not on cell #327

src/main/java/fr/quatrevieux/araknemu/game/fight/castable/effect/handler/invocations/KillAndReplaceByInvocationHandler.java

@@ -63,11 +63,15 @@ public void handle(FightCastScope cast, BaseCastScope<Fighter, FightCell>.Effect
         final Fighter caster = cast.caster();
 
         for (Fighter target : effect.targets()) {
+            if (!fight.active()) {
+                break;
+            }
+
             final FightCell cell = target.cell();
 
             target.life().kill(caster);
 
-            if (validator.check(caster)) {
+            if (fight.active() && validator.check(caster)) {
                 addInvocation(caster, effect.effect(), cell);
             }
         }

src/main/java/fr/quatrevieux/araknemu/game/fight/castable/effect/handler/misc/ApplySpellOnStartTurnHandler.java

@@ -25,6 +25,7 @@
 import fr.quatrevieux.araknemu.game.fight.castable.effect.buff.BuffHook;
 import fr.quatrevieux.araknemu.game.fight.castable.effect.handler.EffectHandler;
 import fr.quatrevieux.araknemu.game.fight.fighter.Fighter;
+import fr.quatrevieux.araknemu.game.fight.map.FightCell;
 import fr.quatrevieux.araknemu.game.spell.Spell;
 import fr.quatrevieux.araknemu.game.spell.SpellService;
 import fr.quatrevieux.araknemu.network.game.out.fight.action.ActionEffect;
@@ -70,13 +71,14 @@ public void buff(FightCastScope cast, FightCastScope.EffectScope effect) {
     @Override
     public boolean onStartTurn(Buff buff) {
         final Fighter target = buff.target();
+        final FightCell cell = target.cell();
         final Spell spell = spellService.get(buff.effect().min())
             .level(Asserter.assertPositive(buff.effect().max()))
         ;
 
-        final FightCastScope castScope = FightCastScope.probable(spell, target, target.cell(), spell.effects());
+        final FightCastScope castScope = FightCastScope.probable(spell, target, cell, spell.effects());
 
-        fight.send(ActionEffect.launchVisualEffect(target, target.cell(), spell));
+        fight.send(ActionEffect.launchVisualEffect(target, cell, spell));
         fight.effects().apply(castScope);
 
         return !target.dead();

src/main/java/fr/quatrevieux/araknemu/game/fight/castable/effect/handler/shifting/AvoidDamageByMovingBackHandler.java

@@ -60,7 +60,7 @@ public void buff(FightCastScope cast, FightCastScope.EffectScope effect) {
 
     @Override
     public boolean onCastTarget(Buff buff, FightCastScope cast) {
-        if (!isDamageCast(cast) || buff.target().cell().coordinate().distance(cast.caster().cell()) != 1) {
+        if (!isDamageCast(cast) || buff.target().cell().coordinate().distance(cast.from()) != 1) {
             return true;
         }
 

src/main/java/fr/quatrevieux/araknemu/game/fight/castable/effect/handler/shifting/MoveToTargetCellHandler.java

@@ -49,7 +49,7 @@ public MoveToTargetCellHandler(Fight fight) {
     @Override
     public void handle(FightCastScope cast, FightCastScope.EffectScope effect) {
         final Fighter caster = cast.caster();
-        final FightCell casterCell = caster.cell();
+        final FightCell casterCell = cast.from();
         final CoordinateCell<BattlefieldCell> casterCellCoordinate = casterCell.coordinate();
 
         // Remove 1 because the distance should be computed from the target fighter cell

src/main/java/fr/quatrevieux/araknemu/game/fight/castable/effect/handler/shifting/RaulebaqueHandler.java

@@ -52,7 +52,7 @@ public void handle(FightCastScope cast, FightCastScope.EffectScope effect) {
                 return;
             }
 
-            fighter.move(null);
+            lastCell.removeFighter(fighter);
 
             final Fighter other = startCell.fighter();
 

src/main/java/fr/quatrevieux/araknemu/game/fight/castable/effect/handler/shifting/SwitchPositionApplier.java

@@ -37,16 +37,22 @@ public SwitchPositionApplier(Fight fight) {
     /**
      * Apply the switch effect
      *
+     * If one of the fighter is dead, nothing is done (because the fighter is not on a cell anymore)
+     *
      * @param caster The switch spell caster
      * @param target The other fighter to switch with
      */
     public void apply(Fighter caster, Fighter target) {
+        if (caster.dead() || target.dead()) {
+            return;
+        }
+
         final FightCell casterCell = caster.cell();
         final FightCell targetCell = target.cell();
 
         // Unset cells
-        caster.move(null);
-        target.move(null);
+        casterCell.removeFighter(caster);
+        targetCell.removeFighter(target);
 
         // Switch cells
         caster.move(targetCell);

src/main/java/fr/quatrevieux/araknemu/game/fight/fighter/AbstractFighter.java

@@ -48,7 +48,7 @@ public abstract class AbstractFighter implements Fighter {
     private final Map<Object, Object> attachments = new HashMap<>();
 
     // Mutable attributes
-    private @Nullable FightCell cell;
+    private @MonotonicNonNull FightCell cell;
     private @MonotonicNonNull Fight fight;
     private Direction orientation = Direction.SOUTH_EAST;
     private boolean hidden = false;
@@ -83,7 +83,7 @@ public final void setOrientation(Direction orientation) {
     }
 
     @Override
-    public void setCell(@Nullable FightCell cell) {
+    public void setCell(FightCell cell) {
         final FightCell lastCell = this.cell;
 
         if (lastCell != null) {
@@ -94,20 +94,18 @@ public void setCell(@Nullable FightCell cell) {
     }
 
     @Override
-    public final void move(@Nullable FightCell cell) {
+    public final void move(FightCell cell) {
         final FightCell lastCell = this.cell;
+        final Fight fight = this.fight;
 
         if (lastCell != null) {
             lastCell.removeFighter(this);
         }
 
-        if (cell != null) {
-            cell.set(this);
-        }
-
+        cell.set(this);
         this.cell = cell;
 
-        if (cell != null && fight != null) {
+        if (fight != null) {
             fight.dispatch(new FighterMoved(this, cell));
         }
     }

src/main/java/fr/quatrevieux/araknemu/game/fight/fighter/Fighter.java

@@ -24,8 +24,8 @@
 import fr.quatrevieux.araknemu.game.fight.Fight;
 import fr.quatrevieux.araknemu.game.fight.FighterSprite;
 import fr.quatrevieux.araknemu.game.fight.castable.Castable;
-import fr.quatrevieux.araknemu.game.fight.castable.effect.buff.BuffList;
 import fr.quatrevieux.araknemu.game.fight.castable.closeCombat.CastableWeapon;
+import fr.quatrevieux.araknemu.game.fight.castable.effect.buff.BuffList;
 import fr.quatrevieux.araknemu.game.fight.fighter.operation.FighterOperation;
 import fr.quatrevieux.araknemu.game.fight.map.FightCell;
 import fr.quatrevieux.araknemu.game.fight.team.FightTeam;
@@ -66,14 +66,14 @@ public default boolean dead() {
      *
      * If the fighter is already on a cell, it will be removed from it
      */
-    public void setCell(@Nullable FightCell cell);
+    public void setCell(FightCell cell);
 
     /**
      * Go to the given cell
      *
      * @see fr.quatrevieux.araknemu.game.fight.fighter.event.FighterMoved Trigger when the fighter is actually moved
      */
-    public void move(@Nullable FightCell cell);
+    public void move(FightCell cell);
 
     /**
      * Change the hidden state of the fighter

src/main/java/fr/quatrevieux/araknemu/game/fight/turn/action/ActionHandler.java

@@ -85,6 +85,7 @@ public boolean start(FightAction action) {
      */
     public void terminate() {
         final PendingAction action = pending;
+        pending = null;
 
         if (action == null) {
             return;
@@ -98,9 +99,13 @@ public void terminate() {
         }
 
         try {
-            action.result.apply(turn);
+            // Do not apply the result if the caster is dead
+            // This can happen if the caster leave the fight before the action is terminated
+            // See: https://github.com/Arakne/Araknemu/issues/327
+            if (!action.action.performer().dead()) {
+                action.result.apply(turn);
+            }
         } finally {
-            pending = null;
             fight.dispatch(new FightActionTerminated(action.action));
 
             termination.forEach(Runnable::run);

src/main/java/fr/quatrevieux/araknemu/game/listener/fight/fighter/RemoveDeadFighter.java

@@ -21,6 +21,7 @@
 
 import fr.quatrevieux.araknemu.core.event.Listener;
 import fr.quatrevieux.araknemu.game.fight.Fight;
+import fr.quatrevieux.araknemu.game.fight.fighter.Fighter;
 import fr.quatrevieux.araknemu.game.fight.fighter.event.FighterDie;
 import fr.quatrevieux.araknemu.game.fight.turn.FightTurn;
 
@@ -36,11 +37,13 @@ public RemoveDeadFighter(Fight fight) {
 
     @Override
     public void on(FighterDie event) {
-        event.fighter().move(null);
+        final Fighter fighter = event.fighter();
+
+        fighter.cell().removeFighter(fighter);
 
         // Stop turn if it's the playing fighter
         fight.turnList().current()
-            .filter(turn -> turn.fighter().equals(event.fighter()))
+            .filter(turn -> turn.fighter().equals(fighter))
             .ifPresent(FightTurn::stop)
         ;
     }

src/test/java/fr/quatrevieux/araknemu/game/GameDataSet.java

@@ -566,6 +566,7 @@ public GameDataSet pushFunctionalSpells() throws SQLException, ContainerExceptio
                 "(344, 'Elemental Spear', 3001, '41,1,1', '6,3,,,0,0;94,1,,,0,0,0d0+1||2|1|4|0|0|true|true|false|false|0|1|0|0|PaPa||18;19;3;1;41|0|false', '6,3,,,0,0;91,1,,,0,0,0d0+1||2|1|4|0|0|true|true|false|false|0|1|0|0|PaPa||18;19;3;1;41|0|false', '6,3,,,0,0;92,1,,,0,0,0d0+1||2|1|4|0|0|true|true|false|false|0|1|0|0|PaPa||18;19;3;1;41|0|false', '6,3,,,0,0;93,1,,,0,0,0d0+1||2|1|4|0|0|true|true|false|false|0|1|0|0|PaPa||18;19;3;1;41|0|false', '6,3,,,0,0;95,1,,,0,0,0d0+1||2|1|4|0|0|true|true|false|false|0|1|0|0|PaPa||18;19;3;1;41|0|false', '', '')",
                 "(391, 'Maîtrise de l Epée', 4800, '10,1,1', '165,6,10,,4,0|165,6,15,,4,0|6|0|0|50|100|false|false|false|false|3|0|0|6|PaPa||8;18;19;3;1;41|0|false', '165,6,15,,4,0|165,6,20,,4,0|6|0|0|50|100|false|false|false|false|3|0|0|6|PaPa||8;18;19;3;1;41|0|false', '165,6,20,,4,0|165,6,25,,4,0|5|0|0|50|100|false|false|false|false|3|0|0|6|PaPa||8;18;19;3;1;41|0|false', '165,6,25,,4,0|165,6,30,,4,0|4|0|0|50|100|false|false|false|false|3|0|0|6|PaPa||8;18;19;3;1;41|0|false', '165,6,30,,4,0|165,6,35,,4,0|3|0|0|50|100|false|false|false|false|3|0|0|6|PaPa||8;18;19;3;1;41|0|false', '165,6,35,,4,0|165,6,40,,4,0|2|0|0|50|100|false|false|false|false|3|0|0|6|PaPa||8;18;19;3;1;41|100|false', '')",
                 "(447, 'Furie', 1054, '11,1,1', '96,6,10,,0,0,1d5+5;112,4,,,3,0,0d0+4;89,1,,,1,0,0d0+1|96,11,15,,0,0,1d5+10;112,4,,,4,0,0d0+4;89,1,,,1,0,0d0+1|5|1|3|50|100|false|true|false|false|0|0|0|0|XbPaPaXbPaPa||18;19;3;1;41|70|false', '96,7,11,,0,0,1d5+6;112,5,,,3,0,0d0+5;89,1,,,1,0,0d0+1|96,11,15,,0,0,1d5+10;112,5,,,4,0,0d0+5;89,1,,,1,0,0d0+1|5|1|3|50|100|false|true|false|false|0|0|0|0|XbPaPaXbPaPa||18;19;3;1;41|70|false', '96,8,12,,0,0,1d5+7;112,6,,,3,0,0d0+6;89,1,,,1,0,0d0+1|96,11,15,,0,0,1d5+10;112,6,,,4,0,0d0+6;89,1,,,1,0,0d0+1|5|1|3|50|100|false|true|false|false|0|0|0|0|XbPaPaXbPaPa||18;19;3;1;41|70|false', '96,9,13,,0,0,1d5+8;112,7,,,3,0,0d0+7;89,1,,,1,0,0d0+1|96,11,15,,0,0,1d5+10;112,7,,,4,0,0d0+7;89,1,,,1,0,0d0+1|5|1|3|50|100|false|true|false|false|0|0|0|0|XbPaPaXbPaPa||18;19;3;1;41|70|false', '96,9,13,,0,0,1d5+8;112,8,,,3,0,0d0+8;89,1,,,1,0,0d0+1|96,11,15,,0,0,1d5+10;112,8,,,4,0,0d0+8;89,1,,,1,0,0d0+1|4|1|3|50|100|false|true|false|false|0|0|0|0|XbPaPaXbPaPa||18;19;3;1;41|70|false', '96,9,13,,0,0,1d5+8;112,8,,,4,0,0d0+8;89,1,,,1,0,0d0+1|96,11,15,,0,0,1d5+10;112,8,,,4,0,0d0+8;89,1,,,1,0,0d0+1|3|1|3|50|100|false|true|false|false|0|0|0|0|XbPaPaXbPaPa||18;19;3;1;41|170|false', '0;32;32')",
+                "(319, 'Oniside', 0, '10,1,1', '98,21,70,,0,0,1d50+20;8,,,,0,0|98,101,150,,0,0,1d50+100;8,,,,0,0|7|1|6|20|0|false|true|false|true|0|0|0|0|PaPaPaPa||7;18;19;3;1;41|0|false', '98,26,75,,0,0,1d50+25;8,,,,0,0|98,101,150,,0,0,1d50+100;8,,,,0,0|7|1|6|20|0|false|true|false|true|0|0|0|0|PaPaPaPa||7;18;19;3;1;41|0|false', '98,31,80,,0,0,1d50+30;8,,,,0,0|98,101,150,,0,0,1d50+100;8,,,,0,0|7|1|6|20|0|false|true|false|true|0|0|0|0|PaPaPaPa||7;18;19;3;1;41|0|false', '98,41,90,,0,0,1d50+40;8,,,,0,0|98,101,150,,0,0,1d50+100;8,,,,0,0|7|1|6|20|0|false|true|false|true|0|0|0|0|PaPaPaPa||7;18;19;3;1;41|0|false', '98,51,100,,0,0,1d50+50;8,,,,0,0|98,101,150,,0,0,1d50+100;8,,,,0,0|7|1|6|20|0|false|true|false|true|0|0|0|0|PaPaPaPa||7;18;19;3;1;41|0|false', '', '')",
             }, ",") + ";"
         );
 

src/test/java/fr/quatrevieux/araknemu/game/fight/castable/effect/FunctionalTest.java

@@ -2010,6 +2010,47 @@ void shouldNotApplyingEffectWhenFightEnds() {
         assertTrue(fighter1.life().isFull());
     }
 
+    /**
+     * See: https://github.com/Arakne/Araknemu/issues/327
+     */
+    @Test
+    void killCasterThenSwitch() {
+        List<Fighter> fighters = configureFight(builder -> builder
+            .addSelf(fb -> fb.cell(136).currentLife(10).charac(Characteristic.INTELLIGENCE, 1000000))
+            .addAlly(fb -> fb.cell(165))
+            .addEnemy(fb -> fb.cell(122).charac(Characteristic.COUNTER_DAMAGE, 100).currentLife(1000).maxLife(1000))
+        );
+
+        fight.turnList().current().get().points().addActionPoints(10);
+        requestStack.clear();
+
+        castNormal(319, fight.map().get(122)); // Oniside
+
+        assertTrue(player.fighter().dead());
+        assertFalse(fighters.get(2).life().isFull());
+        assertEquals(122, fighters.get(2).cell().id());
+    }
+
+    /**
+     * See: https://github.com/Arakne/Araknemu/issues/327
+     */
+    @Test
+    void killTargetThenSwitch() {
+        List<Fighter> fighters = configureFight(builder -> builder
+            .addSelf(fb -> fb.cell(136))
+            .addAlly(fb -> fb.cell(165))
+            .addEnemy(fb -> fb.cell(122).currentLife(10))
+        );
+
+        fight.turnList().current().get().points().addActionPoints(10);
+        requestStack.clear();
+
+        castNormal(319, fight.map().get(122)); // Oniside
+
+        assertTrue(fighters.get(2).life().dead());
+        assertEquals(136, player.fighter().cell().id());
+    }
+
     private List<Fighter> configureFight(Consumer<FightBuilder> configurator) {
         fight.cancel(true);
 

src/test/java/fr/quatrevieux/araknemu/game/fight/castable/effect/handler/invocations/KillAndReplaceByInvocationHandlerTest.java

@@ -20,6 +20,7 @@
 package fr.quatrevieux.araknemu.game.fight.castable.effect.handler.invocations;
 
 import fr.quatrevieux.araknemu.data.constant.Characteristic;
+import fr.quatrevieux.araknemu.data.value.EffectArea;
 import fr.quatrevieux.araknemu.game.fight.Fight;
 import fr.quatrevieux.araknemu.game.fight.FightBaseCase;
 import fr.quatrevieux.araknemu.game.fight.ai.FighterAI;
@@ -37,6 +38,7 @@
 import fr.quatrevieux.araknemu.game.spell.SpellConstraints;
 import fr.quatrevieux.araknemu.game.spell.effect.SpellEffect;
 import fr.quatrevieux.araknemu.game.spell.effect.area.CellArea;
+import fr.quatrevieux.araknemu.game.spell.effect.area.CircleArea;
 import fr.quatrevieux.araknemu.game.spell.effect.target.SpellEffectTarget;
 import fr.quatrevieux.araknemu.network.game.out.fight.action.ActionEffect;
 import fr.quatrevieux.araknemu.network.game.out.fight.turn.FighterTurnOrder;
@@ -48,6 +50,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -122,6 +125,66 @@ void handle() {
         );
     }
 
+    @Test
+    void handleShouldNotInvocIfFightIfTerminated() {
+        Fighter target = fight.map().get(196).fighter();
+
+        SpellEffect effect = Mockito.mock(SpellEffect.class);
+        Spell spell = Mockito.mock(Spell.class);
+        SpellConstraints constraints = Mockito.mock(SpellConstraints.class);
+
+        Mockito.when(effect.area()).thenReturn(new CellArea());
+        Mockito.when(effect.min()).thenReturn(36); // bouftou
+        Mockito.when(effect.max()).thenReturn(1); // grade 1
+        Mockito.when(effect.target()).thenReturn(SpellEffectTarget.DEFAULT);
+        Mockito.when(spell.constraints()).thenReturn(constraints);
+
+        fight.map().get(167).fighter().life().kill(caster);
+        fight.map().get(168).fighter().life().kill(caster);
+        fight.map().get(197).fighter().life().kill(caster);
+        requestStack.clear();
+
+        FightCastScope scope = makeCastScope(caster, spell, effect, fight.map().get(196));
+        handler.handle(scope, scope.effects().get(0));
+
+        assertNull(fight.map().get(196).fighter());
+        assertTrue(target.dead());
+
+        requestStack.assertAll(
+            ActionEffect.fighterDie(caster, target)
+        );
+    }
+
+    @Test
+    void handleShouldStopApplyEffectOnFightEnd() {
+        Fighter target = fight.map().get(196).fighter();
+
+        SpellEffect effect = Mockito.mock(SpellEffect.class);
+        Spell spell = Mockito.mock(Spell.class);
+        SpellConstraints constraints = Mockito.mock(SpellConstraints.class);
+
+        Mockito.when(effect.area()).thenReturn(new CircleArea(new EffectArea(EffectArea.Type.CIRCLE, 20)));
+        Mockito.when(effect.min()).thenReturn(36); // bouftou
+        Mockito.when(effect.max()).thenReturn(1); // grade 1
+        Mockito.when(effect.target()).thenReturn(SpellEffectTarget.DEFAULT);
+        Mockito.when(spell.constraints()).thenReturn(constraints);
+
+        fight.map().get(167).fighter().life().kill(caster);
+        fight.map().get(168).fighter().life().kill(caster);
+        fight.map().get(197).fighter().life().kill(caster);
+        requestStack.clear();
+
+        FightCastScope scope = makeCastScope(caster, spell, effect, fight.map().get(196));
+        handler.handle(scope, scope.effects().get(0));
+
+        assertTrue(target.dead());
+        assertFalse(caster.dead());
+
+        requestStack.assertAll(
+            ActionEffect.fighterDie(caster, target)
+        );
+    }
+
     @Test
     void handleMaxInvocationCountReachShouldOnlyKill() {
         DoubleFighter invoc = new DoubleFighter(-5, caster);

src/test/java/fr/quatrevieux/araknemu/game/fight/castable/effect/handler/shifting/CarryingApplierTest.java

@@ -147,7 +147,7 @@ void stopWithFreeCellShouldAddCarried() {
         fight.dispatcher().add(FighterMoved.class, ref::set);
 
         applier.carry(caster, target);
-        caster.move(null); // free cell
+        caster.cell().removeFighter(caster); // free cell
         applier.stop(target);
 
         assertFalse(applier.active(caster));
@@ -167,7 +167,7 @@ void stopWithDeadCarrierShouldNotMoveItEvenIfCellIsFree() {
         FightCell cell = caster.cell();
 
         applier.carry(caster, target);
-        caster.move(null); // free cell
+        caster.cell().removeFighter(caster); // free cell
         target.life().kill(target);
         applier.stop(target);
 

src/test/java/fr/quatrevieux/araknemu/game/fight/castable/effect/handler/shifting/RaulebaqueHandlerTest.java

@@ -144,7 +144,7 @@ void handleWillNotMoveDeadFighter() {
         requestStack.assertLast(new FighterPositions(fight.fighters()));
 
         assertEquals(123, caster.cell().id());
-        assertThrows(IllegalStateException.class, other.fighter()::cell);
+        assertEquals(fight.map().get(124), other.fighter().cell());
         assertFalse(fight.map().get(124).hasFighter());
     }