Bloquear la pantalla del celular expulsa al jugador: los pings no cuentan como actividad

[xusuxiang8]

Closes #17

Problema

trackClientActivity retorna antes de actualizar lastActivityAt cuando el paquete es un ping. El barrido de inactividad usa lastActivityAt para decidir a quién expulsar. Como los pings no lo actualizan, un jugador que solo manda pings (pantalla bloqueada, otra app) es tratado como inactivo y desconectado a los 15 minutos.

Solución

Mover ws.lastActivityAt = now antes del early-return del ping. Así los pings mantienen viva la conexión sin necesidad de cambiar la lógica de rate limiting.

Criterios de aceptación

• Un cliente que solo manda pings no es expulsado por inactividad
• Un cliente que dejó de responder sí se limpia (no envía ningún paquete)
• Volver del segundo plano tras 15 minutos mantiene la sesión

[gitar-bot]

⚠️ Edge Case: Ping keepalive now defeats AFK idle-disconnect entirely

The frontend sends a ping every 10s via setInterval(sendPing, 10000) as an automatic keepalive, independent of any user action. By moving ws.lastActivityAt = now before the ping early-return, every connected client with a live socket refreshes lastActivityAt every 10s, so processIdleCharactersTick's idleCharacterTimeoutMs check (server/src/server.ts:735) can never fire for a normally connected client. This fixes the locked-screen case but also disables the general AFK slot-reclamation the idle timeout was built for: only clients that fully stop sending packets get cleaned. If freeing slots from truly-idle-but-connected players is still desired, gate idle detection on gameplay activity (e.g. movement/combat timestamps as getScoutIdleReferenceAt already does) rather than raw connection liveness.

_{Was this helpful? React with 👍 / 👎}

[gitar-bot]

💡 Quality: Remove leftover commented-out lastActivityAt assignment

Line 422 leaves a dead commented-out // ws.lastActivityAt = now; // moved up ... in place of the removed assignment. This is clutter that describes the diff rather than the code; delete the comment since the reasoning belongs in the commit message, not the source.

Drop the leftover commented line.:

    ws.lastPacketAt = now;
}

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

Code Review ⚠️ Changes requested 0 resolved / 2 findings

Updates client activity tracking to include ping packets, but the automated frontend keepalive causes pings to fire every 10 seconds, completely defeating the AFK idle-disconnect mechanism. Please also remove the leftover commented-out code on line 422.

⚠️ Edge Case: Ping keepalive now defeats AFK idle-disconnect entirely

📄 server/src/server.ts:367 📄 server/src/server.ts:372 📄 server/src/server.ts:373 📄 server/src/server.ts:697-711

The frontend sends a ping every 10s via setInterval(sendPing, 10000) as an automatic keepalive, independent of any user action. By moving ws.lastActivityAt = now before the ping early-return, every connected client with a live socket refreshes lastActivityAt every 10s, so processIdleCharactersTick's idleCharacterTimeoutMs check (server/src/server.ts:735) can never fire for a normally connected client. This fixes the locked-screen case but also disables the general AFK slot-reclamation the idle timeout was built for: only clients that fully stop sending packets get cleaned. If freeing slots from truly-idle-but-connected players is still desired, gate idle detection on gameplay activity (e.g. movement/combat timestamps as getScoutIdleReferenceAt already does) rather than raw connection liveness.

💡 Quality: Remove leftover commented-out lastActivityAt assignment

📄 server/src/server.ts:421-422

Line 422 leaves a dead commented-out // ws.lastActivityAt = now; // moved up ... in place of the removed assignment. This is clutter that describes the diff rather than the code; delete the comment since the reasoning belongs in the commit message, not the source.

Drop the leftover commented line.

    ws.lastPacketAt = now;
}

🤖 Prompt for agents

Code Review: Updates client activity tracking to include ping packets, but the automated frontend keepalive causes pings to fire every 10 seconds, completely defeating the AFK idle-disconnect mechanism. Please also remove the leftover commented-out code on line 422.

1. ⚠️ Edge Case: Ping keepalive now defeats AFK idle-disconnect entirely
   Files: server/src/server.ts:367, server/src/server.ts:372, server/src/server.ts:373, server/src/server.ts:697-711

   The frontend sends a ping every 10s via setInterval(sendPing, 10000) as an automatic keepalive, independent of any user action. By moving ws.lastActivityAt = now before the ping early-return, every connected client with a live socket refreshes lastActivityAt every 10s, so processIdleCharactersTick's idleCharacterTimeoutMs check (server/src/server.ts:735) can never fire for a normally connected client. This fixes the locked-screen case but also disables the general AFK slot-reclamation the idle timeout was built for: only clients that fully stop sending packets get cleaned. If freeing slots from truly-idle-but-connected players is still desired, gate idle detection on gameplay activity (e.g. movement/combat timestamps as getScoutIdleReferenceAt already does) rather than raw connection liveness.

2. 💡 Quality: Remove leftover commented-out lastActivityAt assignment
   Files: server/src/server.ts:421-422

   Line 422 leaves a dead commented-out `// ws.lastActivityAt = now;  // moved up ...` in place of the removed assignment. This is clutter that describes the diff rather than the code; delete the comment since the reasoning belongs in the commit message, not the source.

   Fix (Drop the leftover commented line.):
       ws.lastPacketAt = now;
   }

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}