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.
Tags
None
Referenced Files
F5200226: D556.id2221.diff
Mon, Sep 16, 11:03 PM
Unknown Object (File)
Fri, Sep 13, 5:39 AM
Unknown Object (File)
Sun, Sep 8, 9:16 AM
Unknown Object (File)
Tue, Sep 3, 11:07 PM
Unknown Object (File)
Sun, Aug 25, 12:30 AM
Unknown Object (File)
Sun, Aug 25, 12:30 AM
Unknown Object (File)
Sun, Aug 25, 12:29 AM
Unknown Object (File)
Sun, Aug 25, 12:14 AM
Subscribers

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.

unpatched.jpg (1×1 px, 388 KB)

patched2.jpg (1×1 px, 430 KB)

patched1.jpg (1×1 px, 418 KB)

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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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.

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.

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.