Opened 9 years ago

Closed 8 years ago

Last modified 4 years ago

#3216 closed enhancement (fixed)

[PATCH] Add capture statistics to summary screen

Reported by: sanderd17 Owned by: Imarok
Priority: Should Have Milestone: Alpha 21
Component: UI & Simulation Keywords: patch
Cc: Patch:

Description

Since capturing in SVN, the summary screen with the building stats isn't of big value anymore.

It would be better if the captured and lost buildings would also be shown (in case of lack of space, they could be added to the created and destroyed buildings).

Attachments (28)

3216.patch (7.3 KB ) - added by maveric 9 years ago.
3216.2.patch (7.2 KB ) - added by maveric 9 years ago.
3216.3.patch (7.2 KB ) - added by maveric 9 years ago.
pict.PNG (661.5 KB ) - added by maveric 9 years ago.
pict2.PNG (661.2 KB ) - added by maveric 9 years ago.
This color seems much better for me.
3216.4.patch (7.4 KB ) - added by maveric 9 years ago.
pict3.PNG (662.9 KB ) - added by maveric 9 years ago.
pict4.PNG (663.0 KB ) - added by maveric 9 years ago.
pict5.PNG (264.3 KB ) - added by maveric 9 years ago.
pict6.PNG (297.8 KB ) - added by maveric 9 years ago.
pict7.PNG (299.3 KB ) - added by maveric 9 years ago.
pict8.PNG (383.0 KB ) - added by maveric 9 years ago.
pict9.PNG (373.9 KB ) - added by maveric 9 years ago.
pict10.PNG (362.9 KB ) - added by maveric 9 years ago.
3216.5.patch (7.8 KB ) - added by maveric 9 years ago.
3216.6.patch (5.5 KB ) - added by maveric 9 years ago.
pict11.PNG (286.5 KB ) - added by maveric 9 years ago.
3216.7.patch (16.6 KB ) - added by maveric 9 years ago.
3216.8.patch (16.9 KB ) - added by maveric 9 years ago.
3216.9.patch (18.5 KB ) - added by maveric 9 years ago.
pict12.PNG (279.2 KB ) - added by maveric 9 years ago.
t3216_capture_statistics_v10.patch (21.7 KB ) - added by elexis 9 years ago.
The unalignment is caused by #3351. This patch works around the issue by adding another newline in counters.js. Also uses let instead of var in some places and replaces the deprecated "for each" loop with "for ... in".
t3216_capture_statistics_style.patch (3.3 KB ) - added by Imarok 8 years ago.
t3216_capture_statistics_v10_rebasedv2.patch (19.9 KB ) - added by Imarok 8 years ago.
rebased the patch. Not ready for reviewing
t3216_capture_statistics_v11.1.patch (22.4 KB ) - added by Imarok 8 years ago.
Ready to commit. But I don' know if I updated the GUIInterface test correctly
t3216_capture_statistics_v11.2.patch (23.6 KB ) - added by Imarok 8 years ago.
some wrong indentation, move the regex used twice into a seperate function
t3216_capture_statistics_v11.3.patch (22.7 KB ) - added by Imarok 8 years ago.
Remove all regex in one function. Some stylefix
t3216_capture_statistics_v11.4.patch (22.8 KB ) - added by Imarok 8 years ago.
Put more code into the regex function

Change History (66)

comment:1 by maveric, 9 years ago

Owner: set to maveric
Status: newassigned

comment:2 by maveric, 9 years ago

Milestone: BacklogAlpha 19

by maveric, 9 years ago

Attachment: 3216.patch added

comment:3 by maveric, 9 years ago

The color might be not suitable (just chose random one).

by maveric, 9 years ago

Attachment: 3216.2.patch added

comment:4 by sanderd17, 9 years ago

Do you have a screenshot to show what's changed?

Did you also take localisations and supported screen resolution into account? Some languages will need more space than English.

Also don't forget to add the needed keywords when you think it's ready for review, see SubmittingPatches.

comment:5 by maveric, 9 years ago

Keywords: patch review added
Summary: Add capture statistics to summary screen[PATCH] Add capture statistics to summary screen

by maveric, 9 years ago

Attachment: 3216.3.patch added

by maveric, 9 years ago

Attachment: pict.PNG added

in reply to:  3 comment:6 by elexis, 9 years ago

Replying to maveric:

The color might be not suitable (just chose random one).

See #3205

by maveric, 9 years ago

Attachment: pict2.PNG added

This color seems much better for me.

by maveric, 9 years ago

Attachment: 3216.4.patch added

by maveric, 9 years ago

Attachment: pict3.PNG added

in reply to:  4 comment:7 by maveric, 9 years ago

Replying to sanderd17:

Did you also take localisations and supported screen resolution into account? Some languages will need more space than English.

The last screenshot was taken in window mode with 1024x768 resolution and it looks fine I think. I didn't find the way to change it to lesser one in game settings, so I think it's okay with that. Also changing localization doesn't affects on the width of the columns in summary tabs (i.e. "Total", "Houses" etc.), so it's okay too.

comment:8 by leper, 9 years ago

How does it look after actually playing for a while? I have the suspicion that numbers > 10 for at least one of the values will create an issue with the available space.

by maveric, 9 years ago

Attachment: pict4.PNG added

comment:9 by maveric, 9 years ago

Although with one value > 10 it looks better.

by maveric, 9 years ago

Attachment: pict5.PNG added

comment:10 by maveric, 9 years ago

Well, I can reduce player name column.

by maveric, 9 years ago

Attachment: pict6.PNG added

comment:11 by maveric, 9 years ago

What do you think about this?

comment:12 by leper, 9 years ago

I guess having 2 on each line would be better

by maveric, 9 years ago

Attachment: pict7.PNG added

comment:13 by qwertz, 9 years ago

I would remove the second slash and I would use the colour that is used for the actual numbers in the heading (green for Constructed, red for Lost etc.). I'm however noone that can decide if this is really better :-P

by maveric, 9 years ago

Attachment: pict8.PNG added

by maveric, 9 years ago

Attachment: pict9.PNG added

comment:14 by maveric, 9 years ago

Colored heading looks great, but I wouldn't remove the second slash.

comment:15 by qwertz, 9 years ago

Why aren't the two lines identically indented in pict9? I think it would look better if the values for Destroyed and Constructed would start with the same indentation.

comment:16 by trompetin17, 9 years ago

IMO would be more interesting if you have

Constructed/Destoyed Capture/Lost

comment:17 by trompetin17, 9 years ago

btw could you maveric come to IRC #0ad-dev?

by maveric, 9 years ago

Attachment: pict10.PNG added

in reply to:  17 comment:18 by maveric, 9 years ago

Here is the version with same (almost) indentation and changed order as trompetin17 proposed.

Replying to trompetin17:

btw could you maveric come to IRC #0ad-dev?

Yeah, sure.

Last edited 9 years ago by maveric (previous) (diff)

comment:19 by elexis, 9 years ago

I like it, good work!

comment:20 by trompetin17, 9 years ago

Could you upload your current patch(with identation) i will review this ticket

by maveric, 9 years ago

Attachment: 3216.5.patch added

by maveric, 9 years ago

Attachment: 3216.6.patch added

comment:21 by maveric, 9 years ago

I excluded changes in StatisticsTracker from patch (not sure whether I got what you meant about recommendations).

by maveric, 9 years ago

Attachment: pict11.PNG added

by maveric, 9 years ago

Attachment: 3216.7.patch added

by maveric, 9 years ago

Attachment: 3216.8.patch added

by maveric, 9 years ago

Attachment: 3216.9.patch added

comment:22 by maveric, 9 years ago

Added colored headings for all panels.

by maveric, 9 years ago

Attachment: pict12.PNG added

comment:23 by maveric, 9 years ago

Here is screenshot.

comment:24 by leper, 9 years ago

Can you fix the unalignment of the two lines?

comment:25 by leper, 9 years ago

Any updates?

in reply to:  25 comment:26 by maveric, 9 years ago

Replying to leper:

Any updates?

I was a little busy, so didn't work on the patch. Also don't expect updates soon, because I have to prepare for the exams at university.

comment:27 by leper, 9 years ago

Ok, good luck for your exams.

in reply to:  24 comment:28 by elexis, 9 years ago

Replying to leper:

Can you fix the unalignment of the two lines?

The only problem with this is that the second row of each player has a like 2 pixel offset to the right?

by elexis, 9 years ago

The unalignment is caused by #3351. This patch works around the issue by adding another newline in counters.js. Also uses let instead of var in some places and replaces the deprecated "for each" loop with "for ... in".

comment:29 by elexis, 9 years ago

Keywords: review removed

Mentioned by leper in irc on August 1st:

(21:35:54) leper: Why does that use c,l,t and captured,lost,destroyed in some places that do nearly the same (21:36:05) leper: also move that one regex thingy to some function (21:37:04) leper: it also does something with tooltip_style in the summary screen (21:37:17) leper: might that revert the fix for that one ticket? (#3072) (21:38:21) leper: StatisticsTracker.prototype.CapturedBuilding also doesn't check if components are present before doing anything

Mentioned by Itms today:

(16:09:16) Itms: you will have to update the GUIInterface tests (16:09:52) Itms: because the Statistics tracking will be saved in the extended simulation state

comment:30 by Stan, 9 years ago

Milestone: Alpha 19Alpha 20

Pushing as neither I or elexis will have time to work on it before FF.

comment:31 by leper, 8 years ago

Any updates?

comment:32 by elexis, 8 years ago

Milestone: Alpha 20Backlog

Backlogging due to lack of progress.

comment:33 by Imarok, 8 years ago

Owner: changed from maveric to Imarok
Status: assignednew

comment:34 by Imarok, 8 years ago

Keywords: review added

by Imarok, 8 years ago

rebased the patch. Not ready for reviewing

by Imarok, 8 years ago

Ready to commit. But I don' know if I updated the GUIInterface test correctly

by Imarok, 8 years ago

some wrong indentation, move the regex used twice into a seperate function

by Imarok, 8 years ago

Remove all regex in one function. Some stylefix

by Imarok, 8 years ago

Put more code into the regex function

comment:35 by elexis, 8 years ago

Resolution: fixed
Status: newclosed

In 18395:

Capture statistics summary. Patch by maveric and Imarok, fixes #3216.

Also colorize the headings, use two lines per player and remove duplication of an ugly regex hack.

comment:36 by elexis, 8 years ago

Keywords: simple review removed
Milestone: BacklogAlpha 21

Thanks, that one was needed in alpha 19 and 20 already.

  • rewrote the statistics tracker part to not cancel if there are no costs
  • formatCaptured(trained, killed, captured, lost) -> buildings are not trained
  • added comment for the inclusion change
  • missing semicolon
  • if(
  • trailing whitespace
  • multiple consecutive spaces in xml
  • those names like playerNamet should be renamed

comment:37 by elexis, 8 years ago

In 18397:

Missing semicolon, rephrase comment, refs #3216.

comment:38 by elexis, 4 years ago

In 23087:

Refactor session lobby bot client code to use object orientation, refs #5387.

Adapt rating score for exploration score and add comment to keep calculateExplorationScore and calculateScoreTotal in sync following rP14752 (following the formula in rP12914 and rP14098 being otherwise in sync with the summary screen measures), refs #686.
Adapt rating score for captured entities and add comment to keep calculateMilitaryScore in sync following rP18395, refs #3216, rP16550.
Adapt rating score for trade and vegetarian food and add comment to keep calculateEconomyScore in sync following rP19584/D494 and rP20543/D1052, refs #3948.
Remove ceasefire sending from the client from rP16624 since the bot doesn't record it and since ceasefire time is expected to be 0 after a 1v1 was decided, refs #2749.
Resolve column value computation fragmentation and remove unneeded intermediary object encoding following rP14703, refs #686.
Remove hardcoding/duplication of unit and structure classes and send domesticUnitsTrained too, refs rP23086/D2384.
Move session code including the two globals to a separate folder so as to maximize separation of and ease distribution without lobby code.
Migrate useless ugly trailing commas from rP14098, to be removed in a patch modifying the bot code.

Differential Revision: https://code.wildfiregames.com/D2385

Note: See TracTickets for help on using tickets.