Page MenuHomeWildfire Games

Lobby Dialog Overlay in-game and gamesetup
ClosedPublic

Authored by ffffffff on Aug 21 2017, 1:51 AM.

Details

Summary

lobby_panels.xml split can be reverted to lobby.xml and be merged back into lobby.xml, as this patch uses style parameters in js to set lobby to dialog and normal window.

fullscreen ingame/gamesetup overlayed lobby (as pushed gui, no join button, no host button):

having lobby available in-game and in game-setup (and maybe in end-game summary) as overlay

shortcut hotkey should be configured ("Alt+L") globaly

if dialoged desired: (but that needs massive xml code moving)

Test Plan

Test connection to lobby in-game.
Test connection to lobby in gamesetup.
Test player presence when in game and return from.
Test Relogin by connecting in another window with same account to lobby to get a disconnect and reconnect then from within game by entering lobby again.
Test also in summary hitting lobby hotkey.

Also test bubble icon in gamesetup layout all should look sharp and nice.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

the force jedi escape key works

object hotkey at end so its popping gui at last due to doubled hotkeys escape aka cancel

ffffffff updated this revision to Diff 5150.Jan 6 2018, 9:12 PM

boss and set available when open dialog lobby. set busy when leave toggle.

binaries/data/mods/public/gui/prelobby/prelobby.xml
9 ↗(On Diff #5145)

y

elexis resigned from this revision.Jan 7 2018, 4:37 PM

Lobby overlay dialog in gamesetup and ingame with different outer gap sizes and darken fades in background https://imgur.com/a/Ev75Z
Last Image 42px and 0 0 0 200 darken color decided with stan and imarok. tx

ffffffff updated this revision to Diff 5207.EditedJan 10 2018, 3:03 AM

disconnect lobby fix
outer gap fix for dialog set to 42px
darker fade missing other patch first
comes D1214

ffffffff updated this revision to Diff 5208.Jan 10 2018, 3:05 AM

small correctment

not good and maybe bug is when in gamesetup enter chat text input field is focused u hit Alt+L for lobby toggle u open lobby dialog and when u return u got a "l" in ur chat. (Maybe not focus chat input when entering gamesetup)

ffffffff updated this revision to Diff 5289.Jan 14 2018, 8:46 PM

openLobbyDialog generalization and reconnect messageBox in gui/lobby/lobby_dialog.js.

ffffffff added inline comments.Jan 14 2018, 8:47 PM
binaries/data/mods/public/gui/gamesetup/gamesetup.js
348 ↗(On Diff #5289)

Can be deleted. unused.

ffffffff updated this revision to Diff 5291.Jan 14 2018, 9:14 PM

Reconnect messageBox in lobby. Put general code into gui/common/lobby.js. So its nice included into gamesetup, session, summary and lobby.xml.

ffffffff updated this revision to Diff 5307.Jan 15 2018, 2:19 AM

Take IsXmppClientConnected from engine to test connection state of Xmpp client in lobby.

ffffffff added inline comments.Jan 15 2018, 2:22 AM
binaries/data/mods/public/gui/lobby/lobby.js
472 ↗(On Diff #5307)

or disconnected. > according to the connection state.

binaries/data/mods/public/gui/lobby/lobby.xml
7 ↗(On Diff #5307)

unneeded. from previous version.

ffffffff updated this revision to Diff 5310.Jan 15 2018, 3:26 AM

Bubble icon white and gold style.

ffffffff updated this revision to Diff 5317.Jan 15 2018, 4:06 AM
ffffffff added inline comments.Jan 15 2018, 6:14 AM
binaries/data/mods/public/gui/lobby/lobby.js
459 ↗(On Diff #5317)

() => {}

460 ↗(On Diff #5317)

() => {}

ffffffff added inline comments.Jan 15 2018, 6:16 AM
binaries/data/mods/public/gui/lobby/lobby.js
400 ↗(On Diff #5317)

Engine.LobbySetPlayerPresence("available");

initGUIObjects();

can go below !g_Settings if

ffffffff added inline comments.Jan 15 2018, 6:18 AM
binaries/data/mods/public/gui/lobby/lobby.js
1159 ↗(On Diff #5317)
!g_Dialog not needed

hidden anyway when dialog

1159 ↗(On Diff #5317)

untrue needed for double click.

Attached the patch with some changes I made, some of them mentioned below, but it complains about the selected player not being found sometimes:

As I've already mentioned more than once: Engine.SwitchGuiPage("page_lobby.xml") -> Engine.SwitchGuiPage("page_lobby.xml", { "dialog": false });

summary should have a button IMO, would take an icon too if it has to be, but IMO it would look ugly.

You should ask bb which variant he prefers for the gamesetup. No shame to decide differently and not use the icon.

binaries/data/mods/public/gui/gamesetup/gamesetup.xml
256 ↗(On Diff #5317)

do the math

binaries/data/mods/public/gui/gamesetup/sprites.xml
11 ↗(On Diff #5317)

Wrong file, common/

The only other sprite in this file can be deleted afterwards. Then the entire file.

binaries/data/mods/public/gui/lobby/lobby.js
111 ↗(On Diff #5317)

Why isnt there any context in this diff?

130 ↗(On Diff #5317)

Given that the other gui updaters aren't called simultaneously, updateGUIConnectionState

384 ↗(On Diff #5317)

Intersting, comma.

401 ↗(On Diff #5317)

The GUI functions should be grouped, for instance

+ initDialogStyle();
+ initGameFilters();
+ updateConnectedState();

429 ↗(On Diff #5317)

update call can be moved out of the if

441 ↗(On Diff #5317)

You have been disconnected from the lobby. Do you want to reconnect

443 ↗(On Diff #5317)

those too short strings already exist

451 ↗(On Diff #5317)

After having a look at the other GUI functions, initDialogStyle seems to fit better actually

458 ↗(On Diff #5317)

withContext

460 ↗(On Diff #5317)

This looks better if we only assign the onPress in an if g_Dialog statement rather than a ternary with a weird empty function

464 ↗(On Diff #5317)

This one looks preferable to me:

+ Engine.GetGUIObjectByName("gameInfoEmpty").size = "0 0 100% 100%-30" + (g_Dialog ? "" : "-30");
+ Engine.GetGUIObjectByName("gameInfo").size = "0 0 100% 100%-30" + (g_Dialog ? "" : "-60");

466 ↗(On Diff #5317)

This one still wasn't necessary according to my tests.

386 ↗(On Diff #5134)

Putting globals in the middle of the code?

114 ↗(On Diff #5310)

// Ensures that no more than one message box is opened at a time.

binaries/data/mods/public/gui/lobby/lobby_panels.xml
364 ↗(On Diff #5317)

to the top

binaries/data/mods/public/gui/prelobby/prelobby.js
193 ↗(On Diff #5317)

the other 2 calls should have dialog = true for consistency

ok im fine with the patch from u just the points i mentioned pls reply/test. tx

binaries/data/mods/public/gui/gamesetup/sprites.xml
11 ↗(On Diff #5317)

lol sry this was not update to date. +1

binaries/data/mods/public/gui/lobby/lobby.js
111 ↗(On Diff #5317)

Sry was from git diff before IsXmppConnected commit. Now svn diff with context i can take. I guess, because i cant commit to the svn localy i do it with git. Can i make context diffs with git too?

384 ↗(On Diff #5317)

two sentences i guess.

401 ↗(On Diff #5317)

works

443 ↗(On Diff #5317)

so what we gonna do with them

458 ↗(On Diff #5317)

seems to be for the languages

466 ↗(On Diff #5317)

m8 be cleaner if true ommit.

binaries/data/mods/public/gui/lobby/lobby_panels.xml
364 ↗(On Diff #5317)

Stay. Wont work. Test escape. Cancel hotkey is escape used for leaderboard, profile search hide too. It needs to hide that first (even if not shown) and then actualy leave lobby or u get error missing variables and gui elements because of popGui for in the hiding function from that two dialog boxes. So at least cancel key must be down here. I just grouped them both here. Can be only cancelDialog hotkey here.

binaries/data/mods/public/gui/prelobby/prelobby.js
193 ↗(On Diff #5317)

all calls i can find in that patch do have dialog: true/false no?

bug

change
in summary.js
L468 (func continueButton()) dialog: true -> false!

		});

else if (Engine.HasXmppClient())

		Engine.SwitchGuiPage("page_lobby.xml", { "dialog": false });

change
in gamesetup.js
L1734 (func cancelSetup()) dialog: true -> false!

		if (g_IsController)
			Engine.SendUnregisterGame();

		Engine.SwitchGuiPage("page_lobby.xml", { "dialog": false });

You can update the diff if you want, I'll do a rmgen cleanup now.

binaries/data/mods/public/gui/lobby/lobby.js
384 ↗(On Diff #5317)

Just removing the comma should do

443 ↗(On Diff #5317)

keep

binaries/data/mods/public/gui/lobby/lobby_panels.xml
364 ↗(On Diff #5317)

Oh that's where the bug came from... That's not too good. We should add an XML comment then.

elexis accepted this revision.Jan 16 2018, 3:58 PM

Thanks for the patch fpre.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
1113 ↗(On Diff #5321)

Moving to g_MiscControls

binaries/data/mods/public/gui/gamesetup/gamesetup.xml
261 ↗(On Diff #5321)

apparently hotkeys work for hidden and enabled objects. I thought they don't, at least for enabled. so the check is not redundant.
Moving to JS while at it.

binaries/data/mods/public/gui/lobby/lobby.js
512 ↗(On Diff #5321)

unneeded check (tautologic condition)

binaries/data/mods/public/gui/lobby/lobby_panels.xml
363 ↗(On Diff #5321)

Thanks for the comment

binaries/data/mods/public/gui/manual/intro.txt
52 ↗(On Diff #5321)

ideally consistency with default.cfg, both the phrasing and the location of the line

binaries/data/mods/public/gui/pregame/mainmenu.xml
294 ↗(On Diff #5321)

very nice!

binaries/data/mods/public/gui/session/menu.xml
93 ↗(On Diff #5321)

The button should actually be below options. The last item should be exit, the one before that resign (because it ends the game too), the one before that pause (because it interacts with the game too). Summary lobby and options all open independent dialogs.

103 ↗(On Diff #5321)

Lobby hotkey should be disabled completely when not in lobby. Moving to JS.

binaries/data/mods/public/gui/session/session.js
628 ↗(On Diff #5321)

Hiding would be nicer, but then we need to do size mess. Cleanup eventually.

binaries/data/mods/public/gui/summary/summary.xml
34 ↗(On Diff #5321)

Still misses a lobby dialog button or icon, but that can be done afterwards.

This revision is now accepted and ready to land.Jan 16 2018, 3:58 PM
elexis added inline comments.Jan 16 2018, 4:02 PM
binaries/data/mods/public/gui/pregame/mainmenu.xml
300 ↗(On Diff #5321)

needs a if (Engine.StartXmppClient) check for people who compiled without lobby but felt the urge to press the hotkey

This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Jan 16 2018, 4:05 PM
elexis added inline comments.Nov 13 2019, 4:42 PM
binaries/data/mods/public/gui/lobby/lobby_panels.xml
364 ↗(On Diff #5317)
  1. TODO: fixed by rP22200.

That might explain errors with GetGUIObjectByName (but not "missing variables" if that means reference errors).
nani could reproduce this error on a23b released:

(16:35:19) nani: ERROR: JavaScript error: gui/lobby/lobby.js line 871 TypeError: playerList is undefined lookupSelectedUserProfile@gui/lobby/lobby.js:871:6 setLeaderboardVisibility@gui/lobby/lobby.js:844:2 __eventhandler437 (press)@__internal(22) press:0:1

ERROR: JavaScript error: gui/lobby/lobby.js line 851 TypeError: Engine.GetGUIObjectByName(...) is undefined setUserProfileVisibility@gui/lobby/lobby.js:851:53 __eventhandler441 (press)@__internal(33) press:0:1

and I have an a23b patched with rP22200, so I suspect it was that.

We see that even playerList does not throw a referene error but it complains about it being undefined, and the value comes from
let playerList = Engine.GetGUIObjectByName(guiObjectName);
So it seems like all of the complained values come from this GetGUIObjectByName, so it must have been that commit.

  1. Second TODO:

There is another issue, that pressing escape when the page is a dialog closes both the lobby page and the leaderboard page / profile page. But there should only be one page closed per press. This can actually be fixed by reassigning the same hotkey value instead of using the hotkey for 3 objects, (for example 1 gui object or Engine.SetGlobalHotkey from rP22851)

First TODO removal and second TODO fix in D2412.

nani added a subscriber: nani.Nov 19 2019, 5:47 PM
nani added inline comments.
binaries/data/mods/public/gui/lobby/lobby_panels.xml
364 ↗(On Diff #5317)

There is another issue, that pressing escape when the page is a dialog closes both the lobby page and the leaderboard page / profile page. But there should only be one page closed per press. This can actually be fixed by reassigning the same hotkey value instead of using the hotkey for 3 objects, (for example 1 gui object or Engine.SetGlobalHotkey from rP22851)

Leaderboard page / profile page are not really pages but overlays of the same page

elexis added inline comments.Nov 19 2019, 9:10 PM
binaries/data/mods/public/gui/lobby/lobby_panels.xml
364 ↗(On Diff #5317)

I don't know what to respond to that.

nani added inline comments.Nov 19 2019, 9:36 PM
binaries/data/mods/public/gui/lobby/lobby_panels.xml
364 ↗(On Diff #5317)

No need, just stating that they are just overlays not real pages. ?

elexis added inline comments.Nov 20 2019, 11:14 AM
binaries/data/mods/public/gui/lobby/lobby_panels.xml
364 ↗(On Diff #5317)

In the lobby you meant that if it would use PushGUIPage / PopGUIPage, then the "double-close" issue wouldn't happen, so that's true and perhaps it can be changed to that eventually (or perhaps not for performance reasons?)

But before then we need a way to have one page update the other page (thats needed for gamesetup aiconfig page too), since the leaderboardpage uses the profile panel of the lobby page (though that way around its different from gamesetup/aiconfig and session/networkpage). Anyway, I got it bandaged in D2412 for now by reassigning the cancel hotkey upon opening and closing the page.