Opened 8 years ago

Last modified 8 years ago

#4033 closed defect

[PATCH] Siege Engines are not shown in the summary — at Version 11

Reported by: Imarok Owned by: Mate-86
Priority: Should Have Milestone: Alpha 21
Component: UI & Simulation Keywords: simple patch
Cc: Patch:

Description (last modified by Mate-86)

  1. Siege Engines should be listed in the summary
  2. The captured statistic for Siege Engines should be displayed/created too

http://trac.wildfiregames.com/raw-attachment/ticket/4033/captured-building.png

http://trac.wildfiregames.com/raw-attachment/ticket/4033/military-score.png

fix: http://trac.wildfiregames.com/raw-attachment/ticket/4033/units-summary-fixed.png

team summary fixed: http://trac.wildfiregames.com/raw-attachment/ticket/4033/team-summary.png

Change History (19)

comment:1 by Mate-86, 8 years ago

Owner: set to Mate-86
Status: new → assigned

by Mate-86, 8 years ago

Attachment: captured-units.png​ added

by Mate-86, 8 years ago

Attachment: captured-building.png​ added

by Mate-86, 8 years ago

Attachment: military-score.png​ added

comment:2 by Mate-86, 8 years ago

feedback from bb_:

  • captureable summary for Siege units and Total only => fixed
  • horizontal ordering of unit types could be changed to "... siege, naval, trader" => fixed
  • need to double check values printed (captured are marked as lost for the other player) => fixed
  • units and building headers need to be in the same order => fixed
Last edited 8 years ago by Mate-86 (previous) (diff)

comment:3 by Mate-86, 8 years ago

Keywords: rfc patch added
Summary: Siege Engines are not shown in the summary → [PATCH] Siege Engines are not shown in the summary

comment:4 by elexis, 8 years ago

Description: modified (diff)
Milestone: Backlog → Alpha 21

by Mate-86, 8 years ago

Attachment: siege-units-summary.patch​ added

v2

by Mate-86, 8 years ago

Attachment: units-summary-fixed.png​ added

comment:5 by Mate-86, 8 years ago

Description: modified (diff)

comment:6 by Mate-86, 8 years ago

Some open points:

  • not sure whether I should update some unit tests for JS?
  • I've modified counter.js:calculateUnitsTeam as well but I don't know how to test it. Should I setup a match with teams?
  • There is a slight misalignment on my latest screenshot. I may need help how to align the counters to the center.

in reply to:  6 comment:7 by elexis, 8 years ago

Replying to Mate-86:

not sure whether I should update some unit tests for JS?

We don't have tests for the GUI yet and it's not the purpose of this ticket to lie the foundation to that.

I've modified counter.js:calculateUnitsTeam as well but I don't know how to test it. Should I setup a match with teams?

Yes

There is a slight misalignment on my latest screenshot. I may need help how to align the counters to the center.

Been there, #3351, nuke the trailing newline probably.

by Mate-86, 8 years ago

Attachment: siege-units-summary.2.patch​ added

comment:8 by Mate-86, 8 years ago

I've uploaded a new patch. Removing the newline has solved the problem but I could not check the team summary page even if I set up some teams. If no objection I'd move the ticket to review queue.

comment:9 by Imarok, 8 years ago

Have you set Locked Teams to true?

comment:10 by Mate-86, 8 years ago

Thx, now I get the team summary as well. Need to apply some further fixes tomorrow.

by Mate-86, 8 years ago

Attachment: team-summary.png​ added

by Mate-86, 8 years ago

Attachment: siege-units-summary.3.patch​ added

comment:11 by Mate-86, 8 years ago

Description: modified (diff)

Team summary fixed as well, screenshot and latest patch attached.

Note: See TracTickets for help on using tickets.