Page MenuHomeWildfire Games

Add spies to the summary screen
ClosedPublic

Authored by mmoanis on May 28 2017, 3:48 PM.

Details

Summary

Adds statistics about the number of successful bribes a player have made, and show it in the miscellaneous page in the summary.

Test Plan

Once you have made a successful bribe, close the game - to see the summary - should see the number updated.

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

mmoanis created this revision.May 28 2017, 3:48 PM
Imarok requested changes to this revision.May 30 2017, 3:51 PM

Patch looks good in general, but it doesn't work with teams (in gamesetup set teams to locked and give some players teams).
Either you have to extend the ugly hack we use to calculate team scores atm (look at calculateMiscellaneousTeam) or wait for D482.

binaries/data/mods/public/gui/summary/counters.js
75 ↗(On Diff #2277)

Why have you added it there?
(Don't think its needed)

99 ↗(On Diff #2277)

same

488 ↗(On Diff #2277)

newline missing?

This revision now requires changes to proceed.May 30 2017, 3:51 PM

Okay, I will wait the refactoring then continue.

D482 is committed, so you can resume your work ;)

@Imarok Okay I am working on it, sorry my laptop was broken.

mmoanis updated this revision to Diff 3472.Sep 3 2017, 3:27 PM

Added team statistics in summary screen

Vulcan added a subscriber: Vulcan.Sep 3 2017, 4:26 PM

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://jenkins-master:8080/job/phabricator/1958/ for more details.

mmoanis marked an inline comment as done.Sep 4 2017, 11:23 PM
mmoanis added inline comments.
binaries/data/mods/public/gui/summary/counters.js
99 ↗(On Diff #2277)

This will be needed to gather team statistics. I used g_TeamHelperData[team].bribes to accumulate the number of bribes of each player in that team

Imarok added a comment.Sep 6 2017, 5:43 PM

I think you shouldn't use g_TeamHelperData if it's not really needed and calculate the bribes when directly, like it's done for most of the team values.
(I think most of g_TeamHelperData should be removed sometimes)
What about also adding the number of failed bribes, as that also costs resource now?
(Could be done like the Sent/Received tributes)

mmoanis updated this revision to Diff 3663.Sep 15 2017, 2:42 AM

Avoid using g_TeamHelperData and calculate team bribes when requested. Also, show stats about successful and failed bribes.

Summary screen

@Imarok Please review my changes

In D567#36444, @mmoanis wrote:

@Imarok Please review my changes

Yeah, sorry, I don't have that much time these days.
Though I'll review your patch this week.
It looks good at a first glance.

data/mods/public/gui/summary/counters.js
354–356 ↗(On Diff #3663)

not that important, but we could start with let i = 1 and therefor nuke that + 1

Imarok requested changes to this revision.EditedSep 30 2017, 3:45 PM

You should use the right root folder when creating patches.
(The paths of your current patch start with data/mods/public, while it should start with binaries/data/mods/public.)
The version before this had the right root folder.

For the requested changes please look some comments later ;)

This revision now requires changes to proceed.Sep 30 2017, 3:45 PM
elexis added a comment.EditedSep 30 2017, 3:48 PM

You can also apply the patch by downloading it, entering the correct directory and using patch -p0 < filename`.
If you make him do work again, might as well leave some feedback here in advance.

In D567#36705, @elexis wrote:

You can also apply the patch by downloading it, entering the correct directory and using patch -p0 < filename`.
If you make him do work again, might as well leave some feedback here in advance.

Sure, I just was lazy ;)

data/mods/public/gui/summary/counters.js
363 ↗(On Diff #3663)

Trailing tab

Rest is good and works as proposed.

data/mods/public/gui/summary/counters.js
335 ↗(On Diff #3663)

any cause for inventing a new function?
Why not just do it like it's done in L284 for the market team values? (except barterEfficency)

data/mods/public/gui/summary/layout.js
187 ↗(On Diff #3663)

Remove the space between Bribes and \n to correctly center Bribes

data/mods/public/simulation/components/tests/test_VisionSharing.js
138 ↗(On Diff #3663)

=> {} missing

mmoanis updated this revision to Diff 3847.Oct 6 2017, 4:47 PM

Used the summaryArraySum and getPlayerValuesPerTeam instead of writing my own function

This revision was automatically updated to reflect the committed changes.
Imarok added a comment.Oct 8 2017, 1:36 PM

Thanks for the patch :)
(I inlined calculateBribesTeam when commiting)