Merge pull request #340 from vincent4vx/fix-cancel-fight-launch-no-fr…

…ee-cell

fix(fight): Cancel fight when not enough free cells are available

src/main/java/fr/quatrevieux/araknemu/game/fight/FightHandler.java

@@ -50,11 +50,13 @@ public Fight start(Consumer<B> configuration) {
         final Fight fight = builder.build(service.newFightId());
 
         service.modules(fight).forEach(fight::register);
-        fight.nextState();
 
-        fight.dispatcher().register(this);
-
-        service.created(fight);
+        // Execute the fight creation in another thread to avoid blocking the network thread
+        fight.execute(() -> {
+            fight.nextState();
+            fight.dispatcher().register(this);
+            service.created(fight);
+        });
 
         return fight;
     }

src/main/java/fr/quatrevieux/araknemu/game/fight/FighterList.java

@@ -128,6 +128,20 @@ public Stream<Fighter> alive() {
         return fighters.stream().filter(fighter -> !fighter.dead());
     }
 
+    /**
+     * Internal method for remove all given fighters.
+     *
+     * This method should only be used to roll back a call of {@link #join(Fighter, FightCell)},
+     * no events or other actions will be triggered.
+     */
+    public void removeAll(Collection<Fighter> fighters) {
+        for (Fighter fighter : fighters) {
+            if (this.fighters.remove(fighter)) {
+                fighter.cell().removeFighter(fighter);
+            }
+        }
+    }
+
     /**
      * Internal method for clear all fighters objects.
      */

src/main/java/fr/quatrevieux/araknemu/game/fight/map/util/PlacementCellsGenerator.java

@@ -22,6 +22,7 @@
 import fr.arakne.utils.value.helper.RandomUtil;
 import fr.quatrevieux.araknemu.game.fight.map.FightCell;
 import fr.quatrevieux.araknemu.game.fight.map.FightMap;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 import java.util.List;
 
@@ -56,8 +57,9 @@ private PlacementCellsGenerator(FightMap map, List<FightCell> available, RandomU
      * The next cell is free and walkable cell
      *
      * If there is no more available start place, a random cell will be taken from the entire map
+     * If there is no more free cell on the map, return null
      */
-    public FightCell next() {
+    public @Nullable FightCell next() {
         if (number >= available.size() - 1) {
             return randomFightCell();
         }
@@ -68,7 +70,7 @@ public FightCell next() {
     /**
      * Returns the next available start cell
      */
-    private FightCell nextAvailableCell() {
+    private @Nullable FightCell nextAvailableCell() {
         final FightCell cell = available.get(++number);
 
         if (cell.walkable()) {
@@ -81,20 +83,22 @@ private FightCell nextAvailableCell() {
     /**
      * Get a random cell from the entire map
      */
-    private FightCell randomFightCell() {
+    private @Nullable FightCell randomFightCell() {
         final int size = map.size();
 
         if (size < 1) {
-            throw new IllegalStateException("The map " + map.id() + " is empty");
+            return null;
         }
 
-        for (;;) {
+        for (int i = 0; i < 10; ++i) {
             final FightCell cell = map.get(random.nextInt(size));
 
             if (cell.walkable()) {
                 return cell;
             }
         }
+
+        return null;
     }
 
     /**

src/main/java/fr/quatrevieux/araknemu/game/fight/state/PlacementState.java

@@ -83,21 +83,18 @@ public void start(Fight fight) {
         this.fight = fight;
         this.cellsGenerators = new HashMap<>();
 
-        for (FightTeam team : fight.teams()) {
-            cellsGenerators.put(
-                team,
-                randomize
-                    ? PlacementCellsGenerator.randomized(fight.map(), team.startPlaces())
-                    : new PlacementCellsGenerator(fight.map(), team.startPlaces())
-            );
-        }
-
         fight.dispatcher().register(this);
         startTime = System.currentTimeMillis();
 
-        // Add all fighters to fight
-        // Note: fight.fighters() cannot be used because at this state fighters are not yet on fight
-        addFighters(fight.teams().stream().flatMap(team -> team.fighters().stream()).collect(Collectors.toList()));
+        try {
+            // Add all fighters to fight
+            // Note: fight.fighters() cannot be used because at this state fighters are not yet on fight
+            addFighters(fight.teams().stream().flatMap(team -> team.fighters().stream()).collect(Collectors.toList()));
+        } catch (Exception e) {
+            fight.cancel();
+
+            throw new FightException("Cannot add fighters", e);
+        }
 
         if (fight.type().hasPlacementTimeLimit()) {
             timer = fight.schedule(this::innerStartFight, fight.type().placementDuration());
@@ -189,7 +186,13 @@ public synchronized void joinTeam(Fighter fighter, FightTeam team) throws JoinFi
         }
 
         team.join(fighter);
-        addFighters(Collections.singleton(fighter));
+
+        try {
+            addFighters(Collections.singleton(fighter));
+        } catch (FightException e) {
+            team.kick(fighter);
+            throw new JoinFightException(JoinFightError.CHALLENGE_FULL);
+        }
     }
 
     @Override
@@ -287,13 +290,38 @@ private void punishDeserter(Fighter fighter) {
         fighter.dispatch(new FightLeaved(rewardsSheet.rewards().get(0)));
     }
 
+    /**
+     * Add given fighters to the fight
+     *
+     * This method may fail, in this case the fighters list will be unchanged,
+     * so this method can be considered as atomic
+     *
+     * @throws FightException When cannot found a cell for a fighter
+     */
     @RequiresNonNull("fight")
-    @SuppressWarnings("dereference.of.nullable") // cellsGenerators.get(fighter.team()) cannot be null
     private void addFighters(Collection<Fighter> fighters) {
+        final Fight fight = this.fight;
+        final Map<FightTeam, PlacementCellsGenerator> cellsGenerators = NullnessUtil.castNonNull(this.cellsGenerators);
         final FighterList fightersList = fight.fighters();
 
         for (Fighter fighter : fighters) {
-            fightersList.join(fighter, NullnessUtil.castNonNull(cellsGenerators).get(fighter.team()).next());
+            final FightCell joinCell = cellsGenerators
+                .computeIfAbsent(fighter.team(), team -> randomize
+                    ? PlacementCellsGenerator.randomized(fight.map(), team.startPlaces())
+                    : new PlacementCellsGenerator(fight.map(), team.startPlaces())
+                )
+                .next()
+            ;
+
+            // No free cell available, so cancel the join
+            // Note: no events are triggers in this case and the state is unchanged
+            if (joinCell == null) {
+                fightersList.removeAll(fighters);
+
+                throw new FightException("Cannot found a cell for the fighter");
+            }
+
+            fightersList.join(fighter, joinCell);
         }
 
         for (Fighter fighter : fighters) {

src/test/java/fr/quatrevieux/araknemu/game/fight/FighterListTest.java

@@ -20,6 +20,7 @@
 package fr.quatrevieux.araknemu.game.fight;
 
 import fr.quatrevieux.araknemu.game.fight.event.FighterRemoved;
+import fr.quatrevieux.araknemu.game.fight.exception.FightException;
 import fr.quatrevieux.araknemu.game.fight.fighter.AbstractFighter;
 import fr.quatrevieux.araknemu.game.fight.fighter.Fighter;
 import fr.quatrevieux.araknemu.game.fight.fighter.PlayableFighter;
@@ -29,13 +30,16 @@
 
 import java.lang.reflect.Field;
 import java.sql.SQLException;
+import java.util.Collections;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
 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;
 
 class FighterListTest extends FightBaseCase {
@@ -202,4 +206,15 @@ class Foo {}
         fight.dispatchToAll(new Foo());
         assertEquals(1, ai.get());
     }
+
+    @Test
+    void removeAll() throws SQLException {
+        Fighter fighter = new PlayerFighter(makeSimpleGamePlayer(10));
+        fighterList.join(fighter, fight.map().get(146));
+
+        fighterList.removeAll(Collections.singleton(fighter));
+
+        assertFalse(fighterList.all().contains(fighter));
+        assertFalse(fight.map().get(146).hasFighter());
+    }
 }

src/test/java/fr/quatrevieux/araknemu/game/fight/map/util/PlacementCellsGeneratorTest.java

@@ -19,8 +19,16 @@
 
 package fr.quatrevieux.araknemu.game.fight.map.util;
 
+import fr.arakne.utils.maps.serializer.CellData;
+import fr.arakne.utils.value.Dimensions;
+import fr.quatrevieux.araknemu.data.value.Geolocation;
+import fr.quatrevieux.araknemu.data.world.entity.environment.MapTemplate;
 import fr.quatrevieux.araknemu.game.GameBaseCase;
+import fr.quatrevieux.araknemu.game.exploration.area.AreaService;
+import fr.quatrevieux.araknemu.game.exploration.map.ExplorationMap;
 import fr.quatrevieux.araknemu.game.exploration.map.ExplorationMapService;
+import fr.quatrevieux.araknemu.game.exploration.map.cell.CellLoader;
+import fr.quatrevieux.araknemu.game.exploration.map.cell.CellLoaderAggregate;
 import fr.quatrevieux.araknemu.game.fight.FightService;
 import fr.quatrevieux.araknemu.game.fight.fighter.Fighter;
 import fr.quatrevieux.araknemu.game.fight.map.FightCell;
@@ -31,9 +39,11 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 class PlacementCellsGeneratorTest extends GameBaseCase {
@@ -92,4 +102,38 @@ void nextWithAllAvailableCellsNotFree() {
         assertNotEquals(124, cell.id());
         assertNotEquals(125, cell.id());
     }
+
+    @Test
+    void nextWithEmptyMap() {
+        map = new FightMap(
+            new MapTemplate(
+                0,
+                "",
+                new Dimensions(0, 0),
+                "",
+                new CellData[0],
+                new int[2][0],
+                new Geolocation(0, 0),
+                0,
+                false
+            )
+        );
+
+        PlacementCellsGenerator generator = new PlacementCellsGenerator(map, Collections.emptyList());
+
+        assertNull(generator.next());
+    }
+
+    @Test
+    void nextWithoutAvailableCell() {
+        PlacementCellsGenerator generator = new PlacementCellsGenerator(map, Arrays.asList(map.get(123), map.get(124), map.get(125)));
+
+        for (int i = 0; i < map.size(); ++i) {
+            if (map.get(i).walkable()) {
+                map.get(i).set(Mockito.mock(Fighter.class));
+            }
+        }
+
+        assertNull(generator.next());
+    }
 }