Changeset View
Standalone View
binaries/data/mods/public/gui/replaymenu/replay_filters.js
| Show All 17 Lines | |||||
| const g_PopulationCapacities = prepareForDropdown(g_Settings && g_Settings.PopulationCapacities); | const g_PopulationCapacities = prepareForDropdown(g_Settings && g_Settings.PopulationCapacities); | ||||
| /** | /** | ||||
| * Reloads the selectable values in the filters. The filters depend on g_Settings and g_Replays | * Reloads the selectable values in the filters. The filters depend on g_Settings and g_Replays | ||||
| * (including its derivatives g_MapSizes, g_MapNames). | * (including its derivatives g_MapSizes, g_MapNames). | ||||
| */ | */ | ||||
| function initFilters(filters) | function initFilters(filters) | ||||
| { | { | ||||
| Engine.GetGUIObjectByName("compatibilityFilter").checked = !filters || filters.compatibility; | Engine.GetGUIObjectByName("compatibilityFilter").checked = !filters || !("compatibility" in filters) || filters.compatibility; | ||||
elexis: Why not `filters.compatibility`, possibly `!== undefined` so that we have the same order (left… | |||||
Done Inline Actions!filters || filters.compatibility !== false? elexis: `!filters || filters.compatibility !== false`? | |||||
Not Done Inline Actionsnope, as this will error out when compatibility not defined Imarok: nope, as this will error out when compatibility not defined | |||||
Not Done Inline ActionsWould still prefer any "x.y operator z" form (i.e. || x.y === undefined if that above option truly errors out) since we use that order everywhere in the GUI. The reader should first read the subject, then the object. Or how about this: actually specify the property instead of using a fallback. I would suggest to commit this file excluding this line separately as reviewed by me. (I didn't test, but the matter is simple handcraft.) elexis: Would still prefer any "x.y operator z" form (i.e. `|| x.y === undefined` if that above option… | |||||
Not Done Inline Actions
that seems working.
Why do you want to commit separately? (Where is the benefit?) Imarok: > Would still prefer any "x.y operator z" form (i.e. `|| x.y === undefined` if that above… | |||||
Not Done Inline ActionsSince it's unrelated cleanup. No need for a new revision. Splitting things makes it easier to digest for the reader, also easier to revert in case. elexis: Since it's unrelated cleanup. No need for a new revision. Splitting things makes it easier to… | |||||
Not Done Inline ActionsEven better would be always defining filters.compatibility if filters is defined elexis: Even better would be always defining `filters.compatibility` if filters is defined | |||||
| if (filters && filters.playernames) | if (filters && filters.playernames) | ||||
| Engine.GetGUIObjectByName("playersFilter").caption = filters.playernames; | Engine.GetGUIObjectByName("playersFilter").caption = filters.playernames; | ||||
| initDateFilter(filters && filters.date); | initDateFilter(filters && ("date" in filters) && filters.date); | ||||
| initMapSizeFilter(filters && filters.mapSize); | initMapSizeFilter(filters && ("mapSize" in filters) && filters.mapSize); | ||||
| initMapNameFilter(filters && filters.mapName); | initMapNameFilter(filters && ("mapName" in filters) && filters.mapName); | ||||
| initPopCapFilter(filters && filters.popCap); | initPopCapFilter(filters && ("popCap" in filters) && filters.popCap); | ||||
| initDurationFilter(filters && filters.duration); | initDurationFilter(filters && ("duration" in filters) && filters.duration); | ||||
| initSingleplayerFilter(filters && filters.singleplayer); | initSingleplayerFilter(filters && ("singleplayer" in filters) && filters.singleplayer); | ||||
| initVictoryConditionFilter(filters && filters.victoryCondition); | initVictoryConditionFilter(filters && ("victoryCondition" in filters) && filters.victoryCondition); | ||||
| initRatedGamesFilter(filters && filters.ratedGames); | initRatedGamesFilter(filters && ("ratedGames" in filters) && filters.ratedGames); | ||||
elexisUnsubmitted Not Done Inline ActionsWhy are the x in y checks needed? elexis: Why are the `x in y` checks needed?
If they are needed, are the ones coming afterwards unneeded? | |||||
ImarokAuthorUnsubmitted Not Done Inline Actionsx in y checks are needed because I used a incomplete replaySelectionData. Imarok: x in y checks are needed because I used a incomplete replaySelectionData.
Let's look into e.g. | |||||
elexisUnsubmitted Not Done Inline ActionsDoes JS actually complain if you pass x && x.y if x.y is undefined? If not, nuke y in x. elexis: Does JS actually complain if you pass x && x.y if x.y is `undefined`?
If not, nuke `y in x`. | |||||
ImarokAuthorUnsubmitted Not Done Inline ActionsYes it does. Imarok: Yes it does.
`x && x.y || undefined`could work.
I'll see. | |||||
elexisUnsubmitted Not Done Inline Actionsx && x.y && x.y would work too. A bit retarded. Why not just pass filters and let the function decide which property it needs (also helps with encapsulation). elexis: `x && x.y && x.y` would work too. A bit retarded.
Still unhappy about those explicit warning… | |||||
elexisUnsubmitted Not Done Inline Actions
Did you? elexis: > I'll see.
Did you? | |||||
elexisUnsubmitted Not Done Inline ActionsThanks, looks better elexis: Thanks, looks better | |||||
| } | } | ||||
| /** | /** | ||||
| * Allow to filter by month. Uses g_Replays. | * Allow to filter by month. Uses g_Replays. | ||||
| */ | */ | ||||
| function initDateFilter(date) | function initDateFilter(date) | ||||
| { | { | ||||
| var months = g_Replays.map(replay => getReplayMonth(replay)); | var months = g_Replays.map(replay => getReplayMonth(replay)); | ||||
| ▲ Show 20 Lines • Show All 254 Lines • Show Last 20 Lines | |||||
Why not filters.compatibility, possibly !== undefined so that we have the same order (left to right) in the entire GUI?
Especially the last two conditions should become one where possible.