Removed useless paths to maps, removed brackets around dates.
Added colist, which allow to sort savegames by dates/names/descriptions/maps.
Added the checkbox to hide incompatible savegames.
Details
- Reviewers
elexis - Commits
- rP19351: Add sortable columns to the loading screen and a compatibility filter for saved…
- Trac Tickets
- #4413
Save/Load games:
- Create compatible savegames.
- Create partial compatible savegames (without needed mods).
- Create incompatible savegames.
Diff Detail
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Event Timeline
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/564/ for more details.
Did you try arcanist already? I heard it bugs with windows though. If it doesn't work, try creating the diffs you upload with 9999 lines of unified context, so we can see the entire file when reviewing.
Nice feature.
I don't think we need to keep the orange and red savegame distinction. The user won't know either why some are red and others orange. Either it's compatible or it's not.
Looks like generateDateString will only contain Engine.FormatMillisecondsIntoDateStringLocal
I always wonder who is interested in the Map Type option. GameMode seems more interesting. Perhaps Map Type becomes more interesting in the future when we have tutorial and campaign mode though.
This is also another GUI page where we could add optional columns for people with higher resolutions than 1024x768.
binaries/data/mods/public/gui/common/functions_utility_loadsave.js | ||
---|---|---|
15 | so remove the copy from replay_menu | |
binaries/data/mods/public/gui/savedgames/load.js | ||
15 | comment unneeded | |
28 | (1) .id (2) Those three translate calls could become a single translate call here: (3) depending on how many attributes exist, might be shorter to write foo => ([ "prop1": ..., "prop2": ...}), didn't test whether it makes sense here | |
32 | we very rarely use const, maybe let is more in line with the rest of gui/, dont care really | |
78 | can it happen that the properties are undefined? can we remove the || [] checks? | |
83 | those 2 properties don't seem to be used at all for COList, they are inherited from CList, perhaps we can address this sometime. wraitii also was asking about this in D11 | |
87 | length always exists, right? so !! wouldn't be needed? | |
binaries/data/mods/public/gui/savedgames/load.xml | ||
37 | MouseLeftDoubleClickItem, I think this is case insensitive | |
40 | The strings were copied from the replay menu. Since they are in the same translation resource (messags.json), they will be reused (instead of adding a copy to another resource). The context seems pointless at first sight, but the Description string definitely should keep it's context so that translators know where it is used, so keeping the context for the neighboring strings is ok too. Changing the context from "replay" to "savegame" doesn't seem needed, let's reuse the existing strings until someone complains on transifex. Nothing to change here. |
Fixed elexis notes.
binaries/data/mods/public/gui/savedgames/load.js | ||
---|---|---|
28 | (1) Done. | |
32 | We don't change this value, so it has the reason to be const. | |
78 | Look at the replay code, it uses the same logic. I think there could be this case. | |
83 | We use it with all that we used with CList. |
binaries/data/mods/public/gui/common/color.js | ||
---|---|---|
165 | thanks for nuking the copy&pasted function. | |
binaries/data/mods/public/gui/common/functions_utility_loadsave.js | ||
11–12 | red & orange seem pointless on first sight. either its compatible or its not. | |
12 | since we touch all lines having that name and because its in common/, should be renamed | |
14 | spaces | |
20 | same as above, generateSavegameLabel? | |
binaries/data/mods/public/gui/savedgames/load.js | ||
55 | trailing whitespace | |
63 | trailing whitespace |
These inline comments were fixed in the final version and the changes were reviewed by Vladislav in irc.
The thing also works in 1024x768.
We agreed to fix the delete dialog in the future, not colorizing the title, not showing the filename to the path and nuking the gameSelection.list property from the COList.
binaries/data/mods/public/gui/common/functions_utility_loadsave.js | ||
---|---|---|
16 | Should _always_ pass a second argument. Should be true or false, not undefined. This is better as it also avoid the variable and if-statement. return compatibilityColor( Engine.FormatMillisecondsIntoDateStringLocal(metadata.time * 1000, translate("yyyy-MM-dd HH:mm:ss")), isCompatibleSavegame(metadata, engineInfo)); | |
binaries/data/mods/public/gui/savedgames/load.js | ||
83 | Adding // list strings used in the delete dialog for clarification. These two inherited list attributes should be removed from OList sometime. | |
binaries/data/mods/public/gui/savedgames/load.xml | ||
78 | checked by default in accordance with the replay menu, also because incompatible ones aren't useful. | |
binaries/data/mods/public/gui/savedgames/save.js | ||
39 | Not your bug, but while at it we should fix it: save.js doesn't pass the mod info to generateSavegameLabel, so displays all savegames as incompatible when trying to save new state. |