#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)
Change History (66)
comment:1 by , 9 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:2 by , 9 years ago
Milestone: | Backlog → Alpha 19 |
---|
by , 9 years ago
Attachment: | 3216.patch added |
---|
follow-up: 6 comment:3 by , 9 years ago
by , 9 years ago
Attachment: | 3216.2.patch added |
---|
follow-up: 7 comment:4 by , 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 , 9 years ago
Keywords: | patch review added |
---|---|
Summary: | Add capture statistics to summary screen → [PATCH] Add capture statistics to summary screen |
by , 9 years ago
Attachment: | 3216.3.patch added |
---|
by , 9 years ago
comment:6 by , 9 years ago
by , 9 years ago
Attachment: | 3216.4.patch added |
---|
by , 9 years ago
comment:7 by , 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 , 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 , 9 years ago
by , 9 years ago
by , 9 years ago
by , 9 years ago
comment:13 by , 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 , 9 years ago
by , 9 years ago
comment:15 by , 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.
by , 9 years ago
Attachment: | pict10.PNG added |
---|
comment:18 by , 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.
comment:20 by , 9 years ago
Could you upload your current patch(with identation) i will review this ticket
by , 9 years ago
Attachment: | 3216.5.patch added |
---|
by , 9 years ago
Attachment: | 3216.6.patch added |
---|
comment:21 by , 9 years ago
I excluded changes in StatisticsTracker from patch (not sure whether I got what you meant about recommendations).
by , 9 years ago
Attachment: | pict11.PNG added |
---|
by , 9 years ago
Attachment: | 3216.7.patch added |
---|
by , 9 years ago
Attachment: | 3216.8.patch added |
---|
by , 9 years ago
Attachment: | 3216.9.patch added |
---|
by , 9 years ago
Attachment: | pict12.PNG added |
---|
comment:26 by , 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:28 by , 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 , 9 years ago
Attachment: | t3216_capture_statistics_v10.patch added |
---|
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 , 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 , 9 years ago
Milestone: | Alpha 19 → Alpha 20 |
---|
Pushing as neither I or elexis will have time to work on it before FF.
comment:33 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | assigned → new |
by , 8 years ago
Attachment: | t3216_capture_statistics_style.patch added |
---|
comment:34 by , 8 years ago
Keywords: | review added |
---|
by , 8 years ago
Attachment: | t3216_capture_statistics_v10_rebasedv2.patch added |
---|
rebased the patch. Not ready for reviewing
by , 8 years ago
Attachment: | t3216_capture_statistics_v11.1.patch added |
---|
Ready to commit. But I don' know if I updated the GUIInterface test correctly
by , 8 years ago
Attachment: | t3216_capture_statistics_v11.2.patch added |
---|
some wrong indentation, move the regex used twice into a seperate function
by , 8 years ago
Attachment: | t3216_capture_statistics_v11.3.patch added |
---|
Remove all regex in one function. Some stylefix
by , 8 years ago
Attachment: | t3216_capture_statistics_v11.4.patch added |
---|
Put more code into the regex function
comment:36 by , 8 years ago
Keywords: | simple review removed |
---|---|
Milestone: | Backlog → Alpha 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
The color might be not suitable (just chose random one).