From 44ebe11030d1f2c6faf5cf57b83e950318893500 Mon Sep 17 00:00:00 2001
From: Christian Heller <c.heller@plomlompom.de>
Date: Wed, 16 Dec 2020 17:48:11 +0100
Subject: [PATCH] To avoid login/logout race conditions, move login into
 Game.run_tick.

---
 plomrogue/commands.py | 18 +-----------------
 plomrogue/game.py     | 32 +++++++++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/plomrogue/commands.py b/plomrogue/commands.py
index 49312ee..653fc74 100644
--- a/plomrogue/commands.py
+++ b/plomrogue/commands.py
@@ -60,25 +60,9 @@ def cmd_LOGIN(game, nick, connection_id):
     nick = nick.strip()
     if len(nick) == 0:
         raise GameError('empty name')
-    for t in [t for t in game.things if t.type_ == 'Player' and t.name == nick]:
-        raise GameError('name already in use')
     if game.get_player(connection_id):
         raise GameError('cannot log in twice')
-    t = game.add_thing('Player', game.spawn_point)
-    t.name = nick
-    t.thing_char = game.get_next_player_char()
-    game.sessions[connection_id] = {
-        'thing_id': t.id_,
-        'status': 'player'
-    }
-    game.io.send('PLAYER_ID %s' % t.id_, connection_id)
-    game.io.send('LOGIN_OK', connection_id)
-    game.io.send('CHAT ' + quote(t.name + ' entered the map.'))
-    for s in [s for s in game.things
-              if s.type_ == 'SpawnPoint' and s.name == t.name]:
-        t.position = s.position
-        break
-    # game.changed = True  # handled by game.add_thing
+    game.login_requests += [(nick, connection_id)]
 cmd_LOGIN.argtypes = 'string'
 
 def cmd_BECOME_ADMIN(game, password, connection_id):
diff --git a/plomrogue/game.py b/plomrogue/game.py
index 202a0fc..ec663b7 100755
--- a/plomrogue/game.py
+++ b/plomrogue/game.py
@@ -120,6 +120,7 @@ class Game(GameBase):
         self.changed = True
         self.changed_tiles = {'fov': [], 'other': []}
         self.io = GameIO(self, save_file)
+        self.login_requests = []
         self.tasks = {}
         self.thing_types = {}
         self.sessions = {}
@@ -328,7 +329,30 @@ class Game(GameBase):
                                                                       little_yx)]
         self.changed = True
 
+    def login(self, nick, connection_id):
+        for t in [t for t in self.things
+                  if t.type_ == 'Player' and t.name == nick]:
+            self.io.send('GAME_ERROR ' + quote('name already in use'),
+                         connection_id)
+            return
+        t = self.add_thing('Player', self.spawn_point)
+        t.name = nick
+        t.thing_char = self.get_next_player_char()
+        self.sessions[connection_id] = {
+            'thing_id': t.id_,
+            'status': 'player'
+        }
+        self.io.send('PLAYER_ID %s' % t.id_, connection_id)
+        self.io.send('LOGIN_OK', connection_id)
+        self.io.send('CHAT ' + quote(t.name + ' entered the map.'))
+        for s in [s for s in self.things
+                  if s.type_ == 'SpawnPoint' and s.name == t.name]:
+            t.position = s.position
+            break
+
     def run_tick(self):
+
+        # update player sessions
         to_delete = []
         for connection_id in self.sessions:
             connection_id_found = False
@@ -344,7 +368,11 @@ class Game(GameBase):
                 to_delete += [connection_id]
         for connection_id in to_delete:
             del self.sessions[connection_id]
-            # self.changed = True  already handled by remove_thing
+        while len(self.login_requests) > 0:
+            login_request = self.login_requests.pop()
+            self.login(login_request[0], login_request[1])
+
+        # update game state
         for t in [t for t in self.things]:
             if t in self.things:
                 try:
@@ -357,6 +385,8 @@ class Game(GameBase):
                     for connection_id in [c_id for c_id in self.sessions
                                           if self.sessions[c_id]['thing_id'] == t.id_]:
                         self.io.send('PLAY_ERROR ' + quote(str(e)), connection_id)
+
+        # send gamestate if it makes sense at this point
         if self.changed:
             self.turn += 1
             # send_gamestate() can be rather expensive, due to among other reasons
-- 
2.30.2