Page MenuHomeWildfire Games

Hiding profile panel in lobby when it is not required for having a longer playerlist
Needs RevisionPublic

Authored by ffffffff on Sep 8 2017, 7:43 PM.

Details

Reviewers
elexis
JoshuaJB
Trac Tickets
#4972
Summary

Enlarge the playerlist, when no player is selected, by hiding the profile panel (when it is not needed). So we getting or more greater playerlist and can see more players.

When a player is selected, show the profile panel with the profile.


On pressing esc, deselect playerlist and gameslist and returns into initial state.

option

Test Plan

Select a player and see the profile panel unhides and the playerlist gets shortened in space.
Select then also a game.
Try deselect selection from playerlist and gamelist by pressing Esc and see if the lobby design goes back into init state.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

ffffffff updated this revision to Diff 3589.Sep 8 2017, 7:43 PM
ffffffff created this revision.

playerlist

ffffffff updated this revision to Diff 3590.EditedSep 8 2017, 7:46 PM

gamelist

ffffffff edited the summary of this revision. (Show Details)Sep 8 2017, 9:16 PM
ffffffff edited the test plan for this revision. (Show Details)
ffffffff retitled this revision from Lobby Playerlist fullsize esc deselect all selections to Lobby longer Playerlist by hiding profile panel when not required.
ffffffff retitled this revision from Lobby longer Playerlist by hiding profile panel when not required to Lobby longer Playerlist hiding profile panel when not required.
ffffffff retitled this revision from Lobby longer Playerlist hiding profile panel when not required to Hiding profile panel in lobby when it is not required for having a longer playerlist.Sep 10 2017, 6:06 PM
ffffffff edited the summary of this revision. (Show Details)
ffffffff edited the summary of this revision. (Show Details)
ffffffff edited the summary of this revision. (Show Details)
ffffffff edited the summary of this revision. (Show Details)
ffffffff edited the summary of this revision. (Show Details)Sep 10 2017, 6:08 PM
ffffffff edited the test plan for this revision. (Show Details)
ffffffff edited the summary of this revision. (Show Details)Nov 17 2017, 5:21 AM
ffffffff added subscribers: bb, temple, elexis.
ffffffff updated this revision to Diff 4811.Dec 18 2017, 3:00 AM
ffffffff edited the summary of this revision. (Show Details)

optional

ffffffff edited the summary of this revision. (Show Details)Dec 18 2017, 3:01 AM
ffffffff edited the summary of this revision. (Show Details)Dec 23 2017, 1:43 PM
ffffffff added a subscriber: mapkoc.

This is very nice. Maybe you can add a tooltip on top of profile panel so that people know to press ESC.
Remember to clean lines 617 and 927 :)

Tooltip on option description not enough? Default is disabled anyway. So you might read it when enable.

ffffffff updated this revision to Diff 5346.Jan 16 2018, 9:24 PM

Premium cancel hotkey stacked code.

ffffffff updated this revision to Diff 5355.Jan 17 2018, 5:13 PM
ffffffff updated the Trac tickets for this revision.Jan 18 2018, 7:11 AM
JoshuaJB requested changes to this revision.Jan 24 2018, 6:42 AM
JoshuaJB added a subscriber: JoshuaJB.

Fantastic direction! That panel should always have automatically hidden when empty, so no need to make it configurable. Thanks for finally looking into fixing this! I did a quick run over the patch and tried to point out anything that I imagine possibly being a problem.

binaries/data/config/default.cfg
404 ↗(On Diff #5355)

It should just automatically auto hide, no need to make it configurable

binaries/data/mods/public/gui/lobby/lobby.js
129–139

It might be good to rethink how you're doing this so that you don't need all the "return true"s added elsewhere

440–442

Again, no need for configurability. Opinionated design is not a bad thing

531

This doesn't add anything to the value of the function. It will only ever return true, so having it return anything at all is pointless.

712–715

I don't think you meant to include this change. If you did, some explanation would help

860

Same comment as on L531

869

Same comment as on L531

880

Could you name this something more descriptive? Maybe "setUserProfilePanelVisibility"?

1021–1022

I don't think you meant to include this change. If you did, some explanation would help

1190–1192

Should these globals not update automatically after the following two deselects?

If not, an event-driven model like that would probably be favorable.

This revision now requires changes to proceed.Jan 24 2018, 6:42 AM
ffffffff marked 7 inline comments as done.Jan 24 2018, 9:13 AM
ffffffff added inline comments.
binaries/data/mods/public/gui/lobby/lobby.js
712–715

test data. omit on commit.

880

It sets whole left panel size. So instead of setting size in xml we set it now here. Therefore name it setLeftPanelStyle. Same as in D817 function initDialogStyle().

1021–1022

only test data can be omited on commit

ffffffff updated this revision to Diff 5466.Jan 24 2018, 9:22 AM
ffffffff marked an inline comment as done.

"cond", "func" g_CancelHotkey attributes for check

ffffffff updated this revision to Diff 5467.Jan 24 2018, 9:30 AM

Put condition in acccording functions and make appropriate return statement if they have been applied or not. Maybe cleaner way of showing that these functions really done what they have to do and met appropriate conditions that they have to do something. Adapt g_CancelHotkey and call code (in lobby.xml). If you dont like this look in update history the previous diff.

Changes look great at a glance! I will try to take a longer look at it and try running it locally later today.

binaries/data/mods/public/gui/lobby/lobby.js
841

Nice optimization!

880

How about setLeftPanelExpanded then? I would mostly just like the call on L900 to be a bit more self-explanatory.

ffffffff added inline comments.Jan 24 2018, 11:06 PM
binaries/data/mods/public/gui/lobby/lobby.js
880

y
indeed

1188

setListBoxesUnselected()

ffffffff updated this revision to Diff 5492.Jan 25 2018, 8:56 PM

bugfix
rename

ffffffff added inline comments.Jan 25 2018, 8:57 PM
binaries/data/mods/public/gui/lobby/lobby.js
846

Got bug this better as g_SelectedPlayer should be set undefined. on selected = -1;

ffffffff added a comment.EditedFeb 27 2018, 12:36 AM

@JoshuaJB Are you still with us?

Also D948 is waiting..

JoshuaJB requested changes to this revision.Jul 16 2018, 2:56 AM

This seems to almost always work great, and the code looks pretty good. One bug though when I tested locally:

Repro:

  1. open the leaderboard
  2. select an entry
  3. observe player details pop up in background
  4. close leaderboard
  5. press escape
  6. observe a still-visible player details panel
binaries/data/mods/public/gui/lobby/lobby_panels.xml
13

Nit: Can you add a comment to explain what exactly this is doing? It looks a bit cryptic to an older JS dev

This revision now requires changes to proceed.Jul 16 2018, 2:56 AM