Page MenuHomeWildfire Games

Correct Z order for the chat when waiting for other clients to load the game
ClosedPublic

Authored by elexis on May 26 2017, 1:35 PM.

Details

Summary

Often some clients have already finished the loading screen but are waiting for up to a minute for some random client loading terribly slow or timing out.
Since chat is a feature which should be supported, we have to change the Z index of the gray overlay that is rendered above of the chat.

There is a transparency issue when not moving that netStatus object into the root object which has held back this change since years.
The pause page does it right.
While at it, remove that unneeded button label text and the unused styles with it.



Test Plan

The messages.js diff (which should not be committed) keeps the chat window visible and thus simulates people waiting for some client to load.
Revert that file and see the test plan of D120 for a morecomplicated network based test.

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

elexis created this revision.May 26 2017, 1:35 PM
Vulcan added a subscriber: Vulcan.May 26 2017, 1:36 PM
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'Engine'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/messages.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/messages.js
| 585| 585|  */
| 586| 586| function updateTimeNotifications()
| 587| 587| {
| 588|    |-	let notifications =  Engine.GuiInterfaceCall("GetTimeNotifications", g_ViewedPlayer);
|    | 588|+	let notifications = Engine.GuiInterfaceCall("GetTimeNotifications", g_ViewedPlayer);
| 589| 589| 	let notificationText = "";
| 590| 590| 	for (let n of notifications)
| 591| 591| 	{

binaries/data/mods/public/gui/session/messages.js
| 615| »   while·(true)
|    | [NORMAL] ESLintBear (no-constant-condition):
|    | Unexpected constant condition.

binaries/data/mods/public/gui/session/messages.js
| 496| »   let·cheatCode·=·Object.keys(g_Cheats).find(cheatCode·=>·text.indexOf(cheatCode)·==·0);
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/session/messages.js
| 874| »   if·(chatAddressee.selected·>·0·&&·(text.indexOf("/")·!=·0·||·text.indexOf("/me·")·==·0))
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/gui/session/messages.js
| 874| »   if·(chatAddressee.selected·>·0·&&·(text.indexOf("/")·!=·0·||·text.indexOf("/me·")·==·0))
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/session/messages.js
|1088| »   let·isMe·=·msg.text.indexOf("/me·")·==·0;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/session/messages.js
|1092| »   isMe·=·msg.text.indexOf("/me·")·==·0;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/session/messages.js
|1210| »   »   if·(text.indexOf(pName·+·"·")·==·0·&&·pName.length·>·match.length)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/54/ for more details.

elexis edited the summary of this revision. (Show Details)May 26 2017, 1:39 PM

Also I argue that it's not broken to have those controls enabled, these commands are stacked properly. Those controls aren't disabled while the game is paused yet and that should still be fixed at some point, logically independent of the Z order consideration and style removal.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1368/ for more details.

fatherbushido accepted this revision.May 26 2017, 5:09 PM

Works as expected.
Excepting I miss something, it seems a better behavior.
(I won't review code.)

This revision is now accepted and ready to land.May 26 2017, 5:09 PM

Works as expected.

(We tested it in actual MP too)

Excepting I miss something, it seems a better behavior.

(I won't review code.)

Well that one line of code wasn't up for review anyway :P

This revision was automatically updated to reflect the committed changes.