Add a replay cache
Details
- Reviewers
elexis - Commits
- rP19674: Fix replay menu loading time by using a cache file
- Trac Tickets
- #3433
Ensured new replays get added to the cache
Ensured loading the replay menu now is faster
Ensured deleted replays are handled correctly
Use this command on unix to drop the HDD cache:
echo 3 | sudo tee /proc/sys/vm/drop_caches
In order to test for the bug described in #4320, change GetDirectoryName function to return replêys (i.e. ^+e).
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Tested every case.
Besides the missing OsString calls and few style improvements, the implementation leaves a rock solid appearance.
If we have a running game in one instance and another instance in the replay menu and press the reload cache button, the file will be updated with the most recent entry :-)
Besides replacing the // Remove entries without data with a condition when adding those entries, I've applied the style remarks and hence would accept this patch:
VFS reasoning:
The great performance issue is caused by having hundreds if not thousands of files around, in the worst case scattered across a mechanical disk.
Even if defragmented, still requires a massive overhead of individual file open, read and close calls.
Even with a VFS system these calls would be present if the file isn't stored in a single file. i.e. one huge zip archive with all files as we have with public.zip, see also https://trac.wildfiregames.com/wiki/Virtual_File_System.
But this is inadvisable, because we want players to easily share replays. Also storing everything in single file can mean that the entire replay database is lost if writing that file fails in some way.
Therefore if/when VFS support is added to this file, it will still have to use this separate cache file.
Future Work:
- a. The file doesn't need to be indented (even though it passes false in the stringify call, it still becomes intented for any reason).
- b. Better zip that file altogether, so that it's only some kilobytes instead of a handful of megabytes when having many replays.
- The cache file should contain relative file paths (so that the cache file could be moved to another directory without becoming useless):
{ "file": "/home/elexis/.local/share/0ad/replays/0.0.22/2016-11-21_0009/commands.txt", "directory": "2016-11-21_0009", "fileSize": 2643, "attribs": { "settings": { "PlayerData": [ { "Name": "elexis", "Civ": "brit", "Color": { "r": 46, "g": 46, "b": 200 }, "AI": "", "AIDiff": 5, "Team": -1 }, { "Name": "Player 2", "Civ": "ptol", "Color": { "r": 150, "g": 20, "b": 20 }, "AI": "", "AIDiff": 3, "Team": -1 }, { "Name": "Cleon", "Civ": "athen", "Color": { "r": 50, "g": 165, "b": 5 }, "AI": "petra", "AIDiff": 5, "Team": -1 }, { "Name": "Ptolemy Philometor", "Civ": "ptol", "Color": { "r": 230, "g": 230, "b": 75 }, "AI": "petra", "AIDiff": 5, "Team": -1 }, { "Name": "Dasaratha Maurya", "Civ": "maur", "Color": { "r": 50, "g": 170, "b": 170 }, "AI": "petra", "AIDiff": 5, "Team": -1 } ], "CheatsEnabled": true, "CircularMap": true, "Size": 128, "GameType": "conquest", "VictoryScripts": [ "scripts/TriggerHelper.js", "scripts/ConquestCommon.js", "scripts/Conquest.js" ], "WonderDuration": 20, "PopulationCap": 300, "Ceasefire": 0, "StartingResources": 300, "Name": "Survival of the Fittest", "Script": "survivalofthefittest.js", "Description": "[color=\"red\"]IMPORTANT NOTE: AI PLAYERS DO NOT WORK WITH THIS MAP[/color]\n\nProtect your base against endless waves of enemies. Use your woman citizen to collect the treasures at the center of the map before others do, and try to build your base up. The last player remaining will be the winner!", "BaseTerrain": [ "medit_sea_depths" ], "BaseHeight": 30, "Keywords": [ "trigger" ], "Preview": "survivalofthefittest.png", "RatingEnabled": false, "DisabledTemplates": [ "structures/ptol_lighthouse" ], "LockTeams": false, "LastManStanding": true, "TriggerScripts": [ "scripts/TriggerHelper.js", "scripts/ConquestCommon.js", "scripts/Conquest.js", "scripts/TriggerHelper.js", "random/survivalofthefittest_triggers.js" ], "mapType": "random", "Seed": 2454904821, "AISeed": 4164250586 }, "map": "maps/random/survivalofthefittest", "mapType": "random", "mapPath": "maps/random/", "mapFilter": "all", "gameSpeed": 2, "matchID": "CC79D21F5329982B", "script": "survivalofthefittest.js", "timestamp": "1479746954", "engine_version": "0.0.22", "mods": [ "mod", "public" ] }, "duration": 9 },
binaries/data/mods/public/gui/replaymenu/replay_actions.js | ||
---|---|---|
133 ↗ | (On Diff #1646) | Why is 0 excluded? selected != -1 |
binaries/data/mods/public/gui/replaymenu/replay_menu.js | ||
77 ↗ | (On Diff #1646) | Should have a comment for the loadReplays proposed in some other inline comment. @param while adding weird new stuff |
binaries/data/mods/public/gui/replaymenu/replay_menu.xml | ||
251 ↗ | (On Diff #1646) | just Reload, but add a tooltip to explain what this is actually about. <translatableAttribute id="tooltip">Rebuild the replay cache from scratch. Potentially slow!</translatableAttribute> |
binaries/data/mods/public/gui/session/session.js | ||
671 ↗ | (On Diff #1646) | Correct check, because replayDirectory also returns the filename of the currently played replay |
672 ↗ | (On Diff #1646) | It's probably a time-saving feature that this isn't called upon Alt+F4. |
source/ps/VisualReplay.cpp | ||
69 ↗ | (On Diff #1646) | Can be removed IMO, too noisy, only opening of one file (which may hang if the HDD hangs. Still nothing we can do about that with code then.) |
76 ↗ | (On Diff #1646) | not really needed |
77 ↗ | (On Diff #1646) | That sounds a lot like suffering from the same breakage reported in #4320, i.e. should use an OsString(). Should actually be tested by returning something like replêys in GetDirectoryName. |
89 ↗ | (On Diff #1646) | Bit awkward to have one yellow warning and two red errors, but this still isn't an actual error. ERROR: JS_ParseJSON failed! ERROR: SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data WARNING: The replay cache file is corrupted, it will be deleted |
90 ↗ | (On Diff #1646) | OsString() |
96 ↗ | (On Diff #1646) | Unneeded and noisy too IMO |
101 ↗ | (On Diff #1646) | OsString |
105 ↗ | (On Diff #1646) | OsString |
106 ↗ | (On Diff #1646) | OsString |
114 ↗ | (On Diff #1646) | ( Merging those 2 functions doesn't seem to be nicely doable because this one contains return statements which would have to be compensated with ugly scopes. |
116 ↗ | (On Diff #1646) | That can be kept since it is guaranteed to consume a lot of time if many unknown replays are added. |
119 ↗ | (On Diff #1646) | Comment not needed anymore when using the readable types |
120 ↗ | (On Diff #1646) | Since the type is used at least twice, a typedef would be nice, like typedef std::map<OsPath, std::pair<u32, off_t>> replayCacheMap; |
126 ↗ | (On Diff #1646) | Unneeded, noisy |
128 ↗ | (On Diff #1646) | Correct to use u32, because 2^32-1 is the greatest number of elements a JS array can hold. |
141 ↗ | (On Diff #1646) | This C++11 doesn't appear to be saving anything in comparison to fileList[fileName] = ... |
153 ↗ | (On Diff #1646) | // Specifies whether the next replay file should be kept |
156 ↗ | (On Diff #1646) | Not needed |
161 ↗ | (On Diff #1646) | don't want to lose -> want to save |
165 ↗ | (On Diff #1646) | u32 filesize: In file_system.h we see the filesize returned is of the type off_t, not exactly specified by posix. Using std::time_t and then converting it to double in the scriptInterface.SetProperty calls is what we do in the savegame & replay timestamp code, because double is equivalent to the only JS number type used. The greatest number is 2^53 iirc (i.e. holds much more than 2^32, but still less than 2^64). ps/Replay.cpp: m_ScriptInterface.SetProperty(attribs, "timestamp", (double)std::time(nullptr)); ps/SavedGame.cpp: simulation.GetScriptInterface().SetProperty(metadata, "time", (double)now); |
165 ↗ | (On Diff #1646) | Why not remove the`string8` conversion and use the OsPath ref from that DirectoryNames vector? |
166 ↗ | (On Diff #1646) | should be self-evident |
173 ↗ | (On Diff #1646) | Remove the conversion then |
201 ↗ | (On Diff #1646) | |
201 ↗ | (On Diff #1646) | Three lines feel a bit noisy, why not all in one: debug_printf( "Loading %zu cached replays, removed %zu outdated entries, loaded %i new entries\n", fileList.size(), fileList.size() - copyFromOldCache.size(), i); |
211 ↗ | (On Diff #1646) | not |
230 ↗ | (On Diff #1646) | unneeded comment |
232 ↗ | (On Diff #1646) | How about not adding files without attribs instead of removing them? |
233 ↗ | (On Diff #1646) | replayNonEmpty? |
331 ↗ | (On Diff #1646) | Remove this TIMER call, since we want only one call for the loading of all replay files, not one for each IMO |
407 ↗ | (On Diff #1646) | Use (double), the whole JS number type parts takes place at D205 |
449 ↗ | (On Diff #1646) | Since this is only called when ending the game, maybe. |
121 ↗ | (On Diff #160) | Actually we don't have doxygen comments here, but in the header file (unless the function only exists in the cpp file). Also this doxygen comment differs from the one in the header. |
source/ps/VisualReplay.h | ||
66 ↗ | (On Diff #1646) | As mentioned in another inline comment, could be renamed to compareFiles to become agnostic of the way how to achieve that |
68 ↗ | (On Diff #1646) | Remove the default |
74 ↗ | (On Diff #1646) |
|
115 ↗ | (On Diff #1646) | period |
source/ps/scripting/JSInterface_VisualReplay.cpp | ||
56 ↗ | (On Diff #1646) | (Would have suggested OsPath is it wasn't the case everywhere else in this file) |
updated diff coming soon
source/ps/VisualReplay.cpp | ||
---|---|---|
120 ↗ | (On Diff #1646) | Don't think we really need a typedef here ^^ |
source/ps/VisualReplay.cpp | ||
---|---|---|
120 ↗ | (On Diff #1646) | No, but it were nice to not repeat the type right? And the compiler will inline it anyway |
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/1355/ for more details.
Executing section Default... Executing section Source... Executing section JS... binaries/data/mods/public/gui/replaymenu/replay_menu.js | 99| » » if·(g_MapNames.indexOf(replay.attribs.settings.Name)·==·-1·&&·replay.attribs.settings.Name·!=·"") | | [NORMAL] JSHintBear: | | Use '!==' to compare with ''. binaries/data/mods/public/gui/replaymenu/replay_menu.js | 180| » if·(attribs.settings.PlayerData.length·&&·attribs.settings.PlayerData[0]·==·null) | | [NORMAL] JSHintBear: | | Use '===' to compare with 'null'. | | [NORMAL] ESLintBear (no-multi-spaces): | | Multiple spaces found before '+'. |----| | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js |1590|1590| |1591|1591| playerStatistics.economyScore += total + ","; |1592|1592| playerStatistics.militaryScore += Math.round((player.sequences.enemyUnitsKilledValue[maxIndex] + |1593| |- player.sequences.enemyBuildingsDestroyedValue[maxIndex]) / 10) + ","; | |1593|+ player.sequences.enemyBuildingsDestroyedValue[maxIndex]) / 10) + ","; |1594|1594| playerStatistics.totalScore += (total + Math.round((player.sequences.enemyUnitsKilledValue[maxIndex] + |1595|1595| player.sequences.enemyBuildingsDestroyedValue[maxIndex]) / 10)) + ","; |1596|1596| binaries/data/mods/public/gui/session/session.js | 407| » » colorizeHotkey("%(hotkey)s"·+·"·",·"selection.idleworker")·+ | | [NORMAL] ESLintBear (no-useless-concat): | | Unexpected string concatenation of literals. binaries/data/mods/public/gui/session/session.js | 822| » » i·==·0·|| | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. binaries/data/mods/public/gui/session/session.js | 824| » » g_GameAttributes.settings.PlayerData[i].AI·!=·""); | | [NORMAL] JSHintBear: | | Use '!==' to compare with ''. binaries/data/mods/public/gui/session/session.js | 926| » if·(direction·==·0) | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. binaries/data/mods/public/gui/session/session.js | 950| » » let·panelEnt·=·g_PanelEntities.find(panelEnt·=>·ent·==·panelEnt.ent); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js | 974| » g_PanelEntities·=·g_PanelEntities.sort((panelEntA,·panelEntB)·=>·panelEntIndex(panelEntA.ent)·-·panelEntIndex(panelEntB.ent)) | | [NORMAL] JSHintBear: | | Missing semicolon. binaries/data/mods/public/gui/session/session.js |1054| » » button.hidden·=·g_Groups.groups[i].getTotalCount()·==·0; | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. binaries/data/mods/public/gui/session/session.js |1055| » » button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1056| » » button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1057| » » button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1062| » » » let·icon·=·GetTemplateData(GetEntityState(g_Groups.groups[i].getEntsGrouped().reduce((pre,·cur)·=>·{ | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1116| » » if·(player·!=·0·&& | | [NORMAL] JSHintBear: | | Use '!==' to compare with '0'. binaries/data/mods/public/gui/session/session.js |1212| » » button.onpress·=·(function(e)·{·return·function()·{·selectAndMoveTo(e);·};·})(researchStarted[tech].researcher); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1400| » » if·(+playerID·==·0) | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. Executing section Python... Executing section Perl...
http://jw:8080/job/phabricator_lint/42/ for more details.
Should test whether this compiles under windows, but it's not like we don't have a release blocker that requires changing most path calls anyway.
2016-07-19-QuakeNet-#0ad-dev.log 15:48 < Imarok> Any new tickets to suggest me working on? ;) 15:53 < elexis> Imarok: if you want to be really really useful, implement the replay menu cache
Said and done, thanks! The endless loading times have been discouraging me from opening the developer overlay since it's introduction!
binaries/data/mods/public/gui/replaymenu/replay_menu.js | ||
---|---|---|
77 ↗ | (On Diff #1646) | @param {boolean} reload - if the cache should be relaoded. Why not name it compareFiles too? I had written this one since the above description is a bit arbitrary "data contains data" * @param {object} replaySelectionData - Currently selected filters and item to be restored after the loading. * @param {bool} compareFiles - If true, compares files briefly (which might be slow with optical harddrives), * otherwise blindly trusts the replay cache. |
source/ps/VisualReplay.cpp | ||
121 ↗ | (On Diff #160) | See above, no comments in .cpp but .h |
39 ↗ | (On Diff #2194) | That doesn't need to be global, only in the scope that uses it (see my proposal for examplE). |
135 ↗ | (On Diff #2194) | Why is the filesize u32? It should be double so that we are not bound to 4GB but to 2^53 IMO and it's consistent with the other places. |
153 ↗ | (On Diff #2194) | Why this scope? seems unneeded. Sounds like it came from having had a TIMER in there |
157 ↗ | (On Diff #2194) | We |
184 ↗ | (On Diff #2194) | Didn't we say something about the 4GB limitation and what c++ type resembles the JS number type? |
source/ps/VisualReplay.h | ||
49 ↗ | (On Diff #2194) | false otherwise should be self-evident |
57 ↗ | (On Diff #2194) | might want to add the period here and in the other places for consistency |
source/ps/scripting/JSInterface_VisualReplay.cpp | ||
36 ↗ | (On Diff #2194) | compare flies |
source/ps/scripting/JSInterface_VisualReplay.h | ||
28 ↗ | (On Diff #2194) | compare filet |
source/ps/scripting/JSInterface_VisualReplay.h | ||
---|---|---|
28 ↗ | (On Diff #2194) | Beef filet is much more tasty than chicken filet. Though Chicken is mostly cheaper. |
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/1367/ for more details.
Executing section Default... Executing section Source... Executing section JS... binaries/data/mods/public/gui/replaymenu/replay_menu.js | 100| » » if·(g_MapNames.indexOf(replay.attribs.settings.Name)·==·-1·&&·replay.attribs.settings.Name·!=·"") | | [NORMAL] JSHintBear: | | Use '!==' to compare with ''. binaries/data/mods/public/gui/replaymenu/replay_menu.js | 181| » if·(attribs.settings.PlayerData.length·&&·attribs.settings.PlayerData[0]·==·null) | | [NORMAL] JSHintBear: | | Use '===' to compare with 'null'. | | [NORMAL] ESLintBear (no-multi-spaces): | | Multiple spaces found before '+'. |----| | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js |1597|1597| |1598|1598| playerStatistics.economyScore += total + ","; |1599|1599| playerStatistics.militaryScore += Math.round((player.sequences.enemyUnitsKilledValue[maxIndex] + |1600| |- player.sequences.enemyBuildingsDestroyedValue[maxIndex]) / 10) + ","; | |1600|+ player.sequences.enemyBuildingsDestroyedValue[maxIndex]) / 10) + ","; |1601|1601| playerStatistics.totalScore += (total + Math.round((player.sequences.enemyUnitsKilledValue[maxIndex] + |1602|1602| player.sequences.enemyBuildingsDestroyedValue[maxIndex]) / 10)) + ","; |1603|1603| binaries/data/mods/public/gui/session/session.js | 412| » » colorizeHotkey("%(hotkey)s"·+·"·",·"selection.idleworker")·+ | | [NORMAL] ESLintBear (no-useless-concat): | | Unexpected string concatenation of literals. binaries/data/mods/public/gui/session/session.js | 829| » » i·==·0·|| | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. binaries/data/mods/public/gui/session/session.js | 831| » » g_GameAttributes.settings.PlayerData[i].AI·!=·""); | | [NORMAL] JSHintBear: | | Use '!==' to compare with ''. binaries/data/mods/public/gui/session/session.js | 933| » if·(direction·==·0) | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. binaries/data/mods/public/gui/session/session.js | 957| » » let·panelEnt·=·g_PanelEntities.find(panelEnt·=>·ent·==·panelEnt.ent); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js | 981| » g_PanelEntities·=·g_PanelEntities.sort((panelEntA,·panelEntB)·=>·panelEntIndex(panelEntA.ent)·-·panelEntIndex(panelEntB.ent)) | | [NORMAL] JSHintBear: | | Missing semicolon. binaries/data/mods/public/gui/session/session.js |1061| » » button.hidden·=·g_Groups.groups[i].getTotalCount()·==·0; | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. binaries/data/mods/public/gui/session/session.js |1062| » » button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1063| » » button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1064| » » button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1069| » » » let·icon·=·GetTemplateData(GetEntityState(g_Groups.groups[i].getEntsGrouped().reduce((pre,·cur)·=>·{ | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1123| » » if·(player·!=·0·&& | | [NORMAL] JSHintBear: | | Use '!==' to compare with '0'. binaries/data/mods/public/gui/session/session.js |1219| » » button.onpress·=·(function(e)·{·return·function()·{·selectAndMoveTo(e);·};·})(researchStarted[tech].researcher); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1407| » » if·(+playerID·==·0) | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. Executing section Python... Executing section Perl...
http://jw:8080/job/phabricator_lint/53/ for more details.
On Windows, this diff causes a crash deep in SpiderMonkey when opening the replay menu (not if there is nothing to load in the replay menu though).
msvcr120.dll!521e46a9() Unknown [Frames below may be incorrect and/or missing, no symbols loaded for msvcr120.dll] [External Code] mozjs38-ps-release.dll!js::irregexp::ActionNode::Emit(js::irregexp::RegExpCompiler * compiler, js::irregexp::Trace * trace) Line 4418 C++ mozjs38-ps-release.dll!js::irregexp::Trace::PerformDeferredActions(js::LifoAlloc * alloc, js::irregexp::RegExpMacroAssembler * assembler, int max_register, js::irregexp::OutSet & affected_registers, js::irregexp::OutSet * registers_to_pop, js::irregexp::OutSet * registers_to_clear) Line 2617 C++ > mozjs38-ps-release.dll!js::irregexp::Trace::Flush(js::irregexp::RegExpCompiler * compiler, js::irregexp::RegExpNode * successor) Line 2688 C++ [External Code]
source/ps/VisualReplay.cpp | ||
---|---|---|
201 ↗ | (On Diff #1646) | This C99 format specifier %zu seems to be what causes the crash on windows (it's not used anywhere else in the code base), likely because of the limited C99 support in VS2013. |
source/ps/VisualReplay.cpp | ||
---|---|---|
201 ↗ | (On Diff #1646) | Thanks for figuring that out! (03:13:03) Sandarac: https://blogs.msdn.microsoft.com/vcblog/2013/07/19/c99-library-support-in-visual-studio-2013/ |
Executing section Default... Executing section Source... Executing section JS... binaries/data/mods/public/gui/replaymenu/replay_menu.js | 100| » » if·(g_MapNames.indexOf(replay.attribs.settings.Name)·==·-1·&&·replay.attribs.settings.Name·!=·"") | | [NORMAL] JSHintBear: | | Use '!==' to compare with ''. binaries/data/mods/public/gui/replaymenu/replay_menu.js | 181| » if·(attribs.settings.PlayerData.length·&&·attribs.settings.PlayerData[0]·==·null) | | [NORMAL] JSHintBear: | | Use '===' to compare with 'null'. | | [NORMAL] ESLintBear (no-multi-spaces): | | Multiple spaces found before '+'. |----| | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/session.js |1597|1597| |1598|1598| playerStatistics.economyScore += total + ","; |1599|1599| playerStatistics.militaryScore += Math.round((player.sequences.enemyUnitsKilledValue[maxIndex] + |1600| |- player.sequences.enemyBuildingsDestroyedValue[maxIndex]) / 10) + ","; | |1600|+ player.sequences.enemyBuildingsDestroyedValue[maxIndex]) / 10) + ","; |1601|1601| playerStatistics.totalScore += (total + Math.round((player.sequences.enemyUnitsKilledValue[maxIndex] + |1602|1602| player.sequences.enemyBuildingsDestroyedValue[maxIndex]) / 10)) + ","; |1603|1603| binaries/data/mods/public/gui/session/session.js | 412| » » colorizeHotkey("%(hotkey)s"·+·"·",·"selection.idleworker")·+ | | [NORMAL] ESLintBear (no-useless-concat): | | Unexpected string concatenation of literals. binaries/data/mods/public/gui/session/session.js | 432| » » let·panelEnt·=·g_PanelEntities.find(panelEnt·=>·panelEnt.slot·!==·undefined·&&·panelEnt.slot·==·slot); | | [NORMAL] ESLintBear (no-shadow): | | 'panelEnt' is already declared in the upper scope. binaries/data/mods/public/gui/session/session.js | 443| » » let·panelEnt·=·g_PanelEntities.find(panelEnt·=>·panelEnt.slot·!==·undefined·&&·panelEnt.slot·==·slot); | | [NORMAL] ESLintBear (no-shadow): | | 'panelEnt' is already declared in the upper scope. binaries/data/mods/public/gui/session/session.js | 957| » » let·panelEnt·=·g_PanelEntities.find(panelEnt·=>·ent·==·panelEnt.ent); | | [NORMAL] ESLintBear (no-shadow): | | 'panelEnt' is already declared in the upper scope. binaries/data/mods/public/gui/session/session.js | 986| » let·getPanelEntNameTooltip·=·panelEntState·=>·"[font=\"sans-bold-16\"]"·+·template.name.specific·+·"[/font]"; | | [NORMAL] ESLintBear (no-shadow): | | 'panelEntState' is already declared in the upper scope. binaries/data/mods/public/gui/session/session.js |1062| » » button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i); | | [NORMAL] ESLintBear (no-shadow): | | 'i' is already declared in the upper scope. binaries/data/mods/public/gui/session/session.js |1063| » » button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i); | | [NORMAL] ESLintBear (no-shadow): | | 'i' is already declared in the upper scope. binaries/data/mods/public/gui/session/session.js |1064| » » button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i); | | [NORMAL] ESLintBear (no-shadow): | | 'i' is already declared in the upper scope. binaries/data/mods/public/gui/session/session.js | 829| » » i·==·0·|| | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. binaries/data/mods/public/gui/session/session.js | 831| » » g_GameAttributes.settings.PlayerData[i].AI·!=·""); | | [NORMAL] JSHintBear: | | Use '!==' to compare with ''. binaries/data/mods/public/gui/session/session.js | 933| » if·(direction·==·0) | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. binaries/data/mods/public/gui/session/session.js | 957| » » let·panelEnt·=·g_PanelEntities.find(panelEnt·=>·ent·==·panelEnt.ent); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js | 981| » g_PanelEntities·=·g_PanelEntities.sort((panelEntA,·panelEntB)·=>·panelEntIndex(panelEntA.ent)·-·panelEntIndex(panelEntB.ent)) | | [NORMAL] JSHintBear: | | Missing semicolon. binaries/data/mods/public/gui/session/session.js |1061| » » button.hidden·=·g_Groups.groups[i].getTotalCount()·==·0; | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. binaries/data/mods/public/gui/session/session.js |1062| » » button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1063| » » button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1064| » » button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1069| » » » let·icon·=·GetTemplateData(GetEntityState(g_Groups.groups[i].getEntsGrouped().reduce((pre,·cur)·=>·{ | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1123| » » if·(player·!=·0·&& | | [NORMAL] JSHintBear: | | Use '!==' to compare with '0'. binaries/data/mods/public/gui/session/session.js |1219| » » button.onpress·=·(function(e)·{·return·function()·{·selectAndMoveTo(e);·};·})(researchStarted[tech].researcher); | | [NORMAL] JSHintBear: | | Don't make functions within a loop. binaries/data/mods/public/gui/session/session.js |1407| » » if·(+playerID·==·0) | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. Executing section XML GUI... | | [INFO] XMLBear: | | XML can be formatted better. |----| | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/replaymenu/replay_menu.xml | |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/replaymenu/replay_menu.xml | 1| 1| <?xml version="1.0" encoding="utf-8"?> | 2| |- | 3| 2| <objects> | 4| 3| | 5| 4| <!-- Used to display game info. --> | | [INFO] XMLBear: | | XML can be formatted better. |----| | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/replaymenu/replay_menu.xml | |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/replaymenu/replay_menu.xml | 3| 3| <objects> | 4| 4| | 5| 5| <!-- Used to display game info. --> | 6| |- <script file="gui/common/color.js" /> | 7| |- <script file="gui/common/functions_civinfo.js" /> | 8| |- <script file="gui/common/functions_utility.js" /> | | 6|+ <script file="gui/common/color.js"/> | | 7|+ <script file="gui/common/functions_civinfo.js"/> | | 8|+ <script file="gui/common/functions_utility.js"/> | 9| 9| <script file="gui/common/gamedescription.js"/> | 10| |- <script file="gui/common/settings.js" /> | | 10|+ <script file="gui/common/settings.js"/> | 11| 11| | 12| 12| <!-- Used to display message boxes. --> | 13| 13| <script file="gui/common/functions_global_object.js" /> | | [INFO] XMLBear: | | XML can be formatted better. |----| | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/replaymenu/replay_menu.xml | |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/replaymenu/replay_menu.xml | 10|
http://jw:8080/job/phabricator_lint/62/ for more details.
Executing section Default... Executing section Source... Executing section JS... Executing section XML GUI... Executing section Python... Executing section Perl...
http://jw:8080/job/phabricator_lint/65/ for more details.
Build has FAILED
Link to build: http://jw:8080/job/phabricator/1383/
See console output for more information: http://jw:8080/job/phabricator/1383/console
Build has FAILED
Link to build: http://jw:8080/job/phabricator/1386/
See console output for more information: http://jw:8080/job/phabricator/1386/console