Adds a new tab with two graphs which each show one summary value over time.
The value can be selected with three dropdowns.
Details
- Reviewers
bb elexis vladislavbelov - Commits
- rP19517: Show all summary values as a graph
- Trac Tickets
- #3403
Play a game, open the summary and the graphs.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- http://svn.wildfiregames.com/public/ps/trunk
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 531 Build 844: Vulcan Build Jenkins Build 843: arc lint + arc unit
Event Timeline
binaries/data/mods/public/simulation/components/StatisticsTracker.js | ||
---|---|---|
393 | just used that, as we use it somewhere else in the code too |
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/802/ for more details.
Thanks for the updates, much nicer code IMO :-)
binaries/data/mods/public/gui/summary/summary.js | ||
---|---|---|
28 | The color closing tag can be added if and only if the color element is defined. | |
77 | Sure you don't want to make it: let chartsVisible = panel.name == "chartsPanelButton"; generalPanel.hidden =chartsVisible; chartsPanel.hidden = !chartsVisible; if (chartsVisible) for (let i=0; i<2; ++i) updateCategoryDropdown(i); else updatePanelData(g_ScorePanelsData[panel.name.substr(0, panel.name.length - "PanelButton".length)]); ? | |
127 | Those hardcoded numbers are still meh. The numbers will be in trouble if I find time | |
137 | potentially early return | |
binaries/data/mods/public/gui/summary/summary.xml | ||
167 | Looks MUCH better! | |
194 | Those strings sound like transifex people would report them for ambiguity, so should add a translation comment or context |
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/803/ for more details.
binaries/data/mods/public/gui/summary/summary.js | ||
---|---|---|
28 |
What if the postfix should be colored too for a new value some day?
As thats not sure, I'll leave it as is
I think thats ok as it is, though it could be postfix isn't the best word for this property. | |
77 | Now I get your point :D |
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/809/ for more details.
binaries/data/mods/public/gui/summary/counters.js | ||
---|---|---|
482 | calculatePercent? | |
binaries/data/mods/public/gui/summary/summary.js | ||
13–46 | Move those color opening and closing tags to the place that uses them, so that we have actual separation of code and data plus duplication reduction | |
28 | Then the slash should explicitly add color tags. It's really bad to have the opening tag in one place of the code and the closing tag on the other side of the world. | |
83 | looks unconventional. Would be the first time we do such a thing while all the other code uses for() or map() |
binaries/data/mods/public/gui/summary/counters.js | ||
---|---|---|
482 | Nope, as the value already is given as percent. | |
binaries/data/mods/public/gui/summary/summary.js | ||
13–46 | That would mean adding > 10 color tags... | |
28 | that's because the colors are not only used together with the postfix. | |
83 | Sure, but why not? ;D |
No to the color tags. Add them if and only if a color is defined, close the tag in the same place where you open it, so confirmation that tags that are opened are closed becomes trivial.
binaries/data/mods/public/gui/session/session.js | ||
---|---|---|
1273 | Last time we changed something in this function, we broke the bot and the bots database! See rP19329 So needs to update the ratingbot accordingly and we will have to rename some table or something in the sqlite.db | |
binaries/data/mods/public/gui/summary/counters.js | ||
15 | thx | |
21 | This still looks broken. g_InfinitySymbol refered to, but g_InfiniteSymbol defined. Consult jshint. | |
25 | What is this thing actually? A postfix? A delimiter? | |
85 | Since that function is used in only one function, perhaps we can make this ugliness a local variable of that function |
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/881/ for more details.
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/882/ for more details.
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/883/ for more details.
binaries/data/mods/public/gui/session/session.js | ||
---|---|---|
1369 | In original code feminisation wasn't filled, but it was created, also feminisation doesn't seem to exist in the statisticTracker, so not sure i this is correct. Also in some code down we use feminization, so make it consistent | |
binaries/data/mods/public/gui/summary/counters.js | ||
79 | Add whitespaces in the arrays. | |
binaries/data/mods/public/gui/summary/summary.js | ||
83 | missing whitespace | |
98–101 | Use a loop? | |
binaries/data/mods/public/simulation/components/StatisticsTracker.js | ||
222–223 | IIRC we don't have caps after the hyphen, so remove them (some lines below too). If we do use caps there, add them in other comments | |
binaries/data/mods/public/simulation/components/tests/test_GuiInterface.js | ||
156 | Trailling comma, IIRC it was decided that the trailling comma's in the guiInterface shouldn't be removed, so they should be there in the tests too. | |
165 | .... Like a missing trailling comma here | |
249 | And here and more down |
binaries/data/mods/public/gui/session/session.js | ||
---|---|---|
1369 | I know, I think someone forgot to add that. Though, as changing the reportObject bugs the lobby bot I think its better to remove ' feminisation' . |
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/1019/ for more details.
Ignoring all other non quoted object keys and arrow function candidates in the tests, as they are out of scope imo.
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/1021/ for more details.
can u adjust the thickness of the graph lines? blue and red almost invisible on gray background in this thickness. pls xD
This problem is known, and I thing @vladislavbelov wanted to fix this (or did already?)
(Also wrong diff, as the drawing code was separate from this one.)
Maybe we should also show somehow the civs (icons) there in graphs as im missing this when looking at this :) maybe small icon in front of names or behind