Opened 9 years ago

Closed 8 years ago

#3388 closed defect (worksforme)

[PATCH] Lobby - no playerlist after connecting

Reported by: elexis Owned by: elexis
Priority: Should Have Milestone:
Component: Multiplayer lobby Keywords: patch
Cc: Patch:

Description

You can connect to the lobby and sometimes don't have a playerlist.

This is because r16961 removes the call to updatePlayerList when the user has joined. The playerlist is only initialized after something calls that function, for example a click on the column header.

There is still a call to that function in init(), but that never gets the actual playerlist, so it should be removed I think.

It seems we have to wait until our local user has connected and then update the playerlist, the way it was implemented before that commit. For example calling updatePlayerList in the second call to onTick (after the GUI has been initialized) also doesn't fill the playerlist.

Set the milestone to a19 because it must be fixed before the release.

Attachments (4)

ticket3388playerlist_1.diff (655 bytes ) - added by bb 9 years ago.
This patch seems to resolve the problem.
t3388_fix_initial_playerlist.diff (2.6 KB ) - added by elexis 9 years ago.
t3388_fix_initial_playerlist_v2.diff (3.0 KB ) - added by elexis 9 years ago.
t3388_add_comment.diff (2.1 KB ) - added by elexis 9 years ago.

Download all attachments as: .zip

Change History (12)

by bb, 9 years ago

Attachment: ticket3388playerlist_1.diff added

This patch seems to resolve the problem.

comment:1 by bb, 9 years ago

Keywords: review patch added
Summary: Lobby - no playerlist after connecting[PATCH] Lobby - no playerlist after connecting

comment:2 by elexis, 9 years ago

Keywords: review removed

Unfortunately that doesn't fix it yet. Actually that code part is never reached on my computer. Maybe it is reached if you disconnect and reconnect without changing the menu somehow.


For testing you should add the following line to UpdatePlayerList (after the initialization of cleanPlayerList):

error(new Error().stack + "\n" + JSON.stringify(cleanPlayerList));

This way you can see when that function is called, which event called it and which elements that list has. This way you can make sure that nothing accidental like the ratinglist updates the playerlist and hides the bug.

by elexis, 9 years ago

comment:3 by elexis, 9 years ago

Keywords: review added

Here is what happens:

  • Clicking "lobby" in the main menu calls prelobby.js
  • Clicking "login" waits for the "connected" event and then loads lobby.js
  • Immediately after we are connected, init() in lobby.js is called. But at that point, the playerlist was not yet downloaded by the network client. So we must remove this useless call.
  • We know that the userlist was downloaded completely after we received the join event of our own user.
  • Thats why we should just send a new GUI message that calls UpadtePlayerList() then

The patch above implements that and has been tested.

Code that uses the "connected" event should be introduced in #3389.

Last edited 9 years ago by elexis (previous) (diff)

comment:4 by elexis, 9 years ago

Big mistake to remove the updatePlayerList() from init().

It is not needed when connecting to the lobby, as we didn't download the playerlist yet and in that case, the list will be empty. When it has been downloaded, join-completed will be called and update the playerlist.

But if we return from a match, lobby.js will be initialized again and as we already downloaded the list in the past, join-completed will not be called again in that situation. But in that case, the playerlist is filled in init().

So for returning from a match this call is needed.

That's why I added the comment instead of removing that call.

comment:5 by elexis, 9 years ago

Priority: Must HaveShould Have

Since r16997, this bug should be solved too. But I think we should at least add a comment why the code works, as someone modifying these cases below could reintroduce the bug.

Why the current code works: As of r16997, the empty playerlist is replaced in these three situations:

  • when the subject is fetched. It it is always received after the initial joins and after the historical chat messages (even without the call to updateSubject(Engine.LobbyGetRoomSubject());). So this actually fixes the bug implicitly.
  • when the update of the call to Engine.LobbySetPlayerPresence("available"); in init() is received
  • receiving an updated ratinglist (may or may not happen)

Before r16997, the playerlist wasn't reloaded when the subject changed or when someone changed the presence, so the first two cases didn't apply.

Proposed Update: Reloading the playerlist after knowing that it has been downloaded is done in revisions before r16961 and in the proposed patches above. Having it formalized this way might help understanding the code.

On the other hand, having just a comment could be better, as it doesn't increase code complexity.

by elexis, 9 years ago

Attachment: t3388_add_comment.diff added

comment:6 by Stan, 9 years ago

Owner: set to elexis

comment:7 by elexis, 9 years ago

Milestone: Alpha 19Alpha 20

Can wait for a20, since r16997 fixed it coincidentally. We should make sure that future code changes don't introduce it again by adding the comment above.

comment:8 by elexis, 8 years ago

Cc: JoshuaJB removed
Keywords: review removed
Milestone: Alpha 20
Resolution: worksforme
Status: newclosed

Closing it as it's fixed by r16997. A better comment than the proposed one may or may not be added.

Note: See TracTickets for help on using tickets.