Fixes and Refactorings, no major changes

src/main/java/com/seodisparate/TurnBasedMinecraft/common/AttackEventHandler.java

@@ -1,7 +1,6 @@
 package com.seodisparate.TurnBasedMinecraft.common;
 
-import java.util.ArrayDeque;
+import java.util.Iterator;
-import java.util.Queue;
 
 import com.seodisparate.TurnBasedMinecraft.common.networking.PacketBattleMessage;
 
@@ -23,20 +22,20 @@ public class AttackEventHandler
         }
         else
         {
-            Queue<AttackerViaBow> removeQueue = new ArrayDeque<AttackerViaBow>();
             final long now = System.nanoTime();
             boolean isValid = false;
             synchronized(TurnBasedMinecraftMod.proxy.getAttackerViaBowSet())
             {
-                for(AttackerViaBow attacker : TurnBasedMinecraftMod.proxy.getAttackerViaBowSet())
+                for(Iterator<AttackerViaBow> iter = TurnBasedMinecraftMod.proxy.getAttackerViaBowSet().iterator(); iter.hasNext();)
                 {
+                    AttackerViaBow attacker = iter.next();
                     if(now - attacker.attackTime >= AttackerViaBow.ATTACK_TIMEOUT)
                     {
-                        removeQueue.add(attacker);
+                        iter.remove();
                     }
                     else if(event.getSource().getTrueSource().equals(attacker.entity) && event.getSource().isProjectile())
                     {
-                        removeQueue.add(attacker);
+                        iter.remove();
                         if(!isValid)
                         {
                             Battle b = TurnBasedMinecraftMod.proxy.getBattleManager().getBattleByID(attacker.battleID);
@@ -48,10 +47,6 @@ public class AttackEventHandler
                         }
                     }
                 }
-                for(AttackerViaBow next = removeQueue.poll(); next != null; next = removeQueue.poll())
-                {
-                    TurnBasedMinecraftMod.proxy.getAttackerViaBowSet().remove(next);
-                }
             }
             return isValid;
         }

src/main/java/com/seodisparate/TurnBasedMinecraft/common/Battle.java

@@ -5,6 +5,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Hashtable;
+import java.util.Iterator;
 import java.util.Map;
 import java.util.PriorityQueue;
 import java.util.Queue;
@@ -518,50 +519,52 @@ public class Battle
      */
     private boolean healthCheck()
     {
-        Queue<Combatant> removeQueue = new ArrayDeque<Combatant>();
+        boolean didRemove = false;
-        for(Combatant c : sideA.values())
+        for(Iterator<Map.Entry<Integer, Combatant>> iter = sideA.entrySet().iterator(); iter.hasNext();)
         {
-            if(!c.entity.isEntityAlive())
+            Map.Entry<Integer, Combatant> entry = iter.next();
+            if(!entry.getValue().entity.isEntityAlive())
             {
-                removeQueue.add(c);
+                iter.remove();
+                players.remove(entry.getKey());
+                removeCombatantPostRemove(entry.getValue());
+                didRemove = true;
                 String category = new String();
-                if(c.entityInfo != null)
+                if(entry.getValue().entityInfo != null)
                 {
-                    category = c.entityInfo.category;
+                    category = entry.getValue().entityInfo.category;
                 }
-                else if(c.entity instanceof EntityPlayer)
+                else if(entry.getValue().entity instanceof EntityPlayer)
                 {
                     category = "player";
                 }
-                sendMessageToAllPlayers(PacketBattleMessage.MessageType.DIED, c.entity.getEntityId(), 0, 0, category);
+                sendMessageToAllPlayers(PacketBattleMessage.MessageType.DIED, entry.getValue().entity.getEntityId(), 0, 0, category);
             }
         }
-        for(Combatant c : sideB.values())
+        for(Iterator<Map.Entry<Integer, Combatant>> iter = sideB.entrySet().iterator(); iter.hasNext();)
         {
-            if(!c.entity.isEntityAlive())
+            Map.Entry<Integer, Combatant> entry = iter.next();
+            if(!entry.getValue().entity.isEntityAlive())
             {
-                removeQueue.add(c);
+                iter.remove();
+                players.remove(entry.getKey());
+                removeCombatantPostRemove(entry.getValue());
+                didRemove = true;
                 String category = new String();
-                if(c.entityInfo != null)
+                if(entry.getValue().entityInfo != null)
                 {
-                    category = c.entityInfo.category;
+                    category = entry.getValue().entityInfo.category;
                 }
-                else if(c.entity instanceof EntityPlayer)
+                else if(entry.getValue().entity instanceof EntityPlayer)
                 {
                     category = "player";
                 }
-                sendMessageToAllPlayers(PacketBattleMessage.MessageType.DIED, c.entity.getEntityId(), 0, 0, category);
+                sendMessageToAllPlayers(PacketBattleMessage.MessageType.DIED, entry.getValue().entity.getEntityId(), 0, 0, category);
             }
         }
-        boolean didRemove = !removeQueue.isEmpty();
-        for(Combatant toRemove = removeQueue.poll(); toRemove != null; toRemove = removeQueue.poll())
-        {
-            removeCombatant(toRemove);
-        }
         if(players.isEmpty() || sideA.isEmpty() || sideB.isEmpty())
         {
             battleEnded = true;
-            sendMessageToAllPlayers(PacketBattleMessage.MessageType.ENDED, 0, 0, 0);
         }
         else if(didRemove)
         {
@@ -576,20 +579,20 @@ public class Battle
      */
     private boolean isCreativeCheck()
     {
-        Queue<Combatant> removeQueue = new ArrayDeque<Combatant>();
-        for(Combatant c : players.values())
-        {
-            if(c.entity != null && ((EntityPlayer)c.entity).isCreative())
-            {
-                removeQueue.add(c);
-            }
-        }
         boolean didRemove = false;
-        for(Combatant toRemove = removeQueue.poll(); toRemove != null; toRemove = removeQueue.poll())
+        for(Iterator<Map.Entry<Integer, Combatant>> iter = players.entrySet().iterator(); iter.hasNext();)
         {
-            didRemove = true;
+            Map.Entry<Integer, Combatant> entry = iter.next();
-            removeCombatant(toRemove);
+            if(entry.getValue().entity != null && ((EntityPlayer)entry.getValue().entity).isCreative())
-            sendMessageToAllPlayers(PacketBattleMessage.MessageType.BECAME_CREATIVE, toRemove.entity.getEntityId(), 0, 0);
+            {
+                sendMessageToAllPlayers(PacketBattleMessage.MessageType.BECAME_CREATIVE, entry.getValue().entity.getEntityId(), 0, 0);
+                iter.remove();
+                sideA.remove(entry.getKey());
+                sideB.remove(entry.getKey());
+                playerCount.decrementAndGet();
+                removeCombatantPostRemove(entry.getValue());
+                didRemove = true;
+            }
         }
         if(didRemove)
         {
@@ -633,6 +636,14 @@ public class Battle
         if(players.remove(c.entity.getEntityId()) != null)
         {
             playerCount.decrementAndGet();
+        }
+        removeCombatantPostRemove(c);
+    }
+    
+    private void removeCombatantPostRemove(Combatant c)
+    {
+        if(c.entity instanceof EntityPlayer)
+        {
             TurnBasedMinecraftMod.NWINSTANCE.sendTo(new PacketBattleMessage(PacketBattleMessage.MessageType.ENDED, 0, 0, 0), (EntityPlayerMP)c.entity);
         }
         battleManager.addRecentlyLeftBattle(c);

src/main/java/com/seodisparate/TurnBasedMinecraft/common/BattleManager.java

@@ -1,14 +1,10 @@
 package com.seodisparate.TurnBasedMinecraft.common;
 
-import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.Iterator;
-import java.util.Hashtable;
 import java.util.Map;
-import java.util.Queue;
-import java.util.Set;
 
 import org.apache.logging.log4j.Logger;
 
@@ -32,7 +28,7 @@ public class BattleManager
     public BattleManager(Logger logger)
     {
         this.logger = logger;
-        battleMap = new Hashtable<Integer, Battle>();
+        battleMap = new HashMap<Integer, Battle>();
         battleUpdater = new BattleUpdater(this);
         updaterThread = new Thread(battleUpdater);
         updaterThread.start();
@@ -61,12 +57,15 @@ public class BattleManager
         if(entityInfo != null && (TurnBasedMinecraftMod.proxy.getConfig().isIgnoreBattleType(entityInfo.category) || entityInfo.ignoreBattle))
         {
             // attacked entity ignores battle
-            for(Battle b : battleMap.values())
+            synchronized(battleMap)
             {
-                if(b.hasCombatant(event.getSource().getTrueSource().getEntityId()))
+                for(Battle b : battleMap.values())
                 {
-//                    logger.debug("Attack Canceled: attacked ignores battle but attacker in battle");
+                    if(b.hasCombatant(event.getSource().getTrueSource().getEntityId()))
-                    return true;
+                    {
+    //                    logger.debug("Attack Canceled: attacked ignores battle but attacker in battle");
+                        return true;
+                    }
                 }
             }
 //            logger.debug("Attack Not Canceled: attacked ignores battle");
@@ -76,12 +75,15 @@ public class BattleManager
         if(entityInfo != null && (TurnBasedMinecraftMod.proxy.getConfig().isIgnoreBattleType(entityInfo.category) || entityInfo.ignoreBattle))
         {
             // attacker entity ignores battle
-            for(Battle b : battleMap.values())
+            synchronized(battleMap)
             {
-                if(b.hasCombatant(event.getEntity().getEntityId()))
+                for(Battle b : battleMap.values())
                 {
-//                    logger.debug("Attack Canceled: attacker ignores battle but attacked in battle");
+                    if(b.hasCombatant(event.getEntity().getEntityId()))
-                    return true;
+                    {
+    //                    logger.debug("Attack Canceled: attacker ignores battle but attacked in battle");
+                        return true;
+                    }
                 }
             }
 //            logger.debug("Attack Not Canceled: attacker ignores battle");
@@ -93,36 +95,39 @@ public class BattleManager
         Entity notInBattle = null;
         Battle battle = null;
         
-        for(Battle b : battleMap.values())
+        synchronized(battleMap)
         {
-            if(b.hasCombatant(event.getSource().getTrueSource().getEntityId()))
+            for(Battle b : battleMap.values())
             {
-                if(inBattle != null)
+                if(b.hasCombatant(event.getSource().getTrueSource().getEntityId()))
                 {
-                    // both combatants are in battle
+                    if(inBattle != null)
-//                    logger.debug("Attack Canceled: both are in battle");
+                    {
-                    return true;
+                        // both combatants are in battle
+    //                    logger.debug("Attack Canceled: both are in battle");
+                        return true;
+                    }
+                    else
+                    {
+                        inBattle = event.getSource().getTrueSource();
+                        notInBattle = event.getEntity();
+                        battle = b;
+                    }
                 }
-                else
+                if(b.hasCombatant(event.getEntity().getEntityId()))
                 {
-                    inBattle = event.getSource().getTrueSource();
+                    if(inBattle != null)
-                    notInBattle = event.getEntity();
+                    {
-                    battle = b;
+                        // both combatants are in battle
-                }
+    //                    logger.debug("Attack Canceled: both are in battle");
-            }
+                        return true;
-            if(b.hasCombatant(event.getEntity().getEntityId()))
+                    }
-            {
+                    else
-                if(inBattle != null)
+                    {
-                {
+                        inBattle = event.getEntity();
-                    // both combatants are in battle
+                        notInBattle = event.getSource().getTrueSource();
-//                    logger.debug("Attack Canceled: both are in battle");
+                        battle = b;
-                    return true;
+                    }
-                }
-                else
-                {
-                    inBattle = event.getEntity();
-                    notInBattle = event.getSource().getTrueSource();
-                    battle = b;
                 }
             }
         }
@@ -185,34 +190,37 @@ public class BattleManager
         Entity notInBattle = null;
         Battle battle = null;
         
-        for(Battle b : battleMap.values())
+        synchronized(battleMap)
         {
-            if(b.hasCombatant(event.getEntity().getEntityId()))
+            for(Battle b : battleMap.values())
             {
-                if(inBattle != null)
+                if(b.hasCombatant(event.getEntity().getEntityId()))
                 {
-                    // both entities already in battle
+                    if(inBattle != null)
-                    return;
+                    {
+                        // both entities already in battle
+                        return;
+                    }
+                    else
+                    {
+                        inBattle = event.getEntity();
+                        notInBattle = event.getTarget();
+                        battle = b;
+                    }
                 }
-                else
+                if(b.hasCombatant(event.getTarget().getEntityId()))
                 {
-                    inBattle = event.getEntity();
+                    if(inBattle != null)
-                    notInBattle = event.getTarget();
+                    {
-                    battle = b;
+                        // both entities already in battle
-                }
+                        return;
-            }
+                    }
-            if(b.hasCombatant(event.getTarget().getEntityId()))
+                    else
-            {
+                    {
-                if(inBattle != null)
+                        inBattle = event.getTarget();
-                {
+                        notInBattle = event.getEntity();
-                    // both entities already in battle
+                        battle = b;
-                    return;
+                    }
-                }
-                else
-                {
-                    inBattle = event.getTarget();
-                    notInBattle = event.getEntity();
-                    battle = b;
                 }
             }
         }
@@ -251,19 +259,26 @@ public class BattleManager
     
     private Battle createBattle(Collection<Entity> sideA, Collection<Entity> sideB)
     {
-        while(battleMap.containsKey(IDCounter))
+        Battle newBattle = null;
+        synchronized(battleMap)
         {
-            ++IDCounter;
+            while(battleMap.containsKey(IDCounter))
+            {
+                ++IDCounter;
+            }
+            newBattle = new Battle(this, IDCounter, sideA, sideB, true);
+            battleMap.put(IDCounter, newBattle);
         }
-        Battle newBattle = new Battle(this, IDCounter, sideA, sideB, true);
-        battleMap.put(IDCounter, newBattle);
         newBattle.notifyPlayersBattleInfo();
         return newBattle;
     }
     
     public Battle getBattleByID(int id)
     {
-        return battleMap.get(id);
+        synchronized(battleMap)
+        {
+            return battleMap.get(id);
+        }
     }
     
     public void cleanup()
@@ -271,7 +286,10 @@ public class BattleManager
         battleUpdater.setIsRunning(false);
         battleUpdater = null;
         updaterThread = null;
-        battleMap.clear();
+        synchronized(battleMap)
+        {
+            battleMap.clear();
+        }
     }
     
     protected void addRecentlyLeftBattle(Combatant c)
@@ -291,24 +309,20 @@ public class BattleManager
     protected void updateRecentlyLeftBattle()
     {
         long current = System.nanoTime();
-        Queue<Combatant> removeQueue = new ArrayDeque<Combatant>();
         synchronized(recentlyLeftBattle)
         {
-            for(Combatant c : recentlyLeftBattle.values())
+            for(Iterator<Map.Entry<Integer, Combatant>> iter = recentlyLeftBattle.entrySet().iterator(); iter.hasNext();)
             {
-                if(current - c.time > TurnBasedMinecraftMod.proxy.getConfig().getLeaveBattleCooldownNanos())
+                Map.Entry<Integer, Combatant> entry = iter.next();
+                if(current - entry.getValue().time > TurnBasedMinecraftMod.proxy.getConfig().getLeaveBattleCooldownNanos())
                 {
-                    removeQueue.add(c);
+                    iter.remove();
+                    if(entry.getValue().entity instanceof EntityPlayerMP)
+                    {
+                        TurnBasedMinecraftMod.NWINSTANCE.sendTo(new PacketGeneralMessage("Timer ended, you can now attack/be-attacked again."), (EntityPlayerMP)entry.getValue().entity);
+                    }
                 }
             }
-            for(Combatant c = removeQueue.poll(); c != null; c = removeQueue.poll())
-            {
-                if(c.entity instanceof EntityPlayerMP)
-                {
-                    TurnBasedMinecraftMod.NWINSTANCE.sendTo(new PacketGeneralMessage("Timer ended, you can now attack/be-attacked again."), (EntityPlayerMP)c.entity);
-                }
-                recentlyLeftBattle.remove(c.entity.getEntityId());
-            }
         }
     }
     

src/main/java/com/seodisparate/TurnBasedMinecraft/common/BattleUpdater.java

@@ -1,7 +1,7 @@
 package com.seodisparate.TurnBasedMinecraft.common;
 
-import java.util.ArrayDeque;
+import java.util.Iterator;
-import java.util.Queue;
+import java.util.Map;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 public class BattleUpdater implements Runnable
@@ -18,21 +18,19 @@ public class BattleUpdater implements Runnable
     @Override
     public void run()
     {
-        Queue<Integer> endedQueue = new ArrayDeque<Integer>();
-        Integer ended;
         while(isRunning.get())
         {
-            for(Battle e : manager.battleMap.values())
+            synchronized(manager.battleMap)
             {
-                if(e.update())
+                for(Iterator<Map.Entry<Integer, Battle>> iter = manager.battleMap.entrySet().iterator(); iter.hasNext();)
                 {
-                    endedQueue.add(e.getId());
+                    Map.Entry<Integer, Battle> entry = iter.next();
+                    if(entry.getValue().update())
+                    {
+                        iter.remove();
+                    }
                 }
             }
-            for(ended = endedQueue.poll(); ended != null; ended = endedQueue.poll())
-            {
-                manager.battleMap.remove(ended);
-            }
             manager.updateRecentlyLeftBattle();
             try { Thread.sleep(250); } catch (Throwable t) { /* ignored */ }
         }