Page MenuHomeWildfire Games

Lobby class implementation
ClosedPublic

Authored by elexis on Nov 9 2019, 2:09 AM.

Details

Summary

In the course of #5387, this patch rewrites the lobby GUI page code to use object orientation.

Resulting folder structure:

LeaderboardPage/LeaderboardList.js
LeaderboardPage/LeaderboardPage.js
LeaderboardPage/LeaderboardPage.xml
lobby.js
Lobby.js
LobbyPage/AnnouncementHandler.js
LobbyPage/Buttons/BuddyButton.js
LobbyPage/Buttons/HostButton.js
LobbyPage/Buttons/JoinButton.js
LobbyPage/Buttons/LeaderboardButton.js
LobbyPage/Buttons/ProfileButton.js
LobbyPage/Buttons/QuitButton.js
LobbyPage/Chat/ChatCommandHandler.js
LobbyPage/Chat/ChatInputPanel.js
LobbyPage/Chat/ChatMessages/ChatMessageEvents.js
LobbyPage/Chat/ChatMessages/ChatMessageFormat.js
LobbyPage/Chat/ChatMessages/ChatMessageFormatMe.js
LobbyPage/Chat/ChatMessages/ChatMessageFormatSay.js
LobbyPage/Chat/ChatMessages/ChatMessagePrivateWrapper.js
LobbyPage/Chat/ChatMessagesPanel.js
LobbyPage/Chat/ChatPanel.js
LobbyPage/Chat/ChatPanel.xml
LobbyPage/Chat/StatusMessages/StatusMessageEvents.js
LobbyPage/Chat/StatusMessages/StatusMessageFormat.js
LobbyPage/Chat/SystemMessages/SystemMessageEvents.js
LobbyPage/Chat/SystemMessages/SystemMessageFormat.js
LobbyPage/Chat/TimestampWrapper.js
LobbyPage/ConnectionHandler.js
LobbyPage/GameDetails.js
LobbyPage/GameDetails.xml
LobbyPage/Game.js
LobbyPage/GameListFilters/MapSize.js
LobbyPage/GameListFilters/MapType.js
LobbyPage/GameListFilters/OpenGame.js
LobbyPage/GameListFilters/PlayerCount.js
LobbyPage/GameListFilters/Rating.js
LobbyPage/GameListFilters.xml
LobbyPage/GameList.js
LobbyPage/GameList.xml
LobbyPage/KickStrings.js
LobbyPage/LobbyPage.js
LobbyPage/LobbyPage.xml
LobbyPage/PlayerColor.js
LobbyPage/PlayerList.js
LobbyPage/PlayerList.xml
LobbyPage/ProfilePanel.js
LobbyPage/ProfilePanel.xml
LobbyPage/Subject.js
LobbyPage/Subject.xml
lobby.xml
ProfilePage/ProfilePage.js
ProfilePage/ProfilePage.xml
XmppMessages.js

The purpose is to improve moddability, extensibility, readability and scalability by decoupling different page components and separation of concerns.
The class container allows storing variables as locals instead of globals and facilitates information hiding.

Test Plan

Correctness:
Dullishly test functionality of every line. Change default.cfg to the release lobby and bots, then everything gamelist, playerlist and chat related can be tested except joining and hosting games without OOS.

Completeness: One can look through any xmpp message handler easily before and after the patch and see that they do the same as before. I remember that I didn't delete any line but only moved them and added some iteratively. The Game class cache needs to be updated everytime one of its dependent values is updated, so one can go through all dependent variables and see if they are updated equally.
Going through this code several times makes one rediscover all the TODOs since introduction of the lobby, see the comments in the diff to see which things should be accounted here while rewriting and which ones have to be declared out of scope.

Performance:
Use Profiler2 and Engine.GetMicroseconds to measure performance and how often messages occur to estimate the performance impact and locate bottlenecks. See the analysis in the comments below and consequential optimizations.

Event Timeline

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

Lobby rewrite logbook entry:

  • The days before yesterday I
    • implemented some of the TODOs in the last upload and in the committed codebase, identified some new TODOs and fixed them including issues in or fixed by D720, rP19250, rP14098, rP20064, rP21474, rP23157,
      • rewrote the chat to include System and StatusNotification type and sprintf argument object caching. Having rewritten the code to be object oriented first made it trivial to add the cached objects in an appropriate place, seemingly confirming the OOP concept.
  • Yesterday I measured and compared the different message handlers, identified which were the most slow ones, see Experiment (1).
  • Today Using two methods (GetMicroseconds and Profiler2) I measured and optimized the updateGameList mechanism, see Experiment (2) Half of the time is spent in the .list_foo = bigArray assignment. That copy (JS array -> C++ vector of something) seems unavoidable since the renderer can't consume the JS array without big performance impact).

Measurement: Message type performance
I collected 5 different samples of XmppMessages from the arena23b release lobby room when there were about 70 players online.
I used the following a23b diff to obtain the data:

Measurement (A)
Plotted:

On the X axis is the message index (x is the x'th message received).
On the Y axis is the amount of time that the handlers of this message consumed in milliseconds (measured with microsecond precision).

The number of messages per type shown in the chart in the shown order are

 32 chat, join
 28 chat, leave
369 chat, presence
153 chat, room-message
  2 chat, subject
262 game, gamelist
 33 game, ratinglist

Interpretation:

  • 32 join and leave messages happen rarely (1/sec if very busy?), when they happen only 1ms, so order of magnitude of who cares
  • 369 presence changes are by far the most frequent message. There is a lot of work to do in that case, so the 0 line must be wrong, and it i wrong since updatePlayerList is the performane heavy thing triggered by that presence change but thats deferred and thus not measured in the above handler measurement, so that measurement must be repeated.
  • 153 chat, room-messages - we observe that their length varies between 1 and 2 milliseconds processing time per chat room-message. This is probably only explainable with chat text length.
  • 2 chat, subject, never happens and below 4ms processing time, who cares.
  • 33 game, ratinglist empty event handler, same as presence changes handler, needs to be measured again.
  • 262 game, gamelist - at least twice as slow as chat messages, 4 milliseconds per gamelist message and twice as many gamelist messages in that sample than chat messages.

So the question is whether I'm wasting my time with measuring performance that doesnt even matter or whether it's an important mistake to change the code to object orientation and perhaps unknowingly and recklessly making things worse?
The updateGameList handler consumes 5 milliseconds, but that was only for a lobby with 70-80 players, sometimes there are 120 in the arena23b lobby, and it should scale much further ideally.
If we look at the code that triggers gamelist updates, that's in gamesetup/ and session/ and is triggered everytime a player joins or leaves the host, everytime the host changes the map or the teams.
There is some delay mechanism to make it send this message at most once every 2 or 3 seconds or something.
But we must conclude from scalability that this message type can be sent extremely often.
So perhaps worst case scenario each host sends 1 message per second (scrolling through mapselection) and there may be 50 hosts.
At that point there should be throttling within XPartaMupp if that isn't already the case - to limit to 1 gamelist per second per client at most.
There is no throttling currently, see XPartaMupp.py command == 'changestate' calling self.sendGameList() which performs the broadcast instantly!

So conclusively optimization is actually useless because throttling is necessary for network bandwidth reasoning and solves any possible clientside performance issue already.
5 milliseconds per message which arrives at most once per second.
Regardless I optimized a lot as I accounted for current circumstances first, meh.

Measurement: a23b lobby slow loading:
I noticed the a23b lobby takes more than 1 second to display playerlist, gamelist and chat when joining when there are 80+ players online.
This is far too slow and demands message batch processing.
Especially for the chat panel which shall only be updated once all chat messages in the current batch have been processed.
This happens when opening the lobby page for the first time (receiving scrollback) and when opening the dialog page during or after returning a match (possibly receiving hours worth of / thousands of chat messages).
Then batching is absolutely necessary.

Measurement: messages per tick:
There is the question whether the C++ diff changing LobbyGuiPollNewMessages to return an array of all messages queued to be processed is useful or not.
Currently there is an LobbyGuiPollNewMessages call per onTick. The last uploaded and current diff introduces one array that is empty almost always, except when opening the GUI page (i.e. entering the lobby or opening it as a dialog during a running game, or returning from a match).

There are very few messages per onTick call (frame rendered).

373 Not Received
 20 Received

So there'd be 370 arrays created but only 20 times they'd be non-empty whereas currently it returns JS::UndefinedValue.
For that reason the function was implemented to return JS::UndefinedValue instead of an empty array despite that making the code < 5 lines of code more complex.

Creating a single array every half second is nothing, so this optimization is very useful for displaying chat messages only after a chat message batch update when opening the page.
The array enables batch processing easily, without having to introduce a secondary JSInterface function (we currently have LobbyGuiPollHasPlayerListUpdate and that is quite ugly already).

We also notice that this function should not be called per frame at all, but only when a message had been created.
D2410 implements this for the networking code, the same would also apply to the XmppClient GUI message queue.
Then the function is only called 20 times where it is currently called 373 times (every frame vs every 18th frame)!

Measurement updateGameList caching and optimizations:
As mentioned I've implemented some optimizations leveraging the object orientation for caching.
We see some of these add some benefit, like 20% faster computation maybe (the measurement has been outdated by further optimizations already).
But that's still useless if the list is updated at most once per second!
So whatever you see here, it doesn't seem to matter:

Measurement Profiler2:
For the first time in my career I've had used the Profiler2 JavaScript function Engine.ProfileStart / Engine.ProfileStop to measure what the time in updateGameList is spent on.
It turns out half of the time is spent for the COList.cpp list_foo = bigArray assignments.
So even if 100% of the JS performance cost would be cut out, it would achieve at most 50% performance increase.
As mentioned we can't really avoid the copy (JS array <- C++ vector, see FromJSVal<CGUIList>) copy since the renderer can't performantly read a JS array.

So not only is it useless to optimize this, it's also impossible to do a lot, meh^2.

The JS updateGameList code however could still be further optimized by caching most of the strings created in JS.
By creating class Game, reusing this Game for the same ip/port pair and updating that data only if the stanza property changed rather reconverting all stanza properties (translate / escapeText / compute player rating etc).

So the big conlusion is that this is a performance critical part that should be optimized by triggering the event less often rather than optimizing the event handling.
With that all of the optimizations I applied are probably useless and I have to stop implementing the class Game despite it sounding nice and very OOP.

Or perhaps the event handler optimizations are useful?
Back in the day when working on rP19641 scythetwirler asked to still have the gamelist update immediately when someone joined or left the game.
This would kill the assumption to get only 1 of these gamelist messages per second.
And in fact with 50 hosts where one player joins or leaves every second there'd still be 50 gamelistupdates per second according to that design decision.

That would be one updateGameList every 20 milliseconds, with the event processor consuming 5 milliseconds!

So that

Some notes of the changes in the patch so far (subset of the to be committ message):

Rewrite lobby GUI page to use class semantics.

Performance:
Implement batch message processing in the XmppClient to rebuild GUI objects only once when receiving backlog or returning from a match.
Don't create sprintf objects every call for chat messages.

Quality hurt fixes:
Introduce Status chat message type to supersede "/special" chat command + "isSpecial" property from rP14098 (formerly g_specialKey rP17360) deluxe hack.
Introduce System chat message type to supersede system errors disguising as chat from a mock user called "system".
Erase comments defining this chat as IRC, even at the cost of changing strings, this is not IRC in any way!

Defect fixes:
Add escapeText in lobby/ to avoid players breaking the lobby for every participant, refs D720, comments by bb.
Do not hide unrecognized chat commands sent by remote users, fixes #5615.
Removes cancel hotkey note from lobby_panels.xml from D817/rP20886 since the described issue was fixed by rP22200/D1701.
Adapt XmppClient to victoryCondition -> victoryConditions gamesetup.js change from rP21474/D1240 (while the field is still not displayed and has insufficient screenspace to be displayed, refs rP19642).
Update playerpage profile only if the account matches.

Other cleanups:
Move code from XML to JS.
Move size values from JS to XML.
Rename "user" to "player".
Fixes lobby/ eslint warnings, refs D2261.
Remove message.nick emptiness check from rP20064/D835, since XEP-0045 dictates that it MUST be non-empty.
Add translated string for deleted subjects.
Add TODOs for some evident issues.

Here the profiler2 measurements of updateGameList:

Here we see that the list_foo = bigarray assignments (gamelist GUI assignment) makes up 40% of the time spent in updateGameList.

I had applied some optimizations to updateGameList, for example the sort function doesnt have to compute the values again and again per game, but they can be computed in the parseGame function that is applied before the filtering.
Here we see that every part of parseGame function, which now consumes the otehr 40% of the time.
We see that every part is equally slow, which makes it very hard to optimize in individual steps:

But one can optimize it by introducing a class Game which caches exactly every converted stanza property and thus scraps off the translate, escapeText, rating sum, JSON.parse calls.
This I will implemented evidently not today.

Measurement: Batch message count

750 Received 1 message
 44 Received 2 message
  8 Received 3 message
  1 Received 17 message

We see that we almost never get batches, i.e. the batch message optimization really only holds for the page load events.

Measurement: total time per message type

Total time
presenc	1.584
ratinglist	0.393
subject	1.454
join	23.493
leave	21.404
room-message	146.376
gamelist	881.535

This further solidifies the impression that gamelist updates are the more timeconsuming functions (that is if their processing time is relevant at all, depending on the hypothetical future throttling decision).

Measurement: Time between message batches

On this logarithmic chart we see:

on the X axis: message ID (x'th message batch, sorted by time since last message batch)
on the Y axis: time since last message batch in milliseconds

The chart is logarithmic as there seems to be some sort of exponential distribution.

We see:

  • 5% of the message batches (600/640?) have 10seconds between distance to the previous message batch.
  • 50% the messages (350) have less than 1000ms duration between batches.
  • 25% of the message batches (187) have 125ms duration between the batches.

So this means if there is no throttling implemented somewhere, we can assume that every message hits us every 50 milliseconds. (This sample was with 80 players, there are 120 sometimes, and the code should work for more than that).

Here the depicted charts for libreoffice:

So I still wonder whether to conclude from that that the performance optimization is useless bloat to the code or not.

I certainly don't want to see the page loading as slowly as it does currently in a23 when opening it, but that can be achieved with the current implementation of this patch already probably.

I will see, but upload now my current working copy so I have a backup and the version with the Engine.ProfileStart calls and that hypothetical readers can hypothetically read and give hypothetically remarkable feedback.

Implementing the class Game seems in line with the object orientation rewrite (if we already have plain JS Objects why not make them smart JS objects if already rewriting everything) and
necessary if we polish for up to 50ms time between gamelist updates,
and kind of but not completely useless if we assume gamelist updates every 250ms.

However, having a class Game would also allow further event processing.

Because it allows the client to register if a new game was inserted or if a game ended, even if the bot doesnt yet provide that information.
It even allows the client to decide which of the gamelist attributes has changed.

So for example the class Game would allow lobby developers to play an acoustic notification if a buddy has joined or finished a game.
So seems like I justified wasting tomorrow as well on this rewrite.

elexis updated this revision to Diff 10352.Nov 18 2019, 12:21 AM

See above, chat Status/System notification rewrite, updateGameList optimizations, TODO removal

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/599/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lobby/IXmppClient.h
|  24| namespace·StunClient·{
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceStunClient{' is invalid C code. Use --std or --language to configure the language.

source/lobby/scripting/JSInterface_Lobby.h
|  26| namespace·JSI_Lobby
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceJSI_Lobby{' is invalid C code. Use --std or --language to configure the language.

source/lobby/XmppClient.h
|  24| 
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceStunClient{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/XmppMessages.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/XmppMessages.js
|  16|  16| 		this.messageBatchProcessedHandlers = new Set();
|  17|  17| 
|  18|  18| 		Engine.GetGUIObjectByName("lobbyPage").onTick =
|  19|    |-			this.handleMessages.bind(this, Engine.LobbyGuiPollNewMessages)
|    |  19|+			this.handleMessages.bind(this, Engine.LobbyGuiPollNewMessages);
|  20|  20| 	}
|  21|  21| 
|  22|  22| 	registerHandler(type, level, handler)
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/XmppMessages.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/XmppMessages.js
|  48|  48| 			let z = Engine.GetMicroseconds();
|  49|  49| 			for (let handler of this.messageHandlers.game.gamelist)
|  50|  50| 				handler(null, gameList);
|  51|    |-			warn("GameList update with games took " + ((Engine.GetMicroseconds() - z) / 1000))
|    |  51|+			warn("GameList update with games took " + ((Engine.GetMicroseconds() - z) / 1000));
|  52|  52| 		}
|  53|  53| 	}
|  54|  54| 

binaries/data/mods/public/gui/lobby/XmppMessages.js
|  19| »   »   »   this.handleMessages.bind(this,·Engine.LobbyGuiPollNewMessages)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/gui/lobby/XmppMessages.js
|  51| »   »   »   warn("GameList·update·with·games·took·"·+·((Engine.GetMicroseconds()·-·z)·/·1000))
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
|  85|  85| 			Engine.ProfileStart("parseGameList");
|  86|  86| 			let selectedColumn = this.gamesBox.selected_column;
|  87|  87| 			for (let game of this.gameList)
|  88|    |-				this.parseGame(selectedColumn, game)
|    |  88|+				this.parseGame(selectedColumn, game);
|  89|  89| 			Engine.ProfileStop();
|  90|  90| 		}
|  91|  91| 		{

binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
|  91| »   »   {
|    | [NORMAL] ESLintBear (no-lone-blocks):
|    | Nested block is redundant.

binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
|  97| »   »   {
|    | [NORMAL] ESLintBear (no-lone-blocks):
|    | Nested block is redundant.

binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
|  88| »   »   »   »   this.parseGame(selectedColumn,·game)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|  51|  51| 	onSelectionChange()
|  52|  52| 	{
|  53|  53| 		let selectedType = this.gameRatingFilter.list_data[this.gameRatingFilter.selected];
|  54|    |-		let selectedRating = +selectedType.substr(1)
|    |  54|+		let selectedRating = +selectedType.substr(1);
|  55|  55| warn(1234)
|  56|  56| 		this.filter =
|  57|  57| 			(!this.enabled || !selected) ?
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 0.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|  52|  52| 	{
|  53|  53| 		let selectedType = this.gameRatingFilter.list_data[this.gameRatingFilter.selected];
|  54|  54| 		let selectedRating = +selectedType.substr(1)
|  55|    |-warn(1234)
|    |  55|+		warn(1234)
|  56|  56| 		this.filter =
|  57|  57| 			(!this.enabled || !selected) ?
|  58|  58| 				() => true :
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|  52|  52| 	{
|  53|  53| 		let selectedType = this.gameRatingFilter.list_data[this.gameRatingFilter.selected];
|  54|  54| 		let selectedRating = +selectedType.substr(1)
|  55|    |-warn(1234)
|    |  55|+warn(1234);
|  56|  56| 		this.filter =
|  57|  57| 			(!this.enabled || !selected) ?
|  58|  58| 				() => true :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|  56|  56| 		this.filter =
|  57|  57| 			(!this.enabled || !selected) ?
|  58|  58| 				() => true :
|  59|    |-			selectedType.startsWith(">") ?
|    |  59|+				selectedType.startsWith(">") ?
|  60|  60| 				game => game.gameRating >= selectedRating :
|  61|  61| 				game => game.gameRating < selectedRating;
|  62|  62| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|  57|  57| 			(!this.enabled || !selected) ?
|  58|  58| 				() => true :
|  59|  59| 			selectedType.startsWith(">") ?
|  60|    |-				game => game.gameRating >= selectedRating :
|    |  60|+					game => game.gameRating >= selectedRating :
|  61|  61| 				game => game.gameRating < selectedRating;
|  62|  62| 
|  63|  63| 		this.onFilterChange();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|  58|  58| 				() => true :
|  59|  59| 			selectedType.startsWith(">") ?
|  60|  60| 				game => game.gameRating >= selectedRating :
|  61|    |-				game => game.gameRating < selectedRating;
|    |  61|+					game => game.gameRating < selectedRating;
|  62|  62| 
|  63|  63| 		this.onFilterChange();
|  64|  64| 	}

binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|  54| »   »   let·selectedRating·=·+selectedType.substr(1)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|  55| warn(1234)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|  67|  67| ChatCommandHandler.prototype.ChatCommands = {
|  68|  68| 	"away": {
|  69|  69| 		"description": translate("Set your state to 'Away'."),
|  70|    |-		"handler": function (args) {
|    |  70|+		"handler": function(args) {
|  71|  71| 			Engine.LobbySetPlayerPresence("away");
|  72|  72| 		}
|  73|  73| 	},
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|  73|  73| 	},
|  74|  74| 	"back": {
|  75|  75| 		"description": translate("Set your state to 'Online'."),
|  76|    |-		"handler": function (args) {
|    |  76|+		"handler": function(args) {
|  77|  77| 			Engine.LobbySetPlayerPresence("available");
|  78|  78| 		}
|  79|  79| 	},
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|  79|  79| 	},
|  80|  80| 	"kick": {
|  81|  81| 		"description": translate("Kick a specified user from the lobby. Usage: /kick nick reason"),
|  82|    |-		"handler": function (args) {
|    |  82|+		"handler": function(args) {
|  83|  83| 			Engine.LobbyKick(args[0] || "", args[1] || "");
|  84|  84| 		},
|  85|  85| 		"moderatorOnly": true
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|  86|  86| 	},
|  87|  87| 	"ban": {
|  88|  88| 		"description": translate("Ban a specified user from the lobby. Usage: /ban nick reason"),
|  89|    |-		"handler": function (args) {
|    |  89|+		"handler": function(args) {
|  90|  90| 			Engine.LobbyBan(args[0] || "", args[1] || "");
|  91|  91| 		},
|  92|  92| 		"moderatorOnly": true
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|  93|  93| 	},
|  94|  94| 	"help": {
|  95|  95| 		"description": translate("Show this help."),
|  96|    |-		"handler": function (args) {
|    |  96|+		"handler": function(args) {
|  97|  97| 			let isModerator = Engine.LobbyGetPlayerRole(g_Username) == "moderator";
|  98|  98| 			let text = translate("Chat commands:");
|  99|  99| 			for (let command in this.ChatCommands)
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
| 116| 116| 	},
| 117| 117| 	"clear": {
| 118| 118| 		"description": translate("Clear all chat scrollback."),
| 119|    |-		"handler": function (args) {
|    | 119|+		"handler": function(args) {
| 120| 120| 			this.chatMessagesPanel.clearChatMessages();
| 121| 121| 		}
| 122| 122| 	}
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/TimestampWrapper.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/TimestampWrapper.js
|  12|  12| 	format(timestamp, text)
|  13|  13| 	{
|  14|  14| 		this.timeArgs.time =
|  15|    |-			Engine.FormatMillisecondsIntoDateStringLocal(timestamp * 1000, this.TimeFormat)
|    |  15|+			Engine.FormatMillisecondsIntoDateStringLocal(timestamp * 1000, this.TimeFormat);
|  16|  16| 
|  17|  17| 		this.timestampArgs.time = sprintf(this.TimestampFormat, this.timeArgs);
|  18|  18| 		this.timestampArgs.message = text;

binaries/data/mods/public/gui/lobby/LobbyPage/Chat/TimestampWrapper.js
|  15| »   »   »   Engine.FormatMillisecondsIntoDateStringLocal(timestamp·*·1000,·this.TimeFormat)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1115/display/redirect

Freagarach added inline comments.
binaries/data/mods/public/gui/lobby/LobbyPage/AnnouncementHandler.js
14

+.

binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
40

Leaving us in tension ;)

102

Do these need a . as well?

binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatMessages/ChatMessageEvents.js
22

+.

24

S+.

binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatMessages/ChatMessageFormat.js
7

+.

19

+.

26

+.

29

+.

36

+.

binaries/data/mods/public/gui/lobby/LobbyPage/Chat/StatusMessages/StatusMessageEvents.js
4

So they're not events that demand the current players attention? Or are they neither chat messages, nor events. But they do demand the current players attention?
(Context explains it all, but still.)

40

+.

71

Kind of a strange name it seems, in respect of the other names chosen?

126

Can't go from mod to visitor? ;)

binaries/data/mods/public/gui/lobby/LobbyPage/Chat/StatusMessages/StatusMessageFormat.js
24

+.

binaries/data/mods/public/gui/lobby/LobbyPage/Chat/SystemMessages/SystemMessageEvents.js
16–17

+.*2

binaries/data/mods/public/gui/lobby/LobbyPage/Chat/SystemMessages/SystemMessageFormat.js
24

+.

binaries/data/mods/public/gui/lobby/LobbyPage/ConnectionHandler.js
26

+.

49

+.

binaries/data/mods/public/gui/lobby/LobbyPage/GameDetails.js
17

What does sg stand for if I may ask?

binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
28

+.

146

+.

204

+.

332

+.

335

+.

binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/PlayerCount.js
9

Comment on top?

binaries/data/mods/public/gui/lobby/LobbyPage/PlayerList.js
14

+.

68

+.

binaries/data/mods/public/gui/lobby/LobbyPage/ProfilePanel.js
10

+.

13

+.

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

+.

elexis updated this revision to Diff 10356.Nov 18 2019, 5:42 PM

Implement class Game.
Might contain some possible duplication to be attempted to be removed and profiling code.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/603/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lobby/scripting/JSInterface_Lobby.h
|  26| namespace·JSI_Lobby
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceJSI_Lobby{' is invalid C code. Use --std or --language to configure the language.

source/lobby/IXmppClient.h
|  24| namespace·StunClient·{
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceStunClient{' is invalid C code. Use --std or --language to configure the language.

source/lobby/XmppClient.h
|  24| 
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceStunClient{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|  51|  51| 	onSelectionChange()
|  52|  52| 	{
|  53|  53| 		let selectedType = this.gameRatingFilter.list_data[this.gameRatingFilter.selected];
|  54|    |-		let selectedRating = +selectedType.substr(1)
|    |  54|+		let selectedRating = +selectedType.substr(1);
|  55|  55| 
|  56|  56| 		this.filter =
|  57|  57| 			(!this.enabled || !selected) ?
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|  56|  56| 		this.filter =
|  57|  57| 			(!this.enabled || !selected) ?
|  58|  58| 				() => true :
|  59|    |-			selectedType.startsWith(">") ?
|    |  59|+				selectedType.startsWith(">") ?
|  60|  60| 				game => game.gameRating >= selectedRating :
|  61|  61| 				game => game.gameRating < selectedRating;
|  62|  62| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|  57|  57| 			(!this.enabled || !selected) ?
|  58|  58| 				() => true :
|  59|  59| 			selectedType.startsWith(">") ?
|  60|    |-				game => game.gameRating >= selectedRating :
|    |  60|+					game => game.gameRating >= selectedRating :
|  61|  61| 				game => game.gameRating < selectedRating;
|  62|  62| 
|  63|  63| 		this.onFilterChange();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|  58|  58| 				() => true :
|  59|  59| 			selectedType.startsWith(">") ?
|  60|  60| 				game => game.gameRating >= selectedRating :
|  61|    |-				game => game.gameRating < selectedRating;
|    |  61|+					game => game.gameRating < selectedRating;
|  62|  62| 
|  63|  63| 		this.onFilterChange();
|  64|  64| 	}

binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/Rating.js
|  54| »   »   let·selectedRating·=·+selectedType.substr(1)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/XmppMessages.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/XmppMessages.js
|  16|  16| 		this.messageBatchProcessedHandlers = new Set();
|  17|  17| 
|  18|  18| 		Engine.GetGUIObjectByName("lobbyPage").onTick =
|  19|    |-			this.handleMessages.bind(this, Engine.LobbyGuiPollNewMessages)
|    |  19|+			this.handleMessages.bind(this, Engine.LobbyGuiPollNewMessages);
|  20|  20| 	}
|  21|  21| 
|  22|  22| 	registerHandler(type, level, handler)
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/XmppMessages.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/XmppMessages.js
|  48|  48| 			let z = Engine.GetMicroseconds();
|  49|  49| 			for (let handler of this.messageHandlers.game.gamelist)
|  50|  50| 				handler(null, gameList);
|  51|    |-			//warn("GameList update with games took " + ((Engine.GetMicroseconds() - z) / 1000))
|    |  51|+			// warn("GameList update with games took " + ((Engine.GetMicroseconds() - z) / 1000))
|  52|  52| 		}
|  53|  53| 	}
|  54|  54| 

binaries/data/mods/public/gui/lobby/XmppMessages.js
|  19| »   »   »   this.handleMessages.bind(this,·Engine.LobbyGuiPollNewMessages)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
|  82|  82| 	{
|  83|  83| 		Engine.ProfileStart("rebuildGameList");
|  84|  84| 
|  85|    |-		let selectedGame = this.selectedGame()
|    |  85|+		let selectedGame = this.selectedGame();
|  86|  86| 
|  87|  87| 		let gameListData = testGameList || this.lastTestGame; // Engine.GetGameList();
|  88|  88| 		this.lastTestGame = gameListData;
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
|  99|  99| 				if (!exists)
| 100| 100| 				{
| 101| 101| 					game = new Game();
| 102|    |-					//warn("Creating game");
|    | 102|+					// warn("Creating game");
| 103| 103| 				}
| 104| 104| 				//else
| 105| 105| 					//warn("Updating existing game");
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
| 101| 101| 					game = new Game();
| 102| 102| 					//warn("Creating game");
| 103| 103| 				}
| 104|    |-				//else
|    | 104|+				// else
| 105| 105| 					//warn("Updating existing game");
| 106| 106| 				game.update(gameData, selectedColumn);
| 107| 107| 				newGames[gameData.ip] = game;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 5.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
| 102| 102| 					//warn("Creating game");
| 103| 103| 				}
| 104| 104| 				//else
| 105|    |-					//warn("Updating existing game");
|    | 105|+				//warn("Updating existing game");
| 106| 106| 				game.update(gameData, selectedColumn);
| 107| 107| 				newGames[gameData.ip] = game;
| 108| 108| 			}
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
| 102| 102| 					//warn("Creating game");
| 103| 103| 				}
| 104| 104| 				//else
| 105|    |-					//warn("Updating existing game");
|    | 105|+					// warn("Updating existing game");
| 106| 106| 				game.update(gameData, selectedColumn);
| 107| 107| 				newGames[gameData.ip] = game;
| 108| 108| 			}
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
| 117| 117| 			{
| 118| 118| 				let game = this.games[ip];
| 119| 119| 				if (this.filters.every(filter => filter.filter(game)))
| 120|    |-					this.gameList.push(game)
|    | 120|+					this.gameList.push(game);
| 121| 121| 			}
| 122| 122| 			Engine.ProfileStop();
| 123| 123| 		}

binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
| 113| »   »   {
|    | [NORMAL] ESLintBear (no-lone-blocks):
|    | Nested block is redundant.

binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
| 125| »   »   {
|    | [NORMAL] ESLintBear (no-lone-blocks):
|    | Nested block is redundant.

binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
|  85| »   »   let·selectedGame·=·this.selectedGame()
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
| 120| »   »   »   »   »   this.gameList.push(game)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|  67|  67| ChatCommandHandler.prototype.ChatCommands = {
|  68|  68| 	"away": {
|  69|  69| 		"description": translate("Set your state to 'Away'."),
|  70|    |-		"handler": function (args) {
|    |  70|+		"handler": function(args) {
|  71|  71| 			Engine.LobbySetPlayerPresence("away");
|  72|  72| 		}
|  73|  73| 	},
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|  73|  73| 	},
|  74|  74| 	"back": {
|  75|  75| 		"description": translate("Set your state to 'Online'."),
|  76|    |-		"handler": function (args) {
|    |  76|+		"handler": function(args) {
|  77|  77| 			Engine.LobbySetPlayerPresence("available");
|  78|  78| 		}
|  79|  79| 	},
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|  79|  79| 	},
|  80|  80| 	"kick": {
|  81|  81| 		"description": translate("Kick a specified user from the lobby. Usage: /kick nick reason"),
|  82|    |-		"handler": function (args) {
|    |  82|+		"handler": function(args) {
|  83|  83| 			Engine.LobbyKick(args[0] || "", args[1] || "");
|  84|  84| 		},
|  85|  85| 		"moderatorOnly": true
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|  86|  86| 	},
|  87|  87| 	"ban": {
|  88|  88| 		"description": translate("Ban a specified user from the lobby. Usage: /ban nick reason"),
|  89|    |-		"handler": function (args) {
|    |  89|+		"handler": function(args) {
|  90|  90| 			Engine.LobbyBan(args[0] || "", args[1] || "");
|  91|  91| 		},
|  92|  92| 		"moderatorOnly": true
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|  93|  93| 	},
|  94|  94| 	"help": {
|  95|  95| 		"description": translate("Show this help."),
|  96|    |-		"handler": function (args) {
|    |  96|+		"handler": function(args) {
|  97|  97| 			let isModerator = Engine.LobbyGetPlayerRole(g_Username) == "moderator";
|  98|  98| 			let text = translate("Chat commands:");
|  99|  99| 			for (let command in this.ChatCommands)
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
| 116| 116| 	},
| 117| 117| 	"clear": {
| 118| 118| 		"description": translate("Clear all chat scrollback."),
| 119|    |-		"handler": function (args) {
|    | 119|+		"handler": function(args) {
| 120| 120| 			this.chatMessagesPanel.clearChatMessages();
| 121| 121| 		}
| 122| 122| 	}
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/TimestampWrapper.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/TimestampWrapper.js
|  12|  12| 	format(timestamp, text)
|  13|  13| 	{
|  14|  14| 		this.timeArgs.time =
|  15|    |-			Engine.FormatMillisecondsIntoDateStringLocal(timestamp * 1000, this.TimeFormat)
|    |  15|+			Engine.FormatMillisecondsIntoDateStringLocal(timestamp * 1000, this.TimeFormat);
|  16|  16| 
|  17|  17| 		this.timestampArgs.time = sprintf(this.TimestampFormat, this.timeArgs);
|  18|  18| 		this.timestampArgs.message = text;

binaries/data/mods/public/gui/lobby/LobbyPage/Chat/TimestampWrapper.js
|  15| »   »   »   Engine.FormatMillisecondsIntoDateStringLocal(timestamp·*·1000,·this.TimeFormat)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Game.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Game.js
| 225| 225| 
| 226| 226| 	getTranslatedPlayerCount(newStanza)
| 227| 227| 	{
| 228|    |-		let playerCountArgs = this.playerCountArgs
|    | 228|+		let playerCountArgs = this.playerCountArgs;
| 229| 229| 		playerCountArgs.current = setStringTags(escapeText(newStanza.nbp), this.PlayerCountTags.CurrentPlayers);
| 230| 230| 		playerCountArgs.max = setStringTags(escapeText(newStanza.maxnbp), this.PlayerCountTags.MaxPlayers);
| 231| 231| 

binaries/data/mods/public/gui/lobby/LobbyPage/Game.js
| 150| »   »   {
|    | [NORMAL] ESLintBear (no-lone-blocks):
|    | Nested block is redundant.

binaries/data/mods/public/gui/lobby/LobbyPage/Game.js
| 195| »   »   {
|    | [NORMAL] ESLintBear (no-lone-blocks):
|    | Nested block is redundant.

binaries/data/mods/public/gui/lobby/LobbyPage/Game.js
| 228| »   »   let·playerCountArgs·=·this.playerCountArgs
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1119/display/redirect

The previously described class Game implementation was performed and it seems indeed to be the right thing as it is both more performant (performance doubled?) and if performance is irrelevant,
then it's still advantageous as one will be able to trigger event handlers such as a new game being registered, a game having been removed, a buddy having joined a game.

We can see that the approach works as intended and that the code isn't doing those slices more performant, it's just not doing them anymore most of the time.
See that there are only two profile slices in updateGames:
Which means that in the entire new gamelist, only a single game has changed, and that had changed only one property (players):


Compare this to the previous code where every property of every game was recomputed:

For the performance, notice the orange line being the new performance using the cache. That's 1.9614296875ms versus 0.942375ms spent per gamelist update, thats about 50% faster, and those 40% other percent was the copying of the column arrays to C++.


Note for the above graph, the worst case scenario is a hypothetical scenario where every gamesetting of every host changed in every gamelist update.

XPartaMupp.py should probably be changed to only send updates for individual games rather than broadcasting an entire list.
Then this class would probably still make it a bit faster since most of the time only few properties change (due to the fact that human players dont change different settings so frequently and there are few if not no gamesetting types that change more than one or two attributes).

The next step is to consider implementing the Player class as well.

Notice that we already had some sort of playerlist cache implementation that was truly terrible, see removal in #3386 / rP16997.
That stateful approach was broken because it applied the state incorrectly (first replaced the playerlist with the most recent playerlist, then applied historic presence updates to the list from before that list state, thus ending up with players appearing twice or more often in the list, or sometimes not appearing in the list despite being online),
and secondly the stateful approach was terrible because these state updated on tons of historic presence changes froze the GUI page for 10 seconds.
At the time there was a decision made that its fast enough to just rebuild the entire playerlist and remove the cache.
This seems probably largely right, but one could also implement it in a performant and logically correct way as well.

On the other side there is no benefit in terms of to new events gained, because we already have events when a player joined / left / was kicked from the room, changed presence or buddy state.
Perhaps an individual player changing rating is not implemented yet? That should not rely on the bot posting it in chat.

binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
102

If they do, I won't add them in this commit, so as to minimize string changes.
There already are a few of string changes because I had to remove the translation comments "IRC message" due to its terribility.

Also when I look at the "description" fields just below, no, theres a period in every description where applicable.

binaries/data/mods/public/gui/lobby/LobbyPage/Chat/StatusMessages/StatusMessageEvents.js
71

yes, can rename

126

Then it shows "player has been muted"

binaries/data/mods/public/gui/lobby/LobbyPage/GameDetails.js
17

It's there since rP14098 and I had wondered since I saw the code the first time five years ago.
Only while writing this diff I came to the realization that it probably means selectedGame.
And that's why we never never use abbreviations if we only have to type few more characters to make it explicit.

binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
28

But this is not a sentence and not intended as one, more a half-sentence

Thanks for the intent to help, but please not 20 inline comments for that, one is sufficient if I'm not convinced to add these in general.

(First I would have to become convinced of ending half sentences with a period or changing all half sentences to include complete context to form a full sentence qualifying the period, then I can identify the 20+ places too. With the 31 inline comments posted I would have to click more than 31 times to cycle through all of them with this web UI, but I don't intend to repeat the answer 20+ times in 30 of the comments, so this cycle button is basically 1/3 as useful as it was before)

binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/PlayerCount.js
9

y thx

  • Implemented another TODO from rP14098 by displaying the victory conditions.
  • Implemented display of mods if the game has different mods launched.
  • Display gamename in selection
  • Removed Game class duplication



binaries/data/mods/public/gui/lobby/LobbyPage/GameListFilters/PlayerCount.js
9

actually none of the gamelist filters have a comment, and I dont know what to write there that isnt already obvious enough from this small class. There is already this comment for the container class:

/**
 * Each property of this class handles one specific map filter and is defined in external files.
 */
class GameListFilters
{
}
elexis updated this revision to Diff 10357.Nov 19 2019, 12:52 AM

Remove Game class duplication, display victory conditions, mods, hostname in GameDetails.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/604/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lobby/IXmppClient.h
|  24| namespace·StunClient·{
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceStunClient{' is invalid C code. Use --std or --language to configure the language.

source/lobby/XmppClient.h
|  24| 
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceStunClient{' is invalid C code. Use --std or --language to configure the language.

source/lobby/scripting/JSInterface_Lobby.h
|  26| namespace·JSI_Lobby
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceJSI_Lobby{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/TimestampWrapper.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/TimestampWrapper.js
|  12|  12| 	format(timestamp, text)
|  13|  13| 	{
|  14|  14| 		this.timeArgs.time =
|  15|    |-			Engine.FormatMillisecondsIntoDateStringLocal(timestamp * 1000, this.TimeFormat)
|    |  15|+			Engine.FormatMillisecondsIntoDateStringLocal(timestamp * 1000, this.TimeFormat);
|  16|  16| 
|  17|  17| 		this.timestampArgs.time = sprintf(this.TimestampFormat, this.timeArgs);
|  18|  18| 		this.timestampArgs.message = text;

binaries/data/mods/public/gui/lobby/LobbyPage/Chat/TimestampWrapper.js
|  15| »   »   »   Engine.FormatMillisecondsIntoDateStringLocal(timestamp·*·1000,·this.TimeFormat)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
| 107| »   »   {
|    | [NORMAL] ESLintBear (no-lone-blocks):
|    | Nested block is redundant.

binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
| 119| »   »   {
|    | [NORMAL] ESLintBear (no-lone-blocks):
|    | Nested block is redundant.

binaries/data/mods/public/gui/lobby/LobbyPage/Game.js
| 145| »   »   {
|    | [NORMAL] ESLintBear (no-lone-blocks):
|    | Nested block is redundant.

binaries/data/mods/public/gui/lobby/LobbyPage/Game.js
| 193| »   »   {
|    | [NORMAL] ESLintBear (no-lone-blocks):
|    | Nested block is redundant.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1120/display/redirect

Freagarach added inline comments.Nov 19 2019, 9:28 AM
binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
28

You're right, sorry about the inline-spam.
(You know you can hide them and they won't be cycled then?)

elexis edited the test plan for this revision. (Show Details)Nov 19 2019, 10:26 AM
elexis edited the summary of this revision. (Show Details)
elexis edited the test plan for this revision. (Show Details)Nov 19 2019, 10:31 AM
Stan added a subscriber: Stan.Nov 19 2019, 4:00 PM

I guess this supersedes D1136

elexis added a comment.EditedNov 20 2019, 11:24 AM

Further rediscovered bugs:

  • #5638 for the COList missing features (column centering, only updating one line instead of all lines, obtaining positions of columns to align filters)
  • Rated game not updating playerlist: #4621, seems to be a serverside bug, not clientside.
  • IRC Removal in D1136

There still is some noticeable delay between update of playerlist, chat and gamelist when initially opening the page on a busy day, in particular multiple playermessages upon init updating the list multiple times with different sets of players and order, that would be good to fix. Edit: Appears like the playerListUpdate calls are on consecutive frames and occur on various events (init, ratinglist, playerlist).

binaries/data/mods/public/gui/page_lobby.xml
12

D148, without this change the FPS counter background will be intransparently black.

Analysis of initial playerList performance:
TLDR: Delay GUI messages in XmppClient.cpp until m_initialLoadComplete.

As mentioned the PlayerList should be checked a little bit more, and that there is a phenomenon where logging into the lobby will noticably take something in the order of magnitude of a second to fill all GUI objects with their appropriate values.

While testing the diff in the arena23b lobby, I could see that the page build up is much faster, certainly not lagging.

But it is still noticeable that the playerlist is changing about 5 times on init until it receives its final value.

It happens so quickly that its kind of irrelevant, but as it is noticeable, it becomes bothersome as one can't "unsee" it.

By adding a warn to rebuildPlayerList, we see that there are sometimes 5 to 6 calls within the first few frames of the lobby page!

By setting XmppClient.cpp DbgXMPP to 1, printing a warn for every tick, every message type processed and a stack trace showing what triggered the playerlist update we see what triggers those updatePlayerList calls and how many of them:

TIMER| common/global.xml: 304.425 us
WARNING: rebuildPlayerList@gui/lobby/LobbyPage/PlayerList.js:107:8
PlayerList@gui/lobby/LobbyPage/PlayerList.js:34:3
LobbyPage@gui/lobby/LobbyPage/LobbyPage.js:12:20
Lobby@gui/lobby/Lobby.js:12:20
init@gui/lobby/lobby.js:38:13
onLogin@gui/prelobby/login/login.js:48:1
onTick@gui/prelobby/common/feedback/feedback.js:26:4
__eventhandler14 (tick)@__internal(15) tick:0:1

vinme is in the room, presence: playing, role: visitor
Lagos is in the room, presence: playing, role: participant
Berger is in the room, presence: available, role: participant
manu9119 is in the room, presence: playing, role: participant
bsodjunkie is in the room, presence: playing, role: participant
defc0n is in the room, presence: playing, role: participant
AMAXIA is in the room, presence: playing, role: participant
Dunedan is in the room, presence: available, role: participant
Talenters is in the room, presence: playing, role: participant
YilmazGNG is in the room, presence: playing, role: participant
JabxD3 is in the room, presence: playing, role: participant
elexis is in the room, presence: playing, role: moderator
CAGD_lulofun is in the room, presence: playing, role: participant
Havran is in the room, presence: playing, role: participant
layuca is in the room, presence: playing, role: participant
Listo is in the room, presence: playing, role: participant
WARNING: tick
WARNING: rebuildPlayerList@gui/lobby/LobbyPage/PlayerList.js:107:8
onTick@gui/lobby/XmppMessages.js:59:5

revenantos is in the room, presence: playing, role: participant
user1 is in the room, presence: away, role: moderator
randomid is in the room, presence: playing, role: participant
Ratings is in the room, presence: available, role: participant
M.T is in the room, presence: playing, role: participant
CopxD is in the room, presence: playing, role: participant
animopolis is in the room, presence: playing, role: participant
Wollolooow is in the room, presence: playing, role: participant
camel_case is in the room, presence: available, role: participant
Obi is in the room, presence: playing, role: participant
Amiru is in the room, presence: playing, role: participant
WFGbot is in the room, presence: available, role: participant
snelius is in the room, presence: playing, role: participant
aow is in the room, presence: playing, role: participant
nani is in the room, presence: playing, role: participant
DenSpeciellaGrabben is in the room, presence: playing, role: participant
Hannon600BC is in the room, presence: playing, role: participant
B_Smoke is in the room, presence: playing, role: participant
whoisit is in the room, presence: playing, role: participant
WARNING: tick
WARNING: rebuildPlayerList@gui/lobby/LobbyPage/PlayerList.js:107:8
onTick@gui/lobby/XmppMessages.js:59:5

bscoter is in the room, presence: playing, role: participant
halamadrid2 is in the room, presence: playing, role: participant
gringo_axel is in the room, presence: playing, role: participant
bobert is in the room, presence: available, role: participant
xiangxi is in the room, presence: playing, role: participant
hsynersoy is in the room, presence: playing, role: participant
Juleferie is in the room, presence: playing, role: participant
Unknown_Player is in the room, presence: playing, role: participant
axi is in the room, presence: playing, role: participant
RichterAids is in the room, presence: playing, role: participant
borg- is in the room, presence: playing, role: participant
PETRUS1985 is in the room, presence: playing, role: participant
puturrudufu is in the room, presence: available, role: participant
Gianni.m is in the room, presence: playing, role: participant
surplus is in the room, presence: available, role: participant
xtreme022 is in the room, presence: playing, role: participant
Troll19 is in the room, presence: playing, role: participant
Carthage is in the room, presence: playing, role: participant
WARNING: tick
WARNING: rebuildPlayerList@gui/lobby/LobbyPage/PlayerList.js:107:8
onTick@gui/lobby/XmppMessages.js:59:5

n08212000 is in the room, presence: playing, role: participant
goldafternoonfix is in the room, presence: playing, role: participant
kikoojap is in the room, presence: available, role: participant
Bakixeddu is in the room, presence: playing, role: participant
elexis2 is in the room, presence: available, role: participant
RichterAids said : - 0 
RichterAids said no
LipeBr said '-'
Revon said ;lol;
RichterAids said : - 0
RichterAids said i got banned
xiangxi said ?
RichterAids said : - 0
bubi2014 said lol
bubi2014 said quit noobi
RichterAids said : - )
LipeBr said shit
WARNING: tick
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: rebuildPlayerList@gui/lobby/LobbyPage/PlayerList.js:107:8
onTick@gui/lobby/XmppMessages.js:59:5

xiangxi said host again
kikoojap said yo les amiches
mralex said Can't join axi.
Ratings said A rated game has ended. gifox lost against crazynoob. Rating Adjustment: gifox (1070 -> 1059) and crazynoob (1247 -> 1272).
axi said some nice peoppllle join 41% randomizsation game
nani said axi yes
elexis said nani: there are like 6 updatePlayerList calls when joining the lobby, on consecutive ticks how did you solve that?
nani said i didnt touch playerlist in my mod
WARNING: tick
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat room-message
WARNING: msg chat subject
WARNING: msg chat subject
WARNING: tick
handleIq [<iq to='elexis2@lobby.wildfiregames.com/0ad' from='wfgbot23b@lobby.wildfiregames.com/CC' id='425f7a96-cf5f-4838-9103-1dc6c1154905-465BA' type='result'><query xmlns='jabber:iq:gamelist'>....</query></iq>]
elexis2 is in the room, presence: available, role: participant
handleIq [<iq to='elexis2@lobby.wildfiregames.com/0ad' from='wfgbot23b@lobby.wildfiregames.com/CC' id='425f7a96-cf5f-4838-9103-1dc6c1154905-465BB' type='result'><query xmlns='jabber:iq:boardlist'><command>ratinglist</command><board name='elexis' rating='1129'/>...</query></iq>]
WARNING: tick
WARNING: msg game gamelist
WARNING: rebuildPlayerList@gui/lobby/LobbyPage/PlayerList.js:107:8
onTick@gui/lobby/XmppMessages.js:59:5

WARNING: tick
WARNING: tick
WARNING: tick
...

So the lobby server doesnt send a complete playerlist but batches of about 20 players. So if there are 100 players online, there will be 5 of these messages.

This can be perfectly fixed by delaying GUI messages in the XmppClient until m_initialLoadComplete = true, that is the last message of the batch.

While at it, I noticed that 35% of the gamelist stanza seems to be the xml string &quot;; that's humongous and it can't be solved unless using zlib stream compression https://xmpp.org/extensions/xep-0138.html (TLS compression seems to have been dropped in TLS 1.3).

It would probably be good if the playerlist and gamelist both would have this cache optimization, as it is now a bit inconsistent. On the other hand there are more urgent issues than saving some microseconds here, and the first and foremost performance optimization should be the C++ OList assignment of an individual row instead of all rows (#5638). Then the cache might be obsolete again as well. (However the Game class cache is useful even without performance consideration to trigger gamechange events).

elexis updated this revision to Diff 10368.Nov 20 2019, 6:03 PM
elexis edited the test plan for this revision. (Show Details)

Patch update:

Tested every line of the patch and fixed some breakage in this patch and some more stuff that was broken in commit history.

New features:

  • Add Connection status chat notification, since this can actually happen after reconnecting upon disconnect

Open:

  • There is still D2288 outstanding, which is why announcements still show up as private messages without sender. Won't fix in this patch.

Fixed in commit history:

  • Only one playerlist GUI update until m_initialLoadComplete
  • Fix music following rP20886.

Fixed in this uncommitted patch:

  • Fix missing semicolons
  • Delete debug code, keep Profiling code
  • Fix chat commands and nickname highighting
  • Fix playerlist update after buddytoggle
  • Fix reconnect question being triggered multiple times depending on usage of hypothetical, or Js console, or mod- initiated reconnects during session

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/610/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lobby/IXmppClient.h
|  24| namespace·StunClient·{
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceStunClient{' is invalid C code. Use --std or --language to configure the language.

source/lobby/scripting/JSInterface_Lobby.h
|  26| namespace·JSI_Lobby
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceJSI_Lobby{' is invalid C code. Use --std or --language to configure the language.

source/lobby/XmppClient.h
|  24| 
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceStunClient{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/Chat/ChatCommandHandler.js
|  43|  43| 				this.systemMessageFormat.format(
|  44|  44| 					sprintf(translate("The command '%(cmd)s' is restricted to moderators."), {
|  45|  45| 						"cmd": setStringTags(escapeText(command), this.ChatCommandTags)
|  46|    |-				})));
|    |  46|+					})));
|  47|  47| 			this.chatMessagesPanel.flushMessages();
|  48|  48| 			return true;
|  49|  49| 		}

binaries/data/mods/public/gui/lobby/LobbyPage/GameList.js
| 110| »   »   {
|    | [NORMAL] ESLintBear (no-lone-blocks):
|    | Nested block is redundant.

binaries/data/mods/public/gui/lobby/LobbyPage/Game.js
| 153| »   »   {
|    | [NORMAL] ESLintBear (no-lone-blocks):
|    | Nested block is redundant.

binaries/data/mods/public/gui/lobby/LobbyPage/Game.js
| 202| »   »   {
|    | [NORMAL] ESLintBear (no-lone-blocks):
|    | Nested block is redundant.
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/ConnectionHandler.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/LobbyPage/ConnectionHandler.js
|  35|  35| 
|  36|  36| 		// The current player has been kicked from the room, not from the server
|  37|  37| 		Engine.DisconnectXmppClient();
|  38|    |-		mbWidth, mbHeight, mbMessage, mbTitle, mbButtonCaptions, mbBtnCode, mbCallbackArgs
|    |  38|+		mbWidth, mbHeight, mbMessage, mbTitle, mbButtonCaptions, mbBtnCode, mbCallbackArgs;
|  39|  39| 		messageBox(
|  40|  40| 			400, 250,
|  41|  41| 			new KickStrings().get(banned, message),

binaries/data/mods/public/gui/lobby/LobbyPage/ConnectionHandler.js
|  38| »   »   mbWidth,·mbHeight,·mbMessage,·mbTitle,·mbButtonCaptions,·mbBtnCode,·mbCallbackArgs
|    | [NORMAL] ESLintBear (no-unused-expressions):
|    | Expected an assignment or function call and instead saw an expression.

binaries/data/mods/public/gui/lobby/LobbyPage/ConnectionHandler.js
|  38| »   »   mbWidth,·mbHeight,·mbMessage,·mbTitle,·mbButtonCaptions,·mbBtnCode,·mbCallbackArgs
|    | [NORMAL] JSHintBear:
|    | Expected an assignment or function call and instead saw an expression.

binaries/data/mods/public/gui/lobby/LobbyPage/ConnectionHandler.js
|  38| »   »   mbWidth,·mbHeight,·mbMessage,·mbTitle,·mbButtonCaptions,·mbBtnCode,·mbCallbackArgs
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1126/display/redirect

Silier added a subscriber: Silier.Nov 21 2019, 8:51 AM
Silier added inline comments.
binaries/data/mods/public/gui/lobby/LeaderboardPage/LeaderboardList.js
60

boardList ?

Improvements of committed code:

  • g_Username -> g_Nickname
  • Update local nickname on nickchange

Fix of this version of the patch:

  • Fix rating filter
  • Endless game victory condition
  • Profile panel playername is to be not colorized
This revision was not accepted when it landed; it landed in state Needs Review.Nov 21 2019, 2:45 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Nov 21 2019, 2:45 PM