Restructure the calculation of team values in the summary screen. Don't use the ugly hack (reading values from the gui strings) anymore. Showing team values in the summary charts is now super easy. Also fix a team values bug in D144.
Details
Details
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- https://svn.wildfiregames.com/public/ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 2730 Build 4658: Vulcan Build (Windows) Jenkins Build 4657: Vulcan Build Jenkins Build 4656: arc lint + arc unit
Event Timeline
There are a very large number of changes, so older changes are hidden. Show Older Changes
Comment Actions
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! Checking XML files...
http://jw:8080/job/phabricator/1754/ for more details.
Comment Actions
Executing section Default...
Executing section Source...
Executing section JS...
binaries/data/mods/public/gui/summary/counters.js
| 30| » let·ret·=·[];
| | [NORMAL] ESLintBear (no-unreachable):
| | Unreachable code.
binaries/data/mods/public/gui/summary/counters.js
| 30| » let·ret·=·[];
| | [NORMAL] JSHintBear:
| | Unreachable 'let' after 'return'.
binaries/data/mods/public/gui/summary/counters.js
| 92| » })
| | [NORMAL] JSHintBear:
| | Missing semicolon.
binaries/data/mods/public/gui/summary/layout.js
| 283| » » if·(rowPlayerObjectWidth·==·0)
| | [NORMAL] JSHintBear:
| | Use '===' to compare with '0'.
binaries/data/mods/public/gui/summary/summary.js
| 156| » [0,·1].forEach(i·=>·Engine.GetGUIObjectByName("chart["·+·i·+·"]").series_color·=·player_colors);
| | [NORMAL] ESLintBear (no-return-assign):
| | Arrow function should not return assignment.
binaries/data/mods/public/gui/summary/summary.js
| 439| » let·mapType·=·g_Settings.MapTypes.find(mapType·=>·mapType.Name·==·g_GameData.sim.mapSettings.mapType);
| | [NORMAL] ESLintBear (no-shadow):
| | 'mapType' is already declared in the upper scope.
binaries/data/mods/public/gui/summary/summary.js
| 17| }
| | [NORMAL] JSHintBear:
| | Missing semicolon.
Executing section XML GUI...
Executing section Python...
Executing section Perl...http://jw:8080/job/phabricator_lint/324/ for more details.
Comment Actions
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! Checking XML files...
http://jw:8080/job/phabricator/1755/ for more details.
Comment Actions
Executing section Default...
Executing section Source...
Executing section JS...
binaries/data/mods/public/gui/summary/layout.js
| 283| » » if·(rowPlayerObjectWidth·==·0)
| | [NORMAL] JSHintBear:
| | Use '===' to compare with '0'.
binaries/data/mods/public/gui/summary/counters.js
| 83| » })
| | [NORMAL] JSHintBear:
| | Missing semicolon.
binaries/data/mods/public/gui/summary/summary.js
| 156| » [0,·1].forEach(i·=>·Engine.GetGUIObjectByName("chart["·+·i·+·"]").series_color·=·player_colors);
| | [NORMAL] ESLintBear (no-return-assign):
| | Arrow function should not return assignment.
binaries/data/mods/public/gui/summary/summary.js
| 439| » let·mapType·=·g_Settings.MapTypes.find(mapType·=>·mapType.Name·==·g_GameData.sim.mapSettings.mapType);
| | [NORMAL] ESLintBear (no-shadow):
| | 'mapType' is already declared in the upper scope.
binaries/data/mods/public/gui/summary/summary.js
| 17| }
| | [NORMAL] JSHintBear:
| | Missing semicolon.
Executing section XML GUI...
Executing section Python...
Executing section Perl...http://jw:8080/job/phabricator_lint/325/ for more details.
| binaries/data/mods/public/gui/summary/counters.js | ||
|---|---|---|
| 58 | missing closing brace | |
| 88 | missing closing brace | |
| 96–97 | indentation broken | |
| 108 | Fixing the 0's :) | |
| 324–325 | calculateScore and Market use if blocks, idc which one to use but keep it consistent imo | |
| binaries/data/mods/public/gui/summary/summary.js | ||
| 302–303 | We assume here all g_GameData.sim.playerStates have the same length, which should be the case, but perhaps use the minimal length to be sure | |
| binaries/data/mods/public/gui/summary/counters.js | ||
|---|---|---|
| 108 | What do you mean? | |
| 324–325 | I used if for everythign with less than 3 cases and switch-case for >= 3 cases. But I don't really care. | |
| binaries/data/mods/public/gui/summary/summary.js | ||
| 302–303 | I think they always have the same length. If not, there is something else bugged... | |
Comment Actions
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! Checking XML files...
http://jw:8080/job/phabricator/1772/ for more details.
Comment Actions
Executing section Default...
Executing section Source...
Executing section JS...
binaries/data/mods/public/gui/summary/counters.js
| 83| » })
| | [NORMAL] JSHintBear:
| | Missing semicolon.
binaries/data/mods/public/gui/summary/summary.js
| 156| » [0,·1].forEach(i·=>·Engine.GetGUIObjectByName("chart["·+·i·+·"]").series_color·=·player_colors);
| | [NORMAL] ESLintBear (no-return-assign):
| | Arrow function should not return assignment.
binaries/data/mods/public/gui/summary/summary.js
| 439| » let·mapType·=·g_Settings.MapTypes.find(mapType·=>·mapType.Name·==·g_GameData.sim.mapSettings.mapType);
| | [NORMAL] ESLintBear (no-shadow):
| | 'mapType' is already declared in the upper scope.
binaries/data/mods/public/gui/summary/summary.js
| 17| }
| | [NORMAL] JSHintBear:
| | Missing semicolon.
binaries/data/mods/public/gui/summary/layout.js
| 283| » » if·(rowPlayerObjectWidth·==·0)
| | [NORMAL] JSHintBear:
| | Use '===' to compare with '0'.
Executing section XML GUI...
Executing section Python...
Executing section Perl...http://jw:8080/job/phabricator_lint/333/ for more details.
Comment Actions
The values for k/d, feminization and vegi ratio are now unzero-ed and looks like correct, exept those, an all AI replay test yielded no difference in any of the values.
All summary pages including charts still working.
Removed functions not used anymore.
When the changes to g_Teams.length will be pushed, the patch is ok imo (not accepting so we can decide about it)
| binaries/data/mods/public/gui/summary/summary.js | ||
|---|---|---|
| 464 | notice that this line is bugged, but already was before: When researching the market tech then g_PlayerCount == 2 and g_Teams == [,[2, 3]] so no team totals will be displayed also incorrect At end game then g_PlayerCount == 3 and g_Teams == [1, [2, 3]] so team totals will be displayed again (correct now). But do notice when not using team 1 and 2, there are situations this will also be wrong! As the comment is stating // Each player has his own team. Displaying teams makes no sense. we should explicitly check for that so g_Teams.every(team => team && team.length < 2) We could merge this change in here or when considering it unrelated to cleanup and already existing, push it to a one-liner followup commit (such a followup can then also change the other instances of g_Teams.length, which are not currently bugged). | |
| 476–477 | writing of the other instances, here is one, it could be rewritten to a for/in, nuking the check for g_Teams[i] (in layout.js is a similar thing) | |
| binaries/data/mods/public/gui/summary/summary.js | ||
|---|---|---|
| 464 | Good catch. I already noticed that some time ago, seems like I forgot it :\ | |
Comment Actions
Executing section Default...
Executing section Source...
Executing section JS...
binaries/data/mods/public/gui/summary/counters.js
| 83| » })
| | [NORMAL] JSHintBear:
| | Missing semicolon.
binaries/data/mods/public/gui/summary/layout.js
| 283| » » if·(rowPlayerObjectWidth·==·0)
| | [NORMAL] JSHintBear:
| | Use '===' to compare with '0'.
binaries/data/mods/public/gui/summary/summary.js
| 156| » [0,·1].forEach(i·=>·Engine.GetGUIObjectByName("chart["·+·i·+·"]").series_color·=·player_colors);
| | [NORMAL] ESLintBear (no-return-assign):
| | Arrow function should not return assignment.
binaries/data/mods/public/gui/summary/summary.js
| 439| » let·mapType·=·g_Settings.MapTypes.find(mapType·=>·mapType.Name·==·g_GameData.sim.mapSettings.mapType);
| | [NORMAL] ESLintBear (no-shadow):
| | 'mapType' is already declared in the upper scope.
binaries/data/mods/public/gui/summary/summary.js
| 17| }
| | [NORMAL] JSHintBear:
| | Missing semicolon.
Executing section XML GUI...
Executing section Python...
Executing section Perl...http://jw:8080/job/phabricator_lint/352/ for more details.
Comment Actions
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! Checking XML files...
http://jw:8080/job/phabricator/1796/ for more details.
Comment Actions
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! Checking XML files...
http://jw:8080/job/phabricator/1798/ for more details.
Comment Actions
Executing section Default...
Executing section Source...
Executing section JS...
binaries/data/mods/public/gui/summary/layout.js
| 283| » » if·(rowPlayerObjectWidth·==·0)
| | [NORMAL] JSHintBear:
| | Use '===' to compare with '0'.
binaries/data/mods/public/gui/summary/summary.js
| 156| » [0,·1].forEach(i·=>·Engine.GetGUIObjectByName("chart["·+·i·+·"]").series_color·=·player_colors);
| | [NORMAL] ESLintBear (no-return-assign):
| | Arrow function should not return assignment.
binaries/data/mods/public/gui/summary/summary.js
| 439| » let·mapType·=·g_Settings.MapTypes.find(mapType·=>·mapType.Name·==·g_GameData.sim.mapSettings.mapType);
| | [NORMAL] ESLintBear (no-shadow):
| | 'mapType' is already declared in the upper scope.
binaries/data/mods/public/gui/summary/summary.js
| 17| }
| | [NORMAL] JSHintBear:
| | Missing semicolon.
Executing section XML GUI...
Executing section Python...
Executing section Perl...http://jw:8080/job/phabricator_lint/354/ for more details.
Comment Actions
Teams numbers in the screen are now written as 01, 11, 21 etc. so a string concatenation somewhere instead of an addition (couldn't easily find where).
| binaries/data/mods/public/gui/summary/layout.js | ||
|---|---|---|
| 321–322 | maybe see the comments below first, but we shouldn't loop over team -1 (as was avoided in old code), so we could either return to the old situation or.... | |
| 324–325 | ....change this check to i == -1 I guess this is the reason why the summary screen crashes... | |
| binaries/data/mods/public/gui/summary/summary.js | ||
| 464 | notice that every() does not loop over the -1 "index" so this is correct | |
| 478 | On a second thought this code is actually wrong already :(, what it did was counting all players in team -1. now it will simple be 0 always. Both is wrong imo, since there can be teams of 1 player (practically no team), so it would be better I guess to set all solo players to team -1 (also the ones that have a team assigned) by setting that in g_Teams and g_GameData.sim.playerStates[player].team (please verify that I am not talking nonsense here, and the new approach will actually work/be better) g_WithoutTeams would then simply become g_Teams[-1]. I think this will also solve the fact that in a team game players without a team aren't displayed anymore. | |
Comment Actions
I totally forgot about -1. Fixed now.
Inlined g_WithoutTeam and calculated it correctly.
Fixed team caption (before it looped over numbers, now with for in it loops over strings).
Fix the summary, when some players have a team and some have none. (Already broken in A22)
Comment Actions
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! Checking XML files...
http://jw:8080/job/phabricator/1803/ for more details.
Comment Actions
Executing section Default...
Executing section Source...
Executing section JS...
binaries/data/mods/public/gui/summary/layout.js
| 283| » » if·(rowPlayerObjectWidth·==·0)
| | [NORMAL] JSHintBear:
| | Use '===' to compare with '0'.
binaries/data/mods/public/gui/summary/summary.js
| 156| » [0,·1].forEach(i·=>·Engine.GetGUIObjectByName("chart["·+·i·+·"]").series_color·=·player_colors);
| | [NORMAL] ESLintBear (no-return-assign):
| | Arrow function should not return assignment.
binaries/data/mods/public/gui/summary/summary.js
| 439| » let·mapType·=·g_Settings.MapTypes.find(mapType·=>·mapType.Name·==·g_GameData.sim.mapSettings.mapType);
| | [NORMAL] ESLintBear (no-shadow):
| | 'mapType' is already declared in the upper scope.
binaries/data/mods/public/gui/summary/summary.js
| 17| }
| | [NORMAL] JSHintBear:
| | Missing semicolon.
Executing section XML GUI...
Executing section Python...
Executing section Perl...http://jw:8080/job/phabricator_lint/355/ for more details.
| binaries/data/mods/public/gui/summary/counters.js | ||
|---|---|---|
| 73–74 | caps | |
| 73–74 | cap | |
| binaries/data/mods/public/gui/summary/layout.js | ||
| 313–315 | Why not !g_Teams[-1]? | |
| 321–322 | notice that on small screens some more things run invisible now, but out of scope. | |
| 324 | Using a for in we know g_Teams[i] will always exist i.e. it will be defined, ofc it still can be defined as 0 (or false, undefined whatever), but that won't happen, thus the check seems unneeded. Notice this is similar to the old checks in counter.js in the calculate[panel]Team functions. | |
| binaries/data/mods/public/gui/summary/layout.js | ||
|---|---|---|
| 313–315 | rly in a 2v2 it doesn't crash here, also still unneeded braces | |
| binaries/data/mods/public/gui/summary/layout.js | ||
|---|---|---|
| 313–315 |
Strange, now it works... | |
Comment Actions
removing that one line can be done when committing
| binaries/data/mods/public/gui/summary/summary.js | ||
|---|---|---|
| 109 | line can be removed (why did my grep missed that!?) | |
Comment Actions
changes committed in rP19958, somehow the commit message broke and diff not updated, can be closed now
Comment Actions
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/362/ for more details.
Comment Actions
Build has FAILED
Link to build: http://jw:8080/job/phabricator/1815/
See console output for more information: http://jw:8080/job/phabricator/1815/console
Comment Actions
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/366/ for more details.
Comment Actions
Build has FAILED
Link to build: http://jw:8080/job/phabricator/1819/
See console output for more information: http://jw:8080/job/phabricator/1819/console