Changeset View
Standalone View
binaries/data/mods/public/gui/gamesetup/gamesetup.js
Show First 20 Lines • Show All 151 Lines • ▼ Show 20 Lines | var g_FormatChatMessage = { | ||||
"chat": (msg, user) => sprintf(translate("%(username)s %(message)s"), { | "chat": (msg, user) => sprintf(translate("%(username)s %(message)s"), { | ||||
"username": senderFont(sprintf(translate("<%(username)s>"), { "username": user })), | "username": senderFont(sprintf(translate("<%(username)s>"), { "username": user })), | ||||
"message": escapeText(msg.text || "") | "message": escapeText(msg.text || "") | ||||
}), | }), | ||||
"ready": (msg, user) => sprintf(g_ReadyData[msg.status].chat, { "username": user }), | "ready": (msg, user) => sprintf(g_ReadyData[msg.status].chat, { "username": user }), | ||||
"clientlist": (msg, user) => getUsernameList(), | "clientlist": (msg, user) => getUsernameList(), | ||||
}; | }; | ||||
var g_MapFilterList = prepareForDropdown([ | var g_MapFilterList = [ | ||||
{ | { | ||||
"id": "default", | "id": "default", | ||||
"name": translateWithContext("map filter", "Default"), | "name": translateWithContext("map filter", "Default"), | ||||
"tooltip": translateWithContext("map filter", "All maps except naval and demo maps."), | "tooltip": translateWithContext("map filter", "All maps except naval and demo maps."), | ||||
"filter": mapKeywords => mapKeywords.every(keyword => ["naval", "demo", "hidden"].indexOf(keyword) == -1), | "filter": mapKeywords => mapKeywords.every(keyword => ["naval", "demo", "hidden"].indexOf(keyword) == -1), | ||||
"Default": true | "Default": true | ||||
}, | }, | ||||
{ | { | ||||
Show All 21 Lines | var g_MapFilterList = [ | ||||
"filter": mapKeywords => mapKeywords.indexOf("trigger") != -1 | "filter": mapKeywords => mapKeywords.indexOf("trigger") != -1 | ||||
}, | }, | ||||
{ | { | ||||
"id": "all", | "id": "all", | ||||
"name": translate("All Maps"), | "name": translate("All Maps"), | ||||
"tooltip": translateWithContext("map filter", "Every map of the chosen maptype."), | "tooltip": translateWithContext("map filter", "Every map of the chosen maptype."), | ||||
"filter": mapKeywords => true | "filter": mapKeywords => true | ||||
}, | }, | ||||
]); | ]; | ||||
var g_MapFilterListFiltered = prepareForDropdown(g_MapFilterList); | |||||
elexis: wraitii proposed to name all variables that hold dropdown values as `g_FooList`, so… | |||||
/** | /** | ||||
* Whether this is a single- or multiplayer match. | * Whether this is a single- or multiplayer match. | ||||
*/ | */ | ||||
var g_IsNetworked; | var g_IsNetworked; | ||||
/** | /** | ||||
* Is this user in control of game settings (i.e. singleplayer or host of a multiplayergame). | * Is this user in control of game settings (i.e. singleplayer or host of a multiplayergame). | ||||
▲ Show 20 Lines • Show All 214 Lines • ▼ Show 20 Lines | "select": (idx) => { | ||||
g_GameAttributes.mapPath = g_MapPath[g_GameAttributes.mapType]; | g_GameAttributes.mapPath = g_MapPath[g_GameAttributes.mapType]; | ||||
delete g_GameAttributes.map; | delete g_GameAttributes.map; | ||||
if (g_GameAttributes.mapType != "scenario") | if (g_GameAttributes.mapType != "scenario") | ||||
g_GameAttributes.settings = { | g_GameAttributes.settings = { | ||||
"PlayerData": g_DefaultPlayerData.slice(0, 4) | "PlayerData": g_DefaultPlayerData.slice(0, 4) | ||||
}; | }; | ||||
reloadMapList(); | reloadMapFilterList(); | ||||
}, | }, | ||||
"autocomplete": true, | "autocomplete": true, | ||||
}, | }, | ||||
"mapFilter": { | "mapFilter": { | ||||
"title": () => translate("Map Filter"), | "title": () => translate("Map Filter"), | ||||
"tooltip": (hoverIdx) => g_MapFilterList.tooltip[hoverIdx] || translate("Select a map filter."), | "tooltip": (hoverIdx) => g_MapFilterListFiltered.tooltip[hoverIdx] || translate("Select a map filter."), | ||||
"labels": () => g_MapFilterList.name, | "labels": () => g_MapFilterListFiltered.name, | ||||
"ids": () => g_MapFilterList.id, | "ids": () => g_MapFilterListFiltered.id, | ||||
"default": () => g_MapFilterList.Default, | "default": () => g_MapFilterListFiltered.Default, | ||||
Done Inline ActionsThis implies that the default map filter contains maps. Could keep track of the working map filters, use the default if that passes and use any other working mapfilter otherwise, but I'm not sure if we care about that. elexis: This implies that the default map filter contains maps. Could keep track of the working map… | |||||
Not Done Inline Actions
Nope, if no default is given, prepareForDropdown sets the default to 0. (https://github.com/0ad/0ad/blob/ec75f37daf825880e19c9095bf06282a98c0ce1d/binaries/data/mods/public/gui/common/settings.js#L291) Imarok: > This implies that the default map filter contains maps.
Nope, if no default is given… | |||||
Not Done Inline ActionsIndeed, if the default "default" mapfilter doesn't contain any valid maps, it's going to be reset to the first mapfilter. elexis: Indeed, if the default "default" mapfilter doesn't contain any valid maps, it's going to be… | |||||
"defined": () => g_GameAttributes.mapFilter !== undefined, | "defined": () => g_GameAttributes.mapFilter !== undefined, | ||||
Done Inline ActionsCan be shortened to g_MapFilterList.id.indexOf(g_GameAttributes.mapFilter || "") != -1 elexis: Can be shortened to `g_MapFilterList.id.indexOf(g_GameAttributes.mapFilter || "") != -1` | |||||
"get": () => g_GameAttributes.mapFilter, | "get": () => g_GameAttributes.mapFilter, | ||||
"select": (idx) => { | "select": (idx) => { | ||||
g_GameAttributes.mapFilter = g_MapFilterList.id[idx]; | g_GameAttributes.mapFilter = g_MapFilterListFiltered.id[idx]; | ||||
delete g_GameAttributes.map; | delete g_GameAttributes.map; | ||||
reloadMapList(); | reloadMapList(); | ||||
}, | }, | ||||
"autocomplete": true, | "autocomplete": true, | ||||
}, | }, | ||||
"mapSelection": { | "mapSelection": { | ||||
"title": () => translate("Select Map"), | "title": () => translate("Select Map"), | ||||
"tooltip": (hoverIdx) => g_MapSelectionList.description[hoverIdx] || translate("Select a map to play on."), | "tooltip": (hoverIdx) => g_MapSelectionList.description[hoverIdx] || translate("Select a map to play on."), | ||||
▲ Show 20 Lines • Show All 905 Lines • ▼ Show 20 Lines | function getMapPreview(map) | ||||
let mapData = loadMapData(map); | let mapData = loadMapData(map); | ||||
if (!mapData || !mapData.settings || !mapData.settings.Preview) | if (!mapData || !mapData.settings || !mapData.settings.Preview) | ||||
return "nopreview.png"; | return "nopreview.png"; | ||||
return mapData.settings.Preview; | return mapData.settings.Preview; | ||||
} | } | ||||
/** | /** | ||||
* Initialize the dropdown containing all maps for the selected maptype and mapfilter. | * Returns all maps that match to filter and the set map type. | ||||
elexisUnsubmitted Done Inline ActionsJSdoc for filter elexis: JSdoc for filter | |||||
*/ | */ | ||||
function reloadMapList() | function getFilteredMaps(filter) | ||||
elexisUnsubmitted Done Inline ActionsThis function is correct, in particular becaues it excludes the "Random" map choice. elexis: This function is correct, in particular becaues it excludes the "Random" map choice. | |||||
{ | { | ||||
Done Inline Actions@return could also mention the type (or both possibilities if we go with the second argument, x|y iirc) elexis: @return could also mention the type (or both possibilities if we go with the second argument… | |||||
if (!g_MapPath[g_GameAttributes.mapType]) | if (!g_MapPath[g_GameAttributes.mapType]) | ||||
{ | { | ||||
error("Unexpected map type: " + g_GameAttributes.mapType); | error("Unexpected map type: " + g_GameAttributes.mapType); | ||||
return; | return []; | ||||
} | } | ||||
let mapFiles = g_GameAttributes.mapType == "random" ? | let mapFiles = g_GameAttributes.mapType == "random" ? | ||||
getJSONFileList(g_GameAttributes.mapPath) : | getJSONFileList(g_GameAttributes.mapPath) : | ||||
getXMLFileList(g_GameAttributes.mapPath); | getXMLFileList(g_GameAttributes.mapPath); | ||||
// Apply map filter, if any defined | let maps = []; | ||||
let mapList = []; | |||||
// TODO: Should verify these are valid maps before adding to list | // TODO: Should verify these are valid maps before adding to list | ||||
for (let mapFile of mapFiles) | for (let mapFile of mapFiles) | ||||
{ | { | ||||
let file = g_GameAttributes.mapPath + mapFile; | let file = g_GameAttributes.mapPath + mapFile; | ||||
let mapData = loadMapData(file); | let mapData = loadMapData(file); | ||||
let filterID = g_MapFilterList.id.findIndex(id => id == g_GameAttributes.mapFilter); | |||||
let mapFilter = g_MapFilterList.filter[filterID] || undefined; | |||||
if (!mapData.settings || mapFilter && !mapFilter(mapData.settings.Keywords || [])) | if (!mapData.settings || filter && !filter(mapData.settings.Keywords || [])) | ||||
continue; | continue; | ||||
mapList.push({ | maps.push({ | ||||
"file": file, | "file": file, | ||||
"name": translate(getMapDisplayName(file)), | "name": translate(getMapDisplayName(file)), | ||||
"color": g_ColorRegular, | "color": g_ColorRegular, | ||||
"description": translate(mapData.settings.Description) | "description": translate(mapData.settings.Description) | ||||
}); | }); | ||||
} | } | ||||
return maps; | |||||
} | |||||
/** | |||||
* Initialize the dropdown containing all map filters for the selected maptype. | |||||
*/ | |||||
function reloadMapFilterList() | |||||
{ | |||||
g_MapFilterListFiltered = prepareForDropdown(g_MapFilterList.filter(entry => | |||||
getFilteredMaps(entry.filter).length > 0 | |||||
)); | |||||
elexisUnsubmitted Done Inline Actionsentry -> mapFilter? This is doing too much work, since need to know only about one map. But fixing this is not worth the code IMO as the maps are already cached by the OS (#4072) and being re-read just few calls later. elexis: entry -> mapFilter?
This is doing too much work, since need to know only about one map. But… | |||||
elexisUnsubmitted Not Done Inline Actionsand loadMapData also caches the map data, so the OS cache is irrelevant even. elexis: and `loadMapData` also caches the map data, so the OS cache is irrelevant even.
| |||||
initDropdown("mapFilter"); | |||||
reloadMapList(); | |||||
} | |||||
/** | |||||
* Initialize the dropdown containing all maps for the selected maptype and mapfilter. | |||||
*/ | |||||
function reloadMapList() | |||||
{ | |||||
// Apply map filter, if any defined | |||||
elexisUnsubmitted Done Inline ActionsThere's always one defined, comment not really needed elexis: There's always one defined, comment not really needed | |||||
let filterID = g_MapFilterListFiltered.id.findIndex(id => id == g_GameAttributes.mapFilter); | |||||
let mapFilter = g_MapFilterListFiltered.filter[filterID]; | |||||
let mapList = getFilteredMaps(mapFilter); | |||||
mapList = mapList.sort(sortNameIgnoreCase); | mapList = mapList.sort(sortNameIgnoreCase); | ||||
elexisUnsubmitted Done Inline Actionscould merge that with the line above, or remove the mapList = as sort sorts in place too elexis: could merge that with the line above, or remove the `mapList =` as sort sorts in place too | |||||
if (g_GameAttributes.mapType == "random") | if (g_GameAttributes.mapType == "random") | ||||
mapList.unshift({ | mapList.unshift({ | ||||
"file": "random", | "file": "random", | ||||
"name": translateWithContext("map selection", "Random"), | "name": translateWithContext("map selection", "Random"), | ||||
"color": g_ColorRandom, | "color": g_ColorRandom, | ||||
"description": "Picks one of the maps of the given maptype and filter at random." | "description": "Picks one of the maps of the given maptype and filter at random." | ||||
Not Done Inline ActionsActually we only have to add a new argument to getFilteredMaps, use a ternaries in the existing return statements of getFilteredMaps and add an early return to the loop to avoid the useless work. It would also inform the reader that we don't need the results in some cases of calling this function. getFilteredMaps should be named so that it works with boolean returns too, filterMaps? elexis: Actually we only have to add a new argument to `getFilteredMaps`, use a ternaries in the… | |||||
Not Done Inline ActionsI don't think that is worth it. In the gamesetup code simple code is more important than performance. Imarok: I don't think that is worth it. In the gamesetup code simple code is more important than… | |||||
Not Done Inline ActionsActually mapFilter should be filterFunc for global consistency (since it isn't an element of g_MapFilters`), right? elexis: Actually mapFilter should be filterFunc for global consistency (since it isn't an element of… | |||||
Not Done Inline ActionsNo, as this is a whole filter and not just the filter func. Imarok: No, as this is a whole filter and not just the filter func. | |||||
Done Inline ActionsIt loops over g_MapFilters.filter which is an array of filter functions right? Not fond of that boolean argument? At least remove the unneeded > 0 elexis: It loops over `g_MapFilters.filter` which is an array of filter functions right?
Not fond of… | |||||
}); | }); | ||||
g_MapSelectionList = prepareForDropdown(mapList); | g_MapSelectionList = prepareForDropdown(mapList); | ||||
initDropdown("mapSelection"); | initDropdown("mapSelection"); | ||||
} | } | ||||
function loadMapData(name) | function loadMapData(name) | ||||
{ | { | ||||
▲ Show 20 Lines • Show All 54 Lines • ▼ Show 20 Lines | if (newMapData && newMapData.settings) | ||||
if (playerData) | if (playerData) | ||||
mapSettings.PlayerData = playerData; | mapSettings.PlayerData = playerData; | ||||
} | } | ||||
if (mapSettings.PlayerData) | if (mapSettings.PlayerData) | ||||
sanitizePlayerData(mapSettings.PlayerData); | sanitizePlayerData(mapSettings.PlayerData); | ||||
// Reload, as the maptype or mapfilter might have changed | // Reload, as the maptype or mapfilter might have changed | ||||
reloadMapList(); | reloadMapFilterList(); | ||||
g_GameAttributes.settings.RatingEnabled = Engine.HasXmppClient(); | g_GameAttributes.settings.RatingEnabled = Engine.HasXmppClient(); | ||||
Engine.SetRankedGame(g_GameAttributes.settings.RatingEnabled); | Engine.SetRankedGame(g_GameAttributes.settings.RatingEnabled); | ||||
supplementDefaults(); | supplementDefaults(); | ||||
g_IsInGuiUpdate = false; | g_IsInGuiUpdate = false; | ||||
} | } | ||||
▲ Show 20 Lines • Show All 364 Lines • ▼ Show 20 Lines | |||||
/** | /** | ||||
* Don't set any attributes here, just show the changes in the GUI. | * Don't set any attributes here, just show the changes in the GUI. | ||||
* | * | ||||
* Unless the mapsettings don't specify a property and the user didn't set it in g_GameAttributes previously. | * Unless the mapsettings don't specify a property and the user didn't set it in g_GameAttributes previously. | ||||
*/ | */ | ||||
function updateGUIObjects() | function updateGUIObjects() | ||||
{ | { | ||||
g_IsInGuiUpdate = true; | g_IsInGuiUpdate = true; | ||||
elexisUnsubmitted Not Done Inline ActionsSomeone didn't update svn before uploading the patch. The reloadMapList(); call that was inserted (restored) here doesn't need to be replaced with the reloadMapFilterList(), because the clients can't change the mapfilter setting and only see what the host set. As mentioned somewhere, the rebuilding of dropdown values should be made generic too later. elexis: Someone didn't update svn before uploading the patch.
The `reloadMapList();` call that was… | |||||
ImarokAuthorUnsubmitted Not Done Inline Actions
I think it would be safer and saner to replace it, because in future clients may change the settings(D431). Imarok: > The `reloadMapList();` call that was inserted (restored) here doesn't need to be replaced… | |||||
elexisUnsubmitted Not Done Inline ActionsThat sounds true. elexis: That sounds true. | |||||
updatePlayerAssignmentChoices(); | updatePlayerAssignmentChoices(); | ||||
// Hide exceeding dropdowns and checkboxes | // Hide exceeding dropdowns and checkboxes | ||||
for (let panel in g_OptionOrderGUI) | for (let panel in g_OptionOrderGUI) | ||||
for (let child of Engine.GetGUIObjectByName(panel + "Options").children) | for (let child of Engine.GetGUIObjectByName(panel + "Options").children) | ||||
child.hidden = true; | child.hidden = true; | ||||
// Show the relevant ones | // Show the relevant ones | ||||
▲ Show 20 Lines • Show All 425 Lines • Show Last 20 Lines |
wraitii proposed to name all variables that hold dropdown values as g_FooList, so g_MapFilters and g_MapFilterList?
A JSdoc comment could explain that this only contains mapfilters for which we have at least one map matching this.