Merge pull request #357 from vincent4vx/fix-end-alter-point-effect-on…

…-current-turn fix(fight): do not modify turn points on debuff
Arakne · Jul 12, 2024 · a56ec07 · a56ec07
2 parents b350d4a + 02fefd7
commit a56ec07
Show file tree

Hide file tree

Showing 12 changed files with 125 additions and 16 deletions.
diff --git a/...x/araknemu/game/fight/castable/effect/handler/characteristic/AlterCharacteristicHook.java b/...x/araknemu/game/fight/castable/effect/handler/characteristic/AlterCharacteristicHook.java
@@ -60,14 +60,14 @@ public final void onBuffStarted(FightBuff buff) {
         final int effectValue = buff.effect().min();
         final int appliedValue = effectValue * multiplier;
 
-        apply(buff, target, appliedValue);
+        apply(buff, target, appliedValue, false);
         fight.send(ActionEffect.buff(buff, applyMultiplierOnPacketValue ? appliedValue : effectValue));
         target.buffs().onCharacteristicAltered(characteristic, appliedValue);
     }
 
     @Override
     public final void onBuffTerminated(FightBuff buff) {
-        apply(buff, buff.target(), -buff.effect().min() * multiplier);
+        apply(buff, buff.target(), -buff.effect().min() * multiplier, true);
     }
 
     /**
@@ -77,8 +77,9 @@ public final void onBuffTerminated(FightBuff buff) {
      * @param buff Buff to apply
      * @param target Buff target
      * @param value Value to apply. Negative for removing, positive for adding
+     * @param buffTerminated If this method is called because the buff is terminated (i.e. the value parameter is negative)
      */
-    protected void apply(FightBuff buff, Fighter target, int value) {
+    protected void apply(FightBuff buff, Fighter target, int value, boolean buffTerminated) {
         target.characteristics().alter(characteristic, value);
     }
 

diff --git a/...revieux/araknemu/game/fight/castable/effect/handler/characteristic/AlterVitalityHook.java b/...revieux/araknemu/game/fight/castable/effect/handler/characteristic/AlterVitalityHook.java
@@ -23,11 +23,10 @@
 import fr.quatrevieux.araknemu.game.fight.Fight;
 import fr.quatrevieux.araknemu.game.fight.castable.effect.buff.FightBuff;
 import fr.quatrevieux.araknemu.game.fight.fighter.Fighter;
-import fr.quatrevieux.araknemu.game.fight.fighter.FighterData;
 
 /**
  * Hook for handle vitality alteration
- * Synchronize fighter life using {@link fr.quatrevieux.araknemu.game.fight.fighter.FighterLife#alterMax(FighterData, int)}
+ * Synchronize fighter life using {@link fr.quatrevieux.araknemu.game.fight.fighter.FighterLife#alterMax(Fighter, int)}
  * in addition to characteristic change performed by the base {@link AlterCharacteristicHook}
  *
  * @see AddVitalityHandler
@@ -43,8 +42,8 @@ private AlterVitalityHook(Fight fight, int multiplier) {
     }
 
     @Override
-    protected void apply(FightBuff buff, Fighter target, int value) {
-        super.apply(buff, target, value);
+    protected void apply(FightBuff buff, Fighter target, int value, boolean buffTerminated) {
+        super.apply(buff, target, value, buffTerminated);
         target.life().alterMax(buff.caster(), value);
     }
 

diff --git a/...ieux/araknemu/game/fight/castable/effect/handler/characteristic/point/AlterPointHook.java b/...ieux/araknemu/game/fight/castable/effect/handler/characteristic/point/AlterPointHook.java
@@ -29,7 +29,7 @@
 
 /**
  * Buff hook for handle altering turn point characteristics (i.e. {@link Characteristic#ACTION_POINT} and {@link Characteristic#MOVEMENT_POINT})
- * This hook will also update point of the current active turn, if applicable
+ * This hook will also update point of the current active turn, if applicable (only on buff start, debuff is ignored)
  *
  * Use factory methods for create the hook instance
  */
@@ -45,13 +45,17 @@ protected AlterPointHook(Fight fight, Characteristic characteristic, int multipl
     }
 
     @Override
-    protected void apply(FightBuff buff, Fighter target, int value) {
-        super.apply(buff, target, value);
+    protected void apply(FightBuff buff, Fighter target, int value, boolean buffTerminated) {
+        super.apply(buff, target, value, buffTerminated);
 
-        fight.turnList().current()
-            .filter(turn -> turn.fighter().equals(target))
-            .ifPresent(turn -> modifier.modify(turn.points(), buff.effect().min()))
-        ;
+        // Only modify turn points on buff start
+        // So turn points will not be modified on debuff / buff caster killed
+        if (!buffTerminated) {
+            fight.turnList().current()
+                .filter(turn -> turn.fighter().equals(target))
+                .ifPresent(turn -> modifier.modify(turn.points(), buff.effect().min()))
+            ;
+        }
     }
 
     /**

diff --git a/src/test/java/fr/quatrevieux/araknemu/game/GameDataSet.java b/src/test/java/fr/quatrevieux/araknemu/game/GameDataSet.java
@@ -573,6 +573,8 @@ public GameDataSet pushFunctionalSpells() throws SQLException, ContainerExceptio
                 "(320, 'Incurable', 0, '10,0,1', '132,,,,0,0|100,51,100,,-1,0,1d50+50|6|1|8|2|0|false|false|false|false|0|0|0|3|PaPa||18;19;3;1;41|0|false', '132,,,,0,0|100,61,110,,-1,0,1d50+60|6|1|9|2|0|false|false|false|false|0|0|0|3|PaPa||18;19;3;1;41|0|false', '132,,,,0,0|100,71,120,,-1,0,1d50+70|6|1|10|2|0|false|false|false|false|0|0|0|3|PaPa||18;19;3;1;41|0|false', '132,,,,0,0|100,81,130,,-1,0,1d50+80|6|1|11|2|0|false|false|false|false|0|0|0|3|PaPa||18;19;3;1;41|0|false', '132,,,,0,0|100,91,140,,-1,0,1d50+90|6|1|12|2|0|false|false|false|false|0|0|0|3|PaPa||18;19;3;1;41|0|false', '', '0,64')",
                 "(1010, 'Rascasse', 0, '0,1,0', '212,50,,,2,0,0d0+50;211,50,,,2,0,0d0+50;213,50,,,2,0,0d0+50;214,50,,,2,0,0d0+50;210,50,,,2,0,0d0+50||3|0|0|0|50|false|true|false|false|0|0|0|3|PaPaPaPaPa||18;19;3;1;41|0|true', '212,50,,,2,0,0d0+50;211,50,,,2,0,0d0+50;213,50,,,2,0,0d0+50;214,50,,,2,0,0d0+50;210,50,,,2,0,0d0+50||3|0|0|0|50|false|true|false|false|0|0|0|3|PaPaPaPaPa||18;19;3;1;41|0|true', '212,50,,,2,0,0d0+50;211,50,,,2,0,0d0+50;213,50,,,2,0,0d0+50;214,50,,,2,0,0d0+50;210,50,,,2,0,0d0+50||3|0|0|0|50|false|true|false|false|0|0|0|3|PaPaPaPaPa||18;19;3;1;41|0|true', '212,50,,,2,0,0d0+50;211,50,,,2,0,0d0+50;213,50,,,2,0,0d0+50;214,50,,,2,0,0d0+50;210,50,,,2,0,0d0+50||3|0|0|0|50|false|true|false|false|0|0|0|3|PaPaPaPaPa||18;19;3;1;41|0|true', '212,50,,,2,0,0d0+50;211,50,,,2,0,0d0+50;213,50,,,2,0,0d0+50;214,50,,,2,0,0d0+50;210,50,,,2,0,0d0+50||3|0|0|0|50|false|true|false|false|0|0|0|3|PaPaPaPaPa||18;19;3;1;41|0|true', '', '320;256;384;128;192')",
                 "(1038, 'Rasage', 0, '0,2,0', '125,100,,,10,0,0d0+100;111,2,,,4,0,0d0+2|125,200,,,10,0,0d0+200;111,4,,,4,0,0d0+4|1|0|0|50|50|false|true|false|false|0|1|0|4|PaPaPaPa||18;19;3;1;41|0|true', '125,100,,,10,0,0d0+100;111,2,,,4,0,0d0+2|125,200,,,10,0,0d0+200;111,4,,,4,0,0d0+4|1|0|0|50|50|false|true|false|false|0|1|0|4|PaPaPaPa||18;19;3;1;41|0|true', '125,100,,,10,0,0d0+100;111,2,,,4,0,0d0+2|125,200,,,10,0,0d0+200;111,4,,,4,0,0d0+4|1|0|0|50|50|false|true|false|false|0|1|0|4|PaPaPaPa||18;19;3;1;41|0|true', '125,100,,,10,0,0d0+100;111,2,,,4,0,0d0+2|125,200,,,10,0,0d0+200;111,4,,,4,0,0d0+4|1|0|0|50|50|false|true|false|false|0|1|0|4|PaPaPaPa||18;19;3;1;41|0|true', '125,100,,,10,0,0d0+100;111,2,,,4,0,0d0+2|125,200,,,10,0,0d0+200;111,4,,,4,0,0d0+4|1|0|0|50|50|false|true|false|false|0|1|0|4|PaPaPaPa||18;19;3;1;41|0|true', '', '0;448')",
+                "(85, 'Flou', 503, '11,1,1', '168,2,,,2,0,0d0+2;666,,,,0,90;120,1,,,0,10,0d0+1;950,,,39,5,0||3|4|4|0|100|false|true|false|false|0|0|0|10|CePaPaC_||39;18;19;3;1;41|21|false', '168,2,,,2,0,0d0+2;666,,,,0,90;120,1,,,0,10,0d0+1;950,,,39,5,0||3|4|4|0|100|false|true|false|false|0|0|0|9|CePaPaC_||39;18;19;3;1;41|21|false', '168,2,,,2,0,0d0+2;666,,,,0,90;120,1,,,0,10,0d0+1;950,,,39,5,0||3|4|4|0|100|false|true|false|false|0|0|0|8|CePaPaC_||39;18;19;3;1;41|21|false', '168,2,,,2,0,0d0+2;666,,,,0,90;120,1,,,0,10,0d0+1;950,,,39,5,0||3|4|4|0|100|false|true|false|false|0|0|0|7|CePaPaC_||39;18;19;3;1;41|21|false', '168,2,,,2,0,0d0+2;666,,,,0,90;120,1,,,0,10,0d0+1;950,,,39,5,0||3|4|4|0|100|false|true|false|false|0|0|0|6|CePaPaC_||39;18;19;3;1;41|21|false', '168,2,,,2,0,0d0+2;666,,,,0,90;120,1,,,0,10,0d0+1;950,,,39,5,0||3|4|4|0|100|false|true|false|false|0|0|0|5|CePaPaC_||39;18;19;3;1;41|121|false', '')",
+                "(188, 'Ronce Insolente', 1015, '51,1,1', '132,,,,1,0||6|0|3|0|100|true|true|false|true|0|0|0|5|Pa||18;19;3;1;41|90|false', '132,,,,1,0||6|0|5|0|100|true|true|false|true|0|0|0|4|Pa||18;19;3;1;41|90|false', '132,,,,1,0||5|0|6|0|100|true|true|false|true|0|0|0|3|Pa||18;19;3;1;41|90|false', '132,,,,1,0||4|0|7|0|100|true|true|false|true|0|0|0|3|Pa||18;19;3;1;41|90|false', '132,,,,1,0||4|0|10|0|100|true|true|false|true|0|0|0|3|Pa||18;19;3;1;41|90|false', '132,,,,1,0||3|0|12|0|100|true|true|false|true|0|0|0|3|Pa||18;19;3;1;41|190|false', '')",
             }, ",") + ";"
         );
 

diff --git a/src/test/java/fr/quatrevieux/araknemu/game/fight/ai/factory/ScriptingAiLoaderTest.java b/src/test/java/fr/quatrevieux/araknemu/game/fight/ai/factory/ScriptingAiLoaderTest.java
@@ -31,6 +31,7 @@
 import java.lang.reflect.Field;
 import java.net.MalformedURLException;
 import java.nio.file.Paths;
+import java.util.Comparator;
 import java.util.List;
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
@@ -82,7 +83,9 @@ void loadWithHotReload() throws NoSuchFieldException, IllegalAccessException, Ma
         assertSame(container.get(Simulator.class), f.get(HotReloadableScript.class.cast(factories.get(1)).getInternalInstance()));
     }
 
-    private <T> List<T> toList(Iterable<T> i) {
-        return StreamSupport.stream(i.spliterator(), false).collect(Collectors.toList());
+    private <T extends NamedAiFactory> List<T> toList(Iterable<T> i) {
+        return StreamSupport.stream(i.spliterator(), false)
+            .sorted(Comparator.comparing(NamedAiFactory::name))
+            .collect(Collectors.toList());
     }
 }
diff --git a/src/test/java/fr/quatrevieux/araknemu/game/fight/castable/effect/FunctionalTest.java b/src/test/java/fr/quatrevieux/araknemu/game/fight/castable/effect/FunctionalTest.java
@@ -2302,6 +2302,36 @@ void applyOnActionPointLost() {
         assertEquals(4, buffs.get(2).remainingTurns());
     }
 
+    @Test
+    void endOfActionPointLostShouldNotChangeCurrentTurnPoints() {
+        fighter1.move(fight.map().get(210));
+        fighter2.move(fight.map().get(150));
+        fighter2.characteristics().alter(Characteristic.ACTION_POINT, 4); // 10 AP
+
+        castNormal(85, fight.map().get(150)); // Flou
+        fighter1.turn().stop();
+
+        assertEquals(8, fighter2.turn().points().actionPoints());
+        castNormal(188, fighter2.cell()); // Ronce insolente
+
+        // Remove only the spell cost, do not change current turn points
+        assertEquals(4, fighter2.turn().points().actionPoints());
+    }
+
+    @Test
+    void endOfActionPointAddShouldNotChangeCurrentTurnPoints() {
+        fighter1.move(fight.map().get(210));
+        fighter2.move(fight.map().get(150));
+
+        castNormal(126, fight.map().get(210)); // Mot stimulant
+        assertEquals(6, fighter1.turn().points().actionPoints()); // -2 + 2
+
+        castNormal(188, fighter1.cell()); // Ronce insolente
+
+        // Remove only the spell cost, do not change current turn points
+        assertEquals(2, fighter1.turn().points().actionPoints());
+    }
+
     private List<Fighter> configureFight(Consumer<FightBuilder> configurator) {
         fight.cancel(true);
 

diff --git a/...raknemu/game/fight/castable/effect/handler/characteristic/AddActionPointsHandlerTest.java b/...raknemu/game/fight/castable/effect/handler/characteristic/AddActionPointsHandlerTest.java
@@ -205,5 +205,6 @@ void onBuffStartedAndTerminatedOnCaster() {
 
         handler.onBuffTerminated(buff);
         assertEquals(6, caster.characteristics().get(Characteristic.ACTION_POINT));
+        assertEquals(9, caster.turn().points().actionPoints());
     }
 }
diff --git a/...knemu/game/fight/castable/effect/handler/characteristic/AddMovementPointsHandlerTest.java b/...knemu/game/fight/castable/effect/handler/characteristic/AddMovementPointsHandlerTest.java
@@ -205,5 +205,6 @@ void onBuffStartedAndTerminatedOnCaster() {
 
         handler.onBuffTerminated(buff);
         assertEquals(3, caster.characteristics().get(Characteristic.MOVEMENT_POINT));
+        assertEquals(6, caster.turn().points().movementPoints());
     }
 }
diff --git a/...nemu/game/fight/castable/effect/handler/characteristic/RemoveActionPointsHandlerTest.java b/...nemu/game/fight/castable/effect/handler/characteristic/RemoveActionPointsHandlerTest.java
@@ -205,5 +205,6 @@ void onBuffStartedAndTerminatedOnCaster() {
 
         handler.onBuffTerminated(buff);
         assertEquals(6, caster.characteristics().get(Characteristic.ACTION_POINT));
+        assertEquals(3, caster.turn().points().actionPoints());
     }
 }
diff --git a/...mu/game/fight/castable/effect/handler/characteristic/RemoveMovementPointsHandlerTest.java b/...mu/game/fight/castable/effect/handler/characteristic/RemoveMovementPointsHandlerTest.java
@@ -205,5 +205,6 @@ void onBuffStartedAndTerminatedOnCaster() {
 
         handler.onBuffTerminated(buff);
         assertEquals(3, caster.characteristics().get(Characteristic.MOVEMENT_POINT));
+        assertEquals(0, caster.turn().points().movementPoints());
     }
 }
diff --git a/...u/game/fight/castable/effect/handler/characteristic/point/ActionPointLostHandlerTest.java b/...u/game/fight/castable/effect/handler/characteristic/point/ActionPointLostHandlerTest.java
@@ -195,6 +195,39 @@ void applyOnSelfShouldRemovePointsFromCurrentTurn() {
         requestStack.assertOne("GIE101;1;1;;0;;2;0");
     }
 
+    @Test
+    void debuffOnCurrentTurnShouldNotModifyTurnPoints() {
+        requestStack.clear();
+
+        SpellEffect effect = Mockito.mock(SpellEffect.class);
+        Spell spell = Mockito.mock(Spell.class);
+        SpellConstraints constraints = Mockito.mock(SpellConstraints.class);
+
+        Mockito.when(effect.effect()).thenReturn(101);
+        Mockito.when(effect.min()).thenReturn(2);
+        Mockito.when(effect.duration()).thenReturn(2);
+        Mockito.when(effect.area()).thenReturn(new CellArea());
+        Mockito.when(effect.target()).thenReturn(SpellEffectTarget.DEFAULT);
+        Mockito.when(spell.constraints()).thenReturn(constraints);
+        Mockito.when(constraints.freeCell()).thenReturn(false);
+
+        FightCastScope scope = makeCastScope(caster, spell, effect, caster.cell());
+        handler.buff(scope, scope.effects().get(0));
+
+        Optional<FightBuff> buff = caster.buffs().stream().filter(b -> b.effect().effect() == 101).findFirst();
+
+        assertTrue(buff.isPresent());
+        assertEquals(1, buff.get().effect().min());
+        assertEquals(3, buff.get().remainingTurns());
+        assertEquals(5, caster.characteristics().get(Characteristic.ACTION_POINT));
+        assertEquals(5, caster.turn().points().actionPoints());
+
+        caster.buffs().removeAll(); // Debuff
+
+        assertEquals(6, caster.characteristics().get(Characteristic.ACTION_POINT));
+        assertEquals(5, caster.turn().points().actionPoints());
+    }
+
     @Test
     void dodgedAllShouldNotAddBuff() {
         target.characteristics().alter(Characteristic.RESISTANCE_ACTION_POINT, 1000);

diff --git a/...game/fight/castable/effect/handler/characteristic/point/MovementPointLostHandlerTest.java b/...game/fight/castable/effect/handler/characteristic/point/MovementPointLostHandlerTest.java
@@ -195,6 +195,39 @@ void applyOnSelfShouldRemovePointsFromCurrentTurn() {
         requestStack.assertOne("GIE127;1;1;;0;;2;0");
     }
 
+    @Test
+    void debuffOnCurrentTurnShouldNotModifyTurnPoints() {
+        requestStack.clear();
+
+        SpellEffect effect = Mockito.mock(SpellEffect.class);
+        Spell spell = Mockito.mock(Spell.class);
+        SpellConstraints constraints = Mockito.mock(SpellConstraints.class);
+
+        Mockito.when(effect.effect()).thenReturn(127);
+        Mockito.when(effect.min()).thenReturn(2);
+        Mockito.when(effect.duration()).thenReturn(2);
+        Mockito.when(effect.area()).thenReturn(new CellArea());
+        Mockito.when(effect.target()).thenReturn(SpellEffectTarget.DEFAULT);
+        Mockito.when(spell.constraints()).thenReturn(constraints);
+        Mockito.when(constraints.freeCell()).thenReturn(false);
+
+        FightCastScope scope = makeCastScope(caster, spell, effect, caster.cell());
+        handler.buff(scope, scope.effects().get(0));
+
+        Optional<FightBuff> buff = caster.buffs().stream().filter(b -> b.effect().effect() == 127).findFirst();
+
+        assertTrue(buff.isPresent());
+        assertEquals(1, buff.get().effect().min());
+        assertEquals(3, buff.get().remainingTurns());
+        assertEquals(2, caster.characteristics().get(Characteristic.MOVEMENT_POINT));
+        assertEquals(2, caster.turn().points().movementPoints());
+
+        caster.buffs().removeAll(); // Debuff
+
+        assertEquals(3, caster.characteristics().get(Characteristic.MOVEMENT_POINT));
+        assertEquals(2, caster.turn().points().movementPoints());
+    }
+
     @Test
     void dodgedAllShouldNotAddBuff() {
         target.characteristics().alter(Characteristic.RESISTANCE_MOVEMENT_POINT, 1000);