Changeset View
Standalone View
binaries/data/mods/public/gui/lobby/lobby.js
Show First 20 Lines • Show All 75 Lines • ▼ Show 20 Lines | |||||
var g_SenderFont = "sans-bold-13"; | var g_SenderFont = "sans-bold-13"; | ||||
/** | /** | ||||
* Color to highlight chat commands in the explanation. | * Color to highlight chat commands in the explanation. | ||||
*/ | */ | ||||
var g_ChatCommandColor = "200 200 255"; | var g_ChatCommandColor = "200 200 255"; | ||||
/** | /** | ||||
* Color for the player count number in the games list. | |||||
*/ | |||||
var g_ObserverCountTags = { "color": "gray" }; | |||||
elexis: Specifying colors and fonts individually is deprecated (D2151), specify the object directly… | |||||
Done Inline Actionsgood elexis: good | |||||
/** | |||||
* Indicates if the lobby is opened as a dialog or window. | * Indicates if the lobby is opened as a dialog or window. | ||||
*/ | */ | ||||
var g_Dialog = false; | var g_Dialog = false; | ||||
/** | /** | ||||
* All chat messages received since init (i.e. after lobby join and after returning from a game). | * All chat messages received since init (i.e. after lobby join and after returning from a game). | ||||
Not Done Inline ActionsFor me, this color is too dark on our current background. I cannot see the text without hurting my eyes (or my monitor). I could not find any existing use of this color in the repository. The closest is a grey 96 96 96 in color.js, which is still brighter (though not by much). The existing 0 128 128 is also quite dark, but still barely within what I can comfortably decipher. Krinkle: For me, this color is too dark on our current background. I cannot see the text without hurting… | |||||
*/ | */ | ||||
var g_ChatMessages = []; | var g_ChatMessages = []; | ||||
/** | /** | ||||
* Rating of the current user. | * Rating of the current user. | ||||
* Contains the number or an empty string in case the user has no rating. | * Contains the number or an empty string in case the user has no rating. | ||||
*/ | */ | ||||
var g_UserRating = ""; | var g_UserRating = ""; | ||||
▲ Show 20 Lines • Show All 527 Lines • ▼ Show 20 Lines | function handleKick(banned, nick, reason, time, historic) | ||||
g_Kicked = true; | g_Kicked = true; | ||||
Engine.DisconnectXmppClient(); | Engine.DisconnectXmppClient(); | ||||
messageBox( | messageBox( | ||||
400, 250, | 400, 250, | ||||
kickString + "\n" + reason, | kickString + "\n" + reason, | ||||
banned ? translate("BANNED") : translate("KICKED") | banned ? translate("BANNED") : translate("KICKED") | ||||
Done Inline ActionsAlso if you/elexis want to minize, you could write something like this: game.observerNumbers = stringifiedTeamListToPlayerData(game.players)).filter(() => { return player.Team == "observer" }).length; vladislavbelov: Also if you/elexis want to minize, you could write something like this:
```
game. | |||||
); | ); | ||||
} | } | ||||
/** | /** | ||||
* Update the subject GUI object. | * Update the subject GUI object. | ||||
*/ | */ | ||||
Done Inline ActionsI think counting of observers shouldn't be here? Because this is a sorting or filter. vladislavbelov: I think counting of observers shouldn't be here? Because this is a sorting or filter.
So split… | |||||
Done Inline Actionsthis is minimize getting gamelist sorting filtering altering? <i can split> ffffffff: this is minimize getting gamelist sorting filtering altering? <i can split>
ive seen this… | |||||
function updateSubject(newSubject) | function updateSubject(newSubject) | ||||
{ | { | ||||
Engine.GetGUIObjectByName("subject").caption = newSubject; | Engine.GetGUIObjectByName("subject").caption = newSubject; | ||||
// If the subject is only whitespace, hide it and reposition the logo. | // If the subject is only whitespace, hide it and reposition the logo. | ||||
let subjectBox = Engine.GetGUIObjectByName("subjectBox"); | let subjectBox = Engine.GetGUIObjectByName("subjectBox"); | ||||
subjectBox.hidden = !newSubject.trim(); | subjectBox.hidden = !newSubject.trim(); | ||||
Show All 17 Lines | function updateToggleBuddy() | ||||
toggleBuddyButton.enabled = !!playerName && playerName != g_Username; | toggleBuddyButton.enabled = !!playerName && playerName != g_Username; | ||||
} | } | ||||
/** | /** | ||||
* Do a full update of the player listing, including ratings from cached C++ information. | * Do a full update of the player listing, including ratings from cached C++ information. | ||||
*/ | */ | ||||
function updatePlayerList() | function updatePlayerList() | ||||
{ | { | ||||
let playersBox = Engine.GetGUIObjectByName("playersBox"); | let playersBox = Engine.GetGUIObjectByName("playersBox"); | ||||
Done Inline ActionsThe filter closure can be further simplified given it's only a single expression: .filter(player => { return player.Team == "observer"}); .filter(player => player.Team === "observer"); Krinkle: The filter closure can be further simplified given it's only a single expression:
```lang=js
. | |||||
Done Inline ActionsHigher up in the code, the gamer.players string is also parsed. Rather than parsing it again, might be worth re-using that result here. Krinkle: Higher up in the code, the `gamer.players` string is also parsed. Rather than parsing it again… | |||||
let sortBy = playersBox.selected_column || "name"; | let sortBy = playersBox.selected_column || "name"; | ||||
let sortOrder = playersBox.selected_column_order || 1; | let sortOrder = playersBox.selected_column_order || 1; | ||||
let buddyStatusList = []; | let buddyStatusList = []; | ||||
let playerList = []; | let playerList = []; | ||||
let presenceList = []; | let presenceList = []; | ||||
let nickList = []; | let nickList = []; | ||||
let ratingList = []; | let ratingList = []; | ||||
let cleanPlayerList = Engine.GetPlayerList().map(player => { | let cleanPlayerList = Engine.GetPlayerList().map(player => { | ||||
player.isBuddy = g_Buddies.indexOf(player.name) != -1; | player.isBuddy = g_Buddies.indexOf(player.name) != -1; | ||||
return player; | return player; | ||||
}).sort((a, b) => { | }).sort((a, b) => { | ||||
let sortA, sortB; | let sortA, sortB; | ||||
let statusOrder = Object.keys(g_PlayerStatuses); | let statusOrder = Object.keys(g_PlayerStatuses); | ||||
let statusA = statusOrder.indexOf(a.presence) + a.name.toLowerCase(); | let statusA = statusOrder.indexOf(a.presence) + a.name.toLowerCase(); | ||||
let statusB = statusOrder.indexOf(b.presence) + b.name.toLowerCase(); | let statusB = statusOrder.indexOf(b.presence) + b.name.toLowerCase(); | ||||
Not Done Inline ActionsWhy different quotes? vladislavbelov: Why different quotes? | |||||
Not Done Inline Actionsno mean change. ffffffff: no mean change. | |||||
Not Done Inline ActionsAnd empty line? vladislavbelov: And empty line? | |||||
Not Done Inline Actionsno mean change. ffffffff: no mean change. | |||||
switch (sortBy) | switch (sortBy) | ||||
{ | { | ||||
case 'buddy': | case 'buddy': | ||||
sortA = (a.isBuddy ? 1 : 2) + statusA; | sortA = (a.isBuddy ? 1 : 2) + statusA; | ||||
sortB = (b.isBuddy ? 1 : 2) + statusB; | sortB = (b.isBuddy ? 1 : 2) + statusB; | ||||
break; | break; | ||||
case 'rating': | case 'rating': | ||||
sortA = +a.rating; | sortA = +a.rating; | ||||
▲ Show 20 Lines • Show All 268 Lines • ▼ Show 20 Lines | function updateGameList() | ||||
{ | { | ||||
g_SelectedGameIP = g_GameList[gamesBox.selected].ip; | g_SelectedGameIP = g_GameList[gamesBox.selected].ip; | ||||
g_SelectedGamePort = g_GameList[gamesBox.selected].port; | g_SelectedGamePort = g_GameList[gamesBox.selected].port; | ||||
} | } | ||||
g_GameList = Engine.GetGameList().map(game => { | g_GameList = Engine.GetGameList().map(game => { | ||||
game.hasBuddies = 0; | game.hasBuddies = 0; | ||||
game.observerCount = 0; | |||||
Not Done Inline Actions(FYI: I change this number locally to e.g. 1, 2 or 14, to see what it would look like with that number of observers). Krinkle: //(FYI: I change this number locally to e.g. 1, 2 or 14, to see what it would look like with… | |||||
// Compute average rating of participating players | // Compute average rating of participating players | ||||
let playerRatings = []; | let playerRatings = []; | ||||
for (let player of stringifiedTeamListToPlayerData(game.players)) | for (let player of stringifiedTeamListToPlayerData(game.players)) | ||||
{ | { | ||||
let playerNickRating = splitRatingFromNick(player.Name); | let playerNickRating = splitRatingFromNick(player.Name); | ||||
if (player.Team != "observer") | if (player.Team != "observer") | ||||
playerRatings.push(playerNickRating.rating || g_DefaultLobbyRating); | playerRatings.push(playerNickRating.rating || g_DefaultLobbyRating); | ||||
else | |||||
++game.observerCount; | |||||
Not Done Inline Actions(Don't blame me, it was my computer science teacher who recommended to change if (x != y) to if (x==y) in if-else patterns to reduce the complexity by one operation) elexis: (Don't blame me, it was my computer science teacher who recommended to change `if (x != y)` to… | |||||
// Sort games with playing buddies above games with spectating buddies | // Sort games with playing buddies above games with spectating buddies | ||||
if (game.hasBuddies < 2 && g_Buddies.indexOf(playerNickRating.nick) != -1) | if (game.hasBuddies < 2 && g_Buddies.indexOf(playerNickRating.nick) != -1) | ||||
game.hasBuddies = player.Team == "observer" ? 1 : 2; | game.hasBuddies = player.Team == "observer" ? 1 : 2; | ||||
} | } | ||||
game.gameRating = | game.gameRating = | ||||
playerRatings.length ? | playerRatings.length ? | ||||
▲ Show 20 Lines • Show All 50 Lines • ▼ Show 20 Lines | function updateGameList() | ||||
let list_mapName = []; | let list_mapName = []; | ||||
let list_mapSize = []; | let list_mapSize = []; | ||||
let list_mapType = []; | let list_mapType = []; | ||||
let list_nPlayers = []; | let list_nPlayers = []; | ||||
let list_gameRating = []; | let list_gameRating = []; | ||||
let list = []; | let list = []; | ||||
let list_data = []; | let list_data = []; | ||||
let selectedGameIndex = -1; | let selectedGameIndex = -1; | ||||
Done Inline ActionsTranslators won't know what this is about, so it should have a translation comment: Don't split the translate and sprintf, so that people see the string arguments right near the string. elexis: Translators won't know what this is about, so it should have a translation comment:
//… | |||||
for (let i in g_GameList) | for (let i in g_GameList) | ||||
{ | { | ||||
let game = g_GameList[i]; | let game = g_GameList[i]; | ||||
let gameName = escapeText(game.name); | let gameName = escapeText(game.name); | ||||
let mapTypeIdx = g_MapTypes.Name.indexOf(game.mapType); | let mapTypeIdx = g_MapTypes.Name.indexOf(game.mapType); | ||||
if (game.ip == g_SelectedGameIP && game.port == g_SelectedGamePort) | if (game.ip == g_SelectedGameIP && game.port == g_SelectedGamePort) | ||||
selectedGameIndex = +i; | selectedGameIndex = +i; | ||||
let playerText; | |||||
if (game.observerCount) | |||||
Done Inline Actions(> 0 unneeded I guesS) elexis: (`> 0` unneeded I guesS) | |||||
Done Inline ActionsYeah, wasn't sure which style is preferred. Typecasting is sometimes frowned upon, but for properties where only one type is allowed, I think it's fair to expect the reader to know those rules for the shorter form to be an easily recognised idiom. Krinkle: Yeah, wasn't sure which style is preferred.
Typecasting is sometimes frowned upon, but for… | |||||
{ | |||||
Done Inline ActionsMight be interesting to use setStringTags for this. See D2151. Stan: Might be interesting to use setStringTags for this. See D2151. | |||||
Done Inline Actionsindeed ffffffff: indeed | |||||
// Translation: The number of non-bot players and observers in this game | |||||
Done Inline Actions+%(observercount)s or +%(observers)s? elexis: `+%(observercount)s` or `+%(observers)s`? | |||||
Done Inline Actionsnon-bot players -> lobby players? Also seen "human player" somewhere. Otherwise bot -> AI. (Its also not obvious whether thats a bug, whether it should reflect human players or ingame players including AI. For example 2 human vs 6 AI player looks like 2/8, as in 6 missing, but theyre not missing. Can be changed, but not sure if it should, also OT) elexis: non-bot players -> lobby players? Also seen "human player" somewhere. Otherwise bot -> AI. | |||||
playerText = setStringTags(sprintf(translate("%(current)s/%(max)s +%(observercount)s"), { | |||||
Done Inline ActionsAt least it would be consistent to make "gray" a global. And the more recent version of that would be to use setStringTags, so that people can also specify a font alongside if they want. That += " " + is preventing translators to change the order of the arguments. Same for playerText and the "/" operator. So
(
But that won't work, since the english strings can only be specified for singular n=1 and plural n>1, but we need 0 and > 0 checks here. elexis: At least it would be consistent to make "gray" a global. And the more recent version of that… | |||||
Done Inline ActionsI don't know if the order needs to be localisable, and using plural for this seems like a workaround. Do you want me to split it into two separate messages? e.g. one for x/y and one for x/y +z? Krinkle: I don't know if the order needs to be localisable, and using plural for this seems like a… | |||||
Done Inline ActionsYes, the order should be localizable, everything that is displayed should be. Plural strings allow the translator to use a different format depending on the count.
Yes. It's not our task to judge what makes sense in every language, but to provide translators the choice to decide it for them. elexis: Yes, the order should be localizable, everything that is displayed should be.
Whenever there is… | |||||
"current": game.nbp, | |||||
"max": game.maxnbp, | |||||
"observercount": game.observerCount | |||||
}), g_ObserverCountTags); | |||||
Not Done Inline ActionsThe gray g_ObserverCountTags is applied to the entire text, shouldn't the playercount have a different color? White and gray, or whatever. elexis: The gray `g_ObserverCountTags` is applied to the entire text, shouldn't the playercount have a… | |||||
} | |||||
else | |||||
{ | |||||
playerText = game.nbp + "/" + game.maxnbp; | |||||
// Translation: The number of non-bot players in this game | |||||
playerText = setStringTags(sprintf(translate("%(current)s/%(max)s"), { | |||||
"current": game.nbp, | |||||
"max": game.maxnbp | |||||
}), g_ObserverCountTags); | |||||
Not Done Inline ActionsThe duplication can be avoided by using game.observerCount ? translate("%(current)s/%(max)s +%(observercount)s") : translate("%(current)s/%(max)s") (optionally with ?\n :\n and some tabs) elexis: The duplication can be avoided by using `game.observerCount ? translate("%(current)s/%(max)s +%… | |||||
Not Done Inline ActionsThe duplication of what exactly? Do you mean we should pass all three variables to both? I avoided that as I wasn't sure whether that'd be allowed (might trip a warning for unused variable, depending on how strict we want things to be). The following could work though if that's preferred. let plainText = game.observerCount ? // Translation: The number of human players and observers in this game translate("%(current)s/%(max)s +%(observercount)s") : // Translation: The number of human players in this game translate("%(current)s/%(max)s"); let playerText = setStringTags(sprintf(plainText, { "current": game.nbp, "max": game.maxnbp, "observercount": game.observerCount }), g_ObserverCountTags); Krinkle: The duplication of what exactly? Do you mean we should pass all three variables to both?
I… | |||||
} | |||||
list_buddy.push(game.hasBuddies ? coloredText(g_BuddySymbol, g_GameColors[game.state]) : ""); | list_buddy.push(game.hasBuddies ? coloredText(g_BuddySymbol, g_GameColors[game.state]) : ""); | ||||
list_name.push(coloredText(gameName, g_GameColors[game.state])); | list_name.push(coloredText(gameName, g_GameColors[game.state])); | ||||
list_mapName.push(translateMapTitle(game.niceMapName)); | list_mapName.push(translateMapTitle(game.niceMapName)); | ||||
list_mapSize.push(translateMapSize(game.mapSize)); | list_mapSize.push(translateMapSize(game.mapSize)); | ||||
list_mapType.push(g_MapTypes.Title[mapTypeIdx] || ""); | list_mapType.push(g_MapTypes.Title[mapTypeIdx] || ""); | ||||
list_nPlayers.push(game.nbp + "/" + game.maxnbp); | list_nPlayers.push(playerText); | ||||
list_gameRating.push(game.gameRating); | list_gameRating.push(game.gameRating); | ||||
list.push(gameName); | list.push(gameName); | ||||
list_data.push(i); | list_data.push(i); | ||||
} | } | ||||
gamesBox.list_buddy = list_buddy; | gamesBox.list_buddy = list_buddy; | ||||
gamesBox.list_name = list_name; | gamesBox.list_name = list_name; | ||||
gamesBox.list_mapName = list_mapName; | gamesBox.list_mapName = list_mapName; | ||||
gamesBox.list_mapSize = list_mapSize; | gamesBox.list_mapSize = list_mapSize; | ||||
gamesBox.list_mapType = list_mapType; | gamesBox.list_mapType = list_mapType; | ||||
Done Inline ActionsOne could say that the unification of the two sprintf's makes the close slightly slower, but then again one could care less if there are only < 30 games at a time and it costs probably something of the order of magnitude of 1 microseconds. elexis: One could say that the unification of the two sprintf's makes the close slightly slower, but… | |||||
gamesBox.list_nPlayers = list_nPlayers; | gamesBox.list_nPlayers = list_nPlayers; | ||||
gamesBox.list_gameRating = list_gameRating; | gamesBox.list_gameRating = list_gameRating; | ||||
// Change these last, otherwise crash | // Change these last, otherwise crash | ||||
gamesBox.list = list; | gamesBox.list = list; | ||||
gamesBox.list_data = list_data; | gamesBox.list_data = list_data; | ||||
gamesBox.auto_scroll = false; | gamesBox.auto_scroll = false; | ||||
▲ Show 20 Lines • Show All 486 Lines • Show Last 20 Lines |
Specifying colors and fonts individually is deprecated (D2151), specify the object directly here, then we dont have to create it multiple times and it can be extended or changed by mods.
g_ObserverCountTags