Fix igloo owner rejoining its open igloo after disconnecting

[brosquinha]

Context

I've observed the following scenario when joining player's igloos:

• Player A goes to its igloo and makes it public;
• Player B joins Player A's igloo;
• Player A disconnects from the game;
• Player A reconnects and goes to its own igloo again;

Both players should now be on Player A's igloo, but they will not see each other. Additionally, whoever leaves the igloo last will have its game crash (i.e., will be stuck on a loading screen indefinitely), raising the following exception:

Traceback (most recent call last):
  File "/usr/src/houdini/houdini/spheniscidae.py", line 174, in run
    await self.__data_received(data)
  File "/usr/src/houdini/houdini/spheniscidae.py", line 161, in __data_received
    await self.__handle_xt_data(data)
  File "/usr/src/houdini/houdini/spheniscidae.py", line 105, in __handle_xt_data
    await listener(self, packet_data)
  File "/usr/src/houdini/houdini/handlers/__init__.py", line 91, in __call__
    await super().__call__(p, packet_data)
  File "/usr/src/houdini/houdini/converters.py", line 139, in __call__
    raise e
  File "/usr/src/houdini/houdini/converters.py", line 131, in __call__
    return await self.callback(*handler_call_arguments, **handler_call_keywords)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/houdini/houdini/handlers/play/navigation.py", line 136, in handle_join_room
    await p.join_room(room)
  File "/usr/src/houdini/houdini/penguin.py", line 88, in join_room
    await room.add_penguin(self)
  File "/usr/src/houdini/houdini/data/room.py", line 137, in add_penguin
    await RoomMixin.add_penguin(self, p)
  File "/usr/src/houdini/houdini/data/room.py", line 30, in add_penguin
    await p.room.remove_penguin(p)
  File "/usr/src/houdini/houdini/data/room.py", line 201, in remove_penguin
    del p.server.igloos_by_penguin_id[self.penguin_id]
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
KeyError: 103

Solution

Check for already loaded player's igloo before pulling a new igloo_room instance from igloo's owner penguin instance.

[AllinolCP]

according to the traceback shouldn't we check if the igloo still exists before we delete it?

[brosquinha]

according to the traceback shouldn't we check if the igloo still exists before we delete it?

@AllinolCP I'm a bit confused by the question, we're not deleting anything at this time. In fact, we are indeed checking if there is an already existing igloo_room for the penguin in question before fetching a new instance for it an attributing to the global dictionary.

[AllinolCP]

according to the traceback shouldn't we check if the igloo still exists before we delete it?

@AllinolCP I'm a bit confused by the question, we're not deleting anything at this time. In fact, we are indeed checking if there is an already existing igloo_room for the penguin in question before fetching a new instance for it an attributing to the global dictionary.

Right but the traceback on the exception is about removing an igloo

[brosquinha]

according to the traceback shouldn't we check if the igloo still exists before we delete it?

@AllinolCP I'm a bit confused by the question, we're not deleting anything at this time. In fact, we are indeed checking if there is an already existing igloo_room for the penguin in question before fetching a new instance for it an attributing to the global dictionary.

Right but the traceback on the exception is about removing an igloo

Ah, yes, sorry about that, a little further explanation on what's happening there:

Because the current implementation does not check for an existing igloo_room before fetching a new instance, we end up with two igloo_room instances for the same igloo. That's why both players successfully join the igloo, but neither can see each other, because the server understands that they are, in fact, on different rooms.

The current implementation is that once all players have left an igloo, it removes the igloo_room from the global dictionary. So, since there is only one player for each igloo_room, the first player to leave will successfully perform this operation, but the second player will not, hence the exception.

Now, I'm not sure why we need to empty this dictionary once there is nobody on a particular igloo, I didn't want to meddle with that. What I did feel was the root cause was the duplication of the igloo_room for the same igloo, so that's what I addressed. If you have further context on this particular piece of logic, please do share :+1:

So, to answer your question, we cannot reliably check if the igloo exists because we now have two instances for the same igloo, and the second instances replaces the first one in the dictionary, since it's keyed by its owner's id.

[AllinolCP]

according to the traceback shouldn't we check if the igloo still exists before we delete it?

@AllinolCP I'm a bit confused by the question, we're not deleting anything at this time. In fact, we are indeed checking if there is an already existing igloo_room for the penguin in question before fetching a new instance for it an attributing to the global dictionary.

Right but the traceback on the exception is about removing an igloo

Ah, yes, sorry about that, a little further explanation on what's happening there:

Because the current implementation does not check for an existing igloo_room before fetching a new instance, we end up with two igloo_room instances for the same igloo. That's why both players successfully join the igloo, but neither can see each other, because the server understands that they are, in fact, on different rooms.

The current implementation is that once all players have left an igloo, it removes the igloo_room from the global dictionary. So, since there is only one player for each igloo_room, the first player to leave will successfully perform this operation, but the second player will not, hence the exception.

Now, I'm not sure why we need to empty this dictionary once there is nobody on a particular igloo, I didn't want to meddle with that. What I did feel was the root cause was the duplication of the igloo_room for the same igloo, so that's what I addressed. If you have further context on this particular piece of logic, please do share :+1:

So, to answer your question, we cannot reliably check if the igloo exists because we now have two instances for the same igloo, and the second instances replaces the first one in the dictionary, since it's keyed by its owner's id.

Gotcha, IIRC there's also a bug with houdini where the owner cannot see the others who's already in their igloo.

Would this fix this issue?

[brosquinha]

Gotcha, IIRC there's also a bug with houdini where the owner cannot see the others who's already in their igloo.

Would this fix this issue?

It does for similar scenarios to the one I've described on the PR description, but I'm not aware if there are others that could trigger this behavior.

[AllinolCP]

Gotcha, IIRC there's also a bug with houdini where the owner cannot see the others who's already in their igloo.

Would this fix this issue?

It does for similar scenarios to the one I've described on the PR description, but I'm not aware if there are others that could trigger this behavior.

Alright then, Thank You!