feat: implement asynchronous bot actions with improved combat AI and real-time game state updates

src/client/src/modules/game/GameView.tsx

@@ -58,6 +58,7 @@ interface GameViewProps {
 }
 
 export const GameView: React.FC<GameViewProps> = ({ gameState, currentPlayerId }) => {
+  const hasPriority = gameState.priorityPlayerId === currentPlayerId;
   // Assuming useGameSocket is a custom hook that provides game state and player info
   // This line was added based on the provided snippet, assuming it's part of the intended context.
   // If useGameSocket is not defined elsewhere, this will cause an error.
@@ -97,15 +98,114 @@ export const GameView: React.FC<GameViewProps> = ({ gameState, currentPlayerId }
     }
   }, [isYielding, gameState.priorityPlayerId, gameState.step, currentPlayerId]);
 
-  // Reset Yield on Turn Change
+  // Auto-Yield Logic (Smart Yield against Bot)
+  const activePlayer = gameState.activePlayerId ? gameState.players[gameState.activePlayerId] : undefined;
+  const isBotTurn = activePlayer?.isBot;
+
+  // Ref to track turn changes to force reset yield
+  const prevActivePlayerId = useRef(gameState.activePlayerId);
+
   useEffect(() => {
-    // If turn changes or phase changes significantly? F4 is until EOT.
-    // We can reset if it's my turn again? Or just let user toggle.
-    // Strict F4 resets at cleanup.
-    if (gameState.step === 'cleanup') {
+    // 1. Detect Turn Change (Active Player changed)
+    if (prevActivePlayerId.current !== gameState.activePlayerId) {
+      // Reset yield strictly on any turn change to prevent leakage
       setIsYielding(false);
+      prevActivePlayerId.current = gameState.activePlayerId;
+      // Logic continues below to re-enable if it IS a bot turn...
     }
-  }, [gameState.step]);
+
+    if (isBotTurn) {
+      // Enforce Yielding during Bot turn (except combat decisions)
+      if (['declare_attackers', 'declare_blockers'].includes(gameState.step || '')) {
+        // Must pause yield for decisions
+        setIsYielding(false);
+        // Note: We don't check !isYielding here because if it was true, we want false. 
+        // If it was false, we keep false. 
+        // But we can't condtionally call setIsYielding based on isYielding inside effect IF the effect has isYielding dependency...
+        // Wait, if I set it to false, isYielding changes -> effect runs again.
+        // It enters here. isYielding is false. checks... checks...
+        // It hit this block. We want it false. It is false. No-op. Stable.
+      } else {
+        // Standard Bot Phase -> Yield
+        // However, we must be careful not to infinite loop if we already set it.
+        // BUT, we just set it to false above if turn changed! 
+        // So for Bot Turn start: 
+        // 1. Turn Change detected -> isYielding = false.
+        // 2. isBotTurn is true.
+        // 3. We enter here. We want isYielding = true.
+        // 4. setIsYielding(true).
+        // 5. Render. Effect runs again.
+        // 6. Turn Change NOT detected. matches.
+        // 7. isBotTurn true.
+        // 8. checks isYielding (true). No-op. Stable.
+
+        // One issue: if I manually "Stop Yielding" during bot turn (e.g. to inspect), 
+        // this logic will FORCE it back on.
+        // That is acceptable for "Auto-Yield vs Bot". Bot turn = You don't play.
+        // If you want to Inspect, inspecting doesn't require priority.
+        // If you want to STOP the game to do something... well, you don't have priority anyway.
+        // So forcing Yield on Bot turn is correct per requirements.
+
+        // Only verify we don't set it if it's already true
+        setIsYielding(prev => {
+          if (prev) return prev; // already true
+          return true;
+        });
+      }
+    } else {
+      // Human Turn
+      // We do NOTHING here?
+      // If we reset on turn change, then it starts false.
+      // If I manually enable it, isBotTurn is false. We simply don't interfere.
+      // This allows accurate Manual Yielding!
+    }
+  }, [gameState.activePlayerId, gameState.step, isBotTurn]); // Removed isYielding dependency to avoid loops?
+  // If I access `isYielding` inside `setIsYielding`, I don't need it in dependency.
+  // But wait, the `if (['declare_...'].includes)` logic needs to potentially set it false.
+  // setIsYielding(false) is fine.
+  // The logic seems sound without isYielding in dependency, assuming stable behavior.
+  // BUT: `prevActivePlayerId` check needs stable run.
+
+  // Let's keep `isYielding` in deps if I read it? 
+  // I replaced reads with functional updates or just strict sets?
+  // "if (isYielding) setIsYielding(false)" -> simply "setIsYielding(false)" is safer/idempotent if efficient.
+  // React state updates bail out if same value.
+
+  // BUT wait, if I remove `isYielding` from deps, and I toggle it manually...
+  // The effect WON'T run.
+  // If I toggle manual yield on My Turn. Effect doesn't run. Correct (we don't want it to interfere).
+
+  // If I am in Bot Turn, and I somehow toggle it false? (Button click).
+  // Effect doesn't run. isYielding stays false.
+  // Bot waits forever? No, bot logic (server) continues.
+  // Client just doesn't auto-pass.
+  // But we WANT to force it.
+  // So we SHOULD depend on `isYielding` so if user turns it off, we force it back on?
+  // No, if user turns it off, maybe they want to see?
+  // User Requirement: "Auto-Yield".
+  // If I rely on `isYielding` in deps, I risk the loop again.
+  // Let's try WITHOUT `isYielding` in deps first.
+  // This means if I turn it off, it stays off until next step change or phase change.
+  // Because `gameState.step` IS in deps.
+  // So if step advances, effect runs -> sees Bot Turn -> Forces True.
+  // This is good behavior. Re-enables each step.
+
+  // So, removing isYielding from dependencies seems robust.
+
+  // One catch: `['declare_attackers']`.
+  // If step is declare_attackers. Effect runs.
+  // Forces isYielding(false).
+  // User manually clicks yield (true).
+  // Effect doesn't run.
+  // isYielding stays true.
+  // This might be BAD if we want to enforce safety?
+  // If user yields in declare_attackers... they yield their chance to attack.
+  // That's on them.
+  // But the "Flickering" issue was us fighting them.
+  // If we don't react to isYielding change, we won't fight.
+  // This solves flickering definitively too.
+
+  // Final Plan: Use the logic block above, remove isYielding from deps.
 
   // --- Combat State ---
   const [proposedAttackers, setProposedAttackers] = useState<Set<string>>(new Set());
@@ -324,6 +424,8 @@ export const GameView: React.FC<GameViewProps> = ({ gameState, currentPlayerId }
     const card = gameState.cards[cardId];
     if (!card) return;
 
+    if (!hasPriority) return; // Strict Lock on executing drops
+
     // --- Drop on Zone ---
     if (over.data.current?.type === 'zone') {
       const zoneName = over.id as string;
@@ -593,18 +695,21 @@ export const GameView: React.FC<GameViewProps> = ({ gameState, currentPlayerId }
                             boxShadow: isAttacking ? '0 20px 40px -10px rgba(239, 68, 68, 0.5)' : 'none'
                           }}
                         >
-                          <DraggableCardWrapper card={card}>
+                          <DraggableCardWrapper card={card} disabled={!hasPriority}>
                             <CardComponent
                               card={card}
                               viewMode="cutout"
                               onDragStart={() => { }}
                               onClick={(id) => {
                                 if (gameState.step === 'declare_attackers') {
+                                  // Attack declaration is special: It happens during the "Pause" where AP has priority but isn't passing yet.
+                                  // We allow toggling attackers if it's our turn to attack.
+                                  if (gameState.activePlayerId !== currentPlayerId) return;
+
                                   // Validate Creature Type
                                   const types = card.types || [];
                                   const typeLine = card.typeLine || '';
                                   if (!types.includes('Creature') && !typeLine.includes('Creature')) {
-                                    // Optional: Shake effect or visual feedback that it's invalid
                                     return;
                                   }
 
@@ -612,7 +717,46 @@ export const GameView: React.FC<GameViewProps> = ({ gameState, currentPlayerId }
                                   if (newSet.has(id)) newSet.delete(id);
                                   else newSet.add(id);
                                   setProposedAttackers(newSet);
+                                } else if (gameState.step === 'declare_blockers') {
+                                  // BLOCKING LOGIC
+                                  // Only Defending Player (NOT active player) can declare blockers
+                                  if (gameState.activePlayerId === currentPlayerId) return;
+
+                                  // Check eligibility (Untapped Creature)
+                                  if (card.tapped) return;
+                                  const types = card.types || [];
+                                  if (!types.includes('Creature') && !card.typeLine?.includes('Creature')) return;
+
+                                  // Find all Valid Attackers
+                                  // Attackers are cards in opponent's control that are marked 'attacking'
+                                  const attackers = Object.values(gameState.cards).filter(c =>
+                                    c.controllerId !== currentPlayerId && c.attacking
+                                  );
+
+                                  if (attackers.length === 0) return; // Nothing to block
+
+                                  const currentTargetId = proposedBlockers.get(id);
+                                  const newMap = new Map(proposedBlockers);
+
+                                  if (!currentTargetId) {
+                                    // Not currently blocking -> Block the first attacker
+                                    newMap.set(id, attackers[0].instanceId);
                                   } else {
+                                    // Currently blocking -> Cycle to next attacker OR unblock if at end of list
+                                    const currentIndex = attackers.findIndex(a => a.instanceId === currentTargetId);
+                                    if (currentIndex === -1 || currentIndex === attackers.length - 1) {
+                                      // Was blocking last one (or invalid), so Unblock
+                                      newMap.delete(id);
+                                    } else {
+                                      // Cycle to next
+                                      newMap.set(id, attackers[currentIndex + 1].instanceId);
+                                    }
+                                  }
+                                  setProposedBlockers(newMap);
+
+                                } else {
+                                  // Regular Tap (Mana/Ability)
+                                  if (!hasPriority) return;
                                   toggleTap(id);
                                 }
                               }}
@@ -772,7 +916,7 @@ export const GameView: React.FC<GameViewProps> = ({ gameState, currentPlayerId }
                         zIndex: index
                       }}
                     >
-                      <DraggableCardWrapper card={card}>
+                      <DraggableCardWrapper card={card} disabled={!hasPriority}>
                         <CardComponent
                           card={card}
                           viewMode="normal"

src/client/src/modules/game/PhaseStrip.tsx

@@ -47,7 +47,7 @@ export const PhaseStrip: React.FC<PhaseStripProps> = ({
 
     if (currentStep === 'declare_attackers') {
       if (gameState.attackersDeclared) {
-        actionLabel = "Confirm (Blockers)";
+        actionLabel = "To Blockers";
         actionType = 'PASS_PRIORITY';
       } else {
         const count = contextData?.attackers?.length || 0;
@@ -63,10 +63,24 @@ export const PhaseStrip: React.FC<PhaseStripProps> = ({
         }
       }
     } else if (currentStep === 'declare_blockers') {
+      // If it's MY turn (Active Player), I should NEVER verify blocks myself?
+      // Actually Rules say AP gets priority after blocks.
+      // So if I have priority, it MUST mean blocks are done (or I'm waiting for them, but then I wouldn't have priority?)
+      // Wait, if I am AP, and I have priority in this step, it means blocks are implicitly done (flag should be true).
+      // Fallback: If I am Active Player, always show "To Damage".
+
+      const showToDamage = gameState.blockersDeclared || isMyTurn; // UI Safety for AP
+
+      if (showToDamage) {
+        actionLabel = "To Damage";
+        actionType = 'PASS_PRIORITY';
+        ActionIcon = Swords;
+      } else {
         actionLabel = "Confirm Blocks";
         actionType = 'DECLARE_BLOCKERS';
         ActionIcon = Shield;
         actionColor = "bg-blue-600 hover:bg-blue-500 shadow-[0_0_10px_rgba(37,99,235,0.4)]";
+      }
     } else if (isStackEmpty) {
       // Standard Pass
       actionType = 'PASS_PRIORITY';

src/client/src/types/game.ts

@@ -67,6 +67,7 @@ export interface PlayerState {
   manaPool?: Record<string, number>;
   handKept?: boolean;
   mulliganCount?: number;
+  isBot?: boolean;
 }
 
 export interface GameState {

src/server/game/RulesEngine.ts

@@ -163,7 +163,9 @@ export class RulesEngine {
     if (this.state.phase !== 'combat' || this.state.step !== 'declare_blockers') throw new Error("Not Declare Blockers step.");
     if (this.state.activePlayerId === playerId) throw new Error("Active Player cannot declare blockers.");
 
-    blockers.forEach(({ blockerId, attackerId }) => {
+    // Safe handling if blockers is undefined
+    const declaredBlockers = blockers || [];
+    declaredBlockers.forEach(({ blockerId, attackerId }) => {
       const blocker = this.state.cards[blockerId];
       const attacker = this.state.cards[attackerId];
 
@@ -178,7 +180,9 @@ export class RulesEngine {
       // Note: 509.2. Damage Assignment Order (if multiple blockers)
     });
 
-    console.log(`Player ${playerId} declared ${blockers.length} blockers.`);
+    console.log(`Player ${playerId} declared ${declaredBlockers.length} blockers.`);
+
+    this.state.blockersDeclared = true; // Fix: Ensure state reflects blockers were declared
 
     // Priority goes to Active Player first after blockers declared
     this.resetPriority(this.state.activePlayerId);

src/server/index.ts

@@ -38,27 +38,14 @@ const persistenceManager = new PersistenceManager(roomManager, draftManager, gam
 // Game Over Listener
 gameManager.on('game_over', ({ gameId, winnerId }) => {
   console.log(`[Index] Game Over received: ${gameId}, Winner: ${winnerId}`);
-  // Find tournament by Room? We need a way to map matchId -> roomId?
-  // Or matchId is unique enough? 
-  // Wait, I used gameId = matchId for 1v1. 
+  // ... existing logic ...
+});
 
-  // Iterate all tournaments to find the match? Inefficient but works.
-  // Ideally we track mapping.
-  // For now, let's assume we can find it.
-
-  // TODO: Optimise lookup
-  // Actually, RoomManager knows the tournament.
-  // We can scan rooms?
-  // Let's implement recordMatchResult that searches if needed, or pass roomId in event?
-  // checkWinCondition passes roomId as gameId... 
-  // Ah, 1v1 match gameId will be the matchId (e.g. "r1-m0"). 
-  // We need the RoomId too.
-
-  // Let's pass roomId in metadata to createGame? 
-  // For now, checkWinCondition(game, gameId).
-
-  // Hack: We iterate rooms to find the tournament that contains this matchId.
-  // TODO: Fix efficiency
+// Game Update Listener (For async bot actions)
+gameManager.on('game_update', (roomId, game) => {
+  if (game && roomId) {
+    io.to(roomId).emit('game_update', game);
+  }
 });
 
 // Load previous state

src/server/managers/GameManager.ts

@@ -4,6 +4,9 @@ import { RulesEngine } from '../game/RulesEngine';
 
 import { EventEmitter } from 'events';
 
+// Augment EventEmitter to type the emit event if we could, but for now standard.
+// We expect SocketService to listen to 'game_update' from GameManager.
+
 export class GameManager extends EventEmitter {
   public games: Map<string, StrictGameState> = new Map();
 
@@ -56,19 +59,15 @@ export class GameManager extends EventEmitter {
     return gameState;
   }
 
+  // Track rooms where a bot is currently "thinking" to avoid double-queuing
+  private thinkingRooms: Set<string> = new Set();
+
   // Helper to trigger bot actions if game is stuck or just started
   public triggerBotCheck(roomId: string): StrictGameState | null {
     const game = this.games.get(roomId);
     if (!game) return null;
 
-    const MAX_LOOPS = 50;
-    let loops = 0;
-    // Iterate if current priority player is bot, OR if we are in Mulligan and ANY bot needs to act?
-    // My processBotActions handles priorityPlayerId.
-    // In Mulligan, does priorityPlayerId matter?
-    // RulesEngine: resolveMulligan checks playerId.
-    // We should iterate ALL bots in mulligan phase.
-
+    // specific hack for Mulligan phase synchronization
     if (game.step === 'mulligan') {
       Object.values(game.players).forEach(p => {
         if (p.isBot && !p.handKept) {
@@ -76,13 +75,37 @@ export class GameManager extends EventEmitter {
           try { engine.resolveMulligan(p.id, true, []); } catch (e) { }
         }
       });
-      // After mulligan, game might auto-advance.
+      // If bots acted in mulligan, we might need to verify if game advances.
+      // But for Mulligan, we don't need delays as much because it's a hidden phase usually.
+      // Let's keep mulligan instant for simplicity, or we can delay it too?
+      // Let's keep instant for mulligan to "Start Game" faster.
     }
 
-    while (game.players[game.priorityPlayerId]?.isBot && loops < MAX_LOOPS) {
-      loops++;
+    const priorityId = game.priorityPlayerId;
+    const priorityPlayer = game.players[priorityId];
+
+    // If it is a Bot's turn to have priority, and we aren't already processing
+    if (priorityPlayer?.isBot && !this.thinkingRooms.has(roomId)) {
+      console.log(`[Bot Loop] Bot ${priorityPlayer.name} is thinking...`);
+      this.thinkingRooms.add(roomId);
+
+      setTimeout(() => {
+        this.thinkingRooms.delete(roomId);
         this.processBotActions(game);
+        // After processing one action, we trigger check again to see if we need to do more (e.g. Pass -> Pass -> My Turn)
+        // But we need to emit the update first! 
+        // processBotActions actually mutates state. 
+        // We should ideally emit 'game_update' here if we were outside the main socket loop.
+        // Since GameManager doesn't have the SocketService instance directly usually, 
+        // strictly speaking we need to rely on the caller to emit, OR GameManager should emit.
+        // GameManager extends EventEmitter. We can emit 'state_change'.
+        this.emit('game_update', roomId, game); // Force emit update
+
+        // Recursive check (will trigger next timeout if still bot's turn)
+        this.triggerBotCheck(roomId);
+      }, 1000);
     }
+
     return game;
   }
 
@@ -149,30 +172,8 @@ export class GameManager extends EventEmitter {
       return null;
     }
 
-    // Bot Cycle: If priority passed to a bot, or it's a bot's turn to act
-    const MAX_LOOPS = 50;
-    let loops = 0;
-
-    // Special Bot Handling for Mulligan (Simultaneous actions allowed, or strict priority ignored by bots)
-    if (game.step === 'mulligan') {
-      console.log(`[GameManager] Checking Bot Mulligans for ${game.roomId}`);
-      Object.values(game.players).forEach(p => {
-        if (p.isBot && !p.handKept) {
-          console.log(`[GameManager] Forcing Bot ${p.name} to keep hand.`);
-          try {
-            // Bots always keep for now
-            engine.resolveMulligan(p.id, true, []);
-          } catch (e) {
-            console.warn(`[Bot Mulligan Error] ${p.name}:`, e);
-          }
-        }
-      });
-    }
-
-    while (game.players[game.priorityPlayerId]?.isBot && loops < MAX_LOOPS) {
-      loops++;
-      this.processBotActions(game);
-    }
+    // Bot Cycle: Trigger Async Check (Instead of synchronous loop)
+    this.triggerBotCheck(roomId);
 
     // Check Win Condition
     this.checkWinCondition(game, roomId);
@@ -253,12 +254,18 @@ export class GameManager extends EventEmitter {
         c.controllerId === botId &&
         c.zone === 'battlefield' &&
         c.types.includes('Creature') &&
-        !c.tapped
+        !c.tapped &&
+        !c.keywords.includes('Defender') // Simple check
       );
+
       const opponents = game.turnOrder.filter(pid => pid !== botId);
       const targetId = opponents[0];
 
+      // Simple Heuristic: Attack with everything if we have profitable attacks?
+      // For now: Attack with everything that isn't summon sick or defender.
       if (attackers.length > 0 && targetId) {
+        // Randomly decide to attack to simulate "thinking" or non-suicidal behavior? 
+        // For MVP: Aggro Bot - always attacks.
         const declaration = attackers.map(c => ({ attackerId: c.instanceId, targetId }));
         console.log(`[Bot AI] ${bot.name} attacks with ${attackers.length} creatures.`);
         try { engine.declareAttackers(botId, declaration); } catch (e) { }
@@ -270,8 +277,63 @@ export class GameManager extends EventEmitter {
       }
     }
 
-    // 6. Default: Pass Priority
-    try { engine.passPriority(botId); } catch (e) { console.warn("Bot failed to pass priority", e); }
+    // 5. Combat: Declare Blockers (Defending Player)
+    if (game.step === 'declare_blockers' && game.activePlayerId !== botId && !game.blockersDeclared) {
+      // Identify attackers attacking ME
+      const attackers = Object.values(game.cards).filter(c => c.attacking === botId);
+
+      if (attackers.length > 0) {
+        // Identify my blockers
+        const blockers = Object.values(game.cards).filter(c =>
+          c.controllerId === botId &&
+          c.zone === 'battlefield' &&
+          c.types.includes('Creature') &&
+          !c.tapped
+        );
+
+        // Simple Heuristic: Block 1-to-1 if possible, just to stop damage.
+        // Don't double block.
+        const declaration: { blockerId: string, attackerId: string }[] = [];
+
+        blockers.forEach((blocker, idx) => {
+          if (idx < attackers.length) {
+            declaration.push({ blockerId: blocker.instanceId, attackerId: attackers[idx].instanceId });
+          }
+        });
+
+        if (declaration.length > 0) {
+          console.log(`[Bot AI] ${bot.name} declares ${declaration.length} blockers.`);
+          try { engine.declareBlockers(botId, declaration); } catch (e) { }
+          return;
+        }
+      }
+
+      // Default: No blocks
+      console.log(`[Bot AI] ${bot.name} declares no blockers.`);
+      try { engine.declareBlockers(botId, []); } catch (e) { }
+      return;
+    }
+
+    // 6. End Step / Cleanup -> Pass
+    if (game.phase === 'ending') {
+      try { engine.passPriority(botId); } catch (e) { }
+      return;
+    }
+
+    // 7. Default: Pass Priority (Catch-all for response windows, or empty stack)
+    // Add artificial delay logic here? Use setTimeout? 
+    // We can't easily wait in this synchronous loop. The loop relies on state updating.
+    // If we want delay, we should likely return from the loop and use `setTimeout` to call `triggerBotCheck` again?
+    // But `handleStrictAction` expects immediate return.
+    // Ideally, the BOT actions should happen asynchronously if we want delay.
+    // For now, we accept instant-speed bots.
+
+    // console.log(`[Bot AI] ${bot.name} passes priority.`);
+    try { engine.passPriority(botId); } catch (e) {
+      console.warn("Bot failed to pass priority", e);
+      // Force break loop if we are stuck?
+      // RulesEngine.passPriority usually always succeeds if it's your turn.
+    }
   }
 
 

src/server/managers/TournamentManager.ts

@@ -41,7 +41,7 @@ export class TournamentManager extends EventEmitter {
     // Calc next power of 2
     const total = shuffled.length;
     const size = Math.pow(2, Math.ceil(Math.log2(total)));
-    const byes = size - total;
+    // const byes = size - total;
 
     // Distribute byes? Simple method: Add "Bye" players, then resolved them immediately.
     // Actually, let's keep it robust.