fix(gamestore): only remove inactive games and notify the chat

blackjack/game/blackjackgame.py

@@ -19,6 +19,7 @@ def __init__(self, gametype=None, game_id=None, lang_id="en"):
         self.list_tie = []
         self.list_lost = []
         self.datetime_started = datetime.now()
+        self.last_activity = self.datetime_started
         self.bets_active = True
         self._current_player = 0
         self.players = []
@@ -65,6 +66,10 @@ def _run_handlers(self, handlers):
             except Exception as e:
                 self.logger.error("Couldn't run handler '{0}' - The following exception occurred: '{1}'".format(handler, e))
 
+    def _update_last_activity(self):
+        """Remembers the current time as the last time something happened in this game"""
+        self.last_activity = datetime.now()
+
     def start(self, user_id):
         """
         Sets up the players' and the dealer's hands
@@ -81,6 +86,7 @@ def start(self, user_id):
             raise errors.InsufficientPermissionsException
 
         self.running = True
+        self._update_last_activity()
 
         # Give every player and the dealer 2 cards
         for player in (self.players + [self.dealer]) * 2:
@@ -122,6 +128,7 @@ def add_player(self, user_id, first_name):
         player = Player(user_id, first_name)
         self.logger.debug("Adding new player: {}!".format(player))
         self.players.append(player)
+        self._update_last_activity()
 
         if self.type == BlackJackGame.Type.SINGLEPLAYER:
             self.logger.debug("Starting game now, because it's a singleplayer game")
@@ -139,6 +146,7 @@ def draw_card(self):
         card = self.deck.pick_one_card()
 
         player.give_card(card)
+        self._update_last_activity()
 
         if player.cardvalue > 21:
             self.logger.debug("While giving user {} the card {}, they busted.".format(player.first_name, card))
@@ -154,6 +162,8 @@ def next_player(self):
         if not self.running:
             raise errors.GameNotRunningException("The game must be started before it's the next player's turn")
 
+        self._update_last_activity()
+
         if self._current_player >= len(self.players) - 1:
             self.logger.debug("Next player is dealer!")
             self._current_player = -1

blackjackbot/gamestore.py

@@ -7,6 +7,9 @@
 import database.statistics
 
 
+STALE_TIMEOUT_MIN = 10
+
+
 class GameStore(object):
     _instance = None
     _initialized = False
@@ -84,19 +87,22 @@ def _game_stopped_callback(self, game):
 
         self.logger.debug("Current games: {}".format(len(self._chat_dict)))
 
-    def cleanup_stale_games(self):
-        stale_timeout_min = 10
+    def cleanup_stale_games(self, stale_timeout_min=STALE_TIMEOUT_MIN):
+        """
+        Removes all games in which nothing happened for the given amount of minutes
+        :param stale_timeout_min: Amount of minutes without activity after which a game is considered stale
+        :return: List of chat_ids whose games were removed
+        """
         now = datetime.now()
         remove_chat_ids = []
 
-        for game_id, chat_id in self._game_dict.items():
-            game = self.get_game(chat_id)
-
-            game_older_than_10_mins = game.datetime_started < (now - timedelta(minutes=stale_timeout_min))
-            if game_older_than_10_mins:
-                logging.info("Killing game with id {} because it's stale for > {} mins".format(game.id, stale_timeout_min))
-                # TODO notify chat
+        # Copy the items, because this runs in the job queue thread while handlers might add or remove games
+        for chat_id, game in list(self._chat_dict.items()):
+            if game.last_activity < (now - timedelta(minutes=stale_timeout_min)):
+                self.logger.info("Killing game with id {} because it's stale for > {} mins".format(game.id, stale_timeout_min))
                 remove_chat_ids.append(chat_id)
 
         for chat_id in remove_chat_ids:
             self.remove_game(chat_id)
+
+        return remove_chat_ids

blackjackbot/lang/strings/translations_de.json

@@ -27,6 +27,7 @@
   "mp_not_your_turn_callback": "Du bist nicht am Zug, {}!",
   "game_starts_now": "Das Spiel startet jetzt. \uD83C\uDFB2\n\n{}Karten des Dealers:\n\n{}",
   "game_ended": "Das Spiel wurde beendet! \uD83D\uDE09 Mit /start kannst du ein neues starten!",
+  "game_ended_inactive": "\u23F1 Das Spiel wurde beendet, weil {} Minuten lang niemand gespielt hat. Mit /start kannst du ein neues starten!",
   "mp_only_creator_can_end": "Nur der Ersteller kann das Spiel beenden!",
   "mp_players_are": "Spieler sind:\n",
   "no_permission": "Du hast dafür keine Berechtigungen!",

blackjackbot/lang/strings/translations_en.json

@@ -27,6 +27,7 @@
   "mp_not_your_turn_callback": "It's not your turn, {}!",
   "game_starts_now": "The game starts now. \uD83C\uDFB2\n\n{}Dealer's cards:\n\n{}",
   "game_ended": "The game has been ended. \uD83D\uDE09 You can /start another one if you like to.",
+  "game_ended_inactive": "\u23F1 The game has been ended because nobody played for {} minutes. You can /start another one if you like to.",
   "mp_only_creator_can_end": "Only the creator can end the game!",
   "mp_players_are": "Players are:\n",
   "no_permission": "You don't have the required permissions to do that!",

blackjackbot/tests/__init__.py

@@ -0,0 +1 @@
+# -*- coding: utf-8 -*-

blackjackbot/tests/gamestore_test.py

@@ -0,0 +1,47 @@
+# -*- coding: utf-8 -*-
+import unittest
+from datetime import datetime, timedelta
+
+from blackjack.game import BlackJackGame
+from blackjackbot.gamestore import GameStore
+
+
+class GameStoreTest(unittest.TestCase):
+
+    def setUp(self):
+        self.game_store = GameStore()
+
+    def _add_game(self, chat_id):
+        game = BlackJackGame(gametype=BlackJackGame.Type.MULTIPLAYER_GROUP)
+        game.add_player(1, "A")
+        game.add_player(2, "B")
+        self.game_store.add_game(chat_id, game)
+        self.addCleanup(self.game_store.remove_game, chat_id)
+        return game
+
+    def test_cleanup_removes_inactive_games(self):
+        """Games without activity for longer than the timeout must be removed"""
+        game = self._add_game(-1001)
+        game.last_activity = datetime.now() - timedelta(minutes=11)
+
+        removed = self.game_store.cleanup_stale_games(stale_timeout_min=10)
+
+        self.assertEqual([-1001], removed)
+        self.assertFalse(self.game_store.has_game(-1001))
+
+    def test_cleanup_keeps_old_but_active_games(self):
+        """Games that were created long ago but are still being played must not be removed"""
+        game = self._add_game(-1002)
+        game.datetime_started = datetime.now() - timedelta(minutes=30)
+        game.last_activity = datetime.now() - timedelta(minutes=30)
+        game.start(1)
+        game.next_player()
+
+        removed = self.game_store.cleanup_stale_games(stale_timeout_min=10)
+
+        self.assertEqual([], removed)
+        self.assertTrue(self.game_store.has_game(-1002))
+
+
+if __name__ == '__main__':
+    unittest.main()

bot.py

@@ -8,7 +8,9 @@
 
 import config
 from blackjackbot import handlers, error_handler
-from blackjackbot.gamestore import GameStore
+from blackjackbot.gamestore import GameStore, STALE_TIMEOUT_MIN
+from blackjackbot.lang import translate
+from database import Database
 
 logdir_path = pathlib.Path(__file__).parent.joinpath("logs").absolute()
 logfile_path = logdir_path.joinpath("bot.log")
@@ -37,7 +39,14 @@
 # Set up jobs
 def stale_game_cleaner(context):
     gs = GameStore()
-    gs.cleanup_stale_games()
+    removed_chat_ids = gs.cleanup_stale_games()
+
+    for chat_id in removed_chat_ids:
+        text = translate("game_ended_inactive", Database().get_lang_id(chat_id)).format(STALE_TIMEOUT_MIN)
+        try:
+            context.bot.send_message(chat_id=chat_id, text=text)
+        except Exception as e:
+            logger.warning("Couldn't notify chat {} about its stale game: {}".format(chat_id, e))
 
 
 updater.job_queue.run_repeating(callback=stale_game_cleaner, interval=300, first=300)