Opened 8 years ago

Last modified 22 months ago

#4099 new defect

[PATCH] Round instead of ceil/floor in the GUI

Reported by: elexis Owned by:
Priority: Must Have Milestone: Work In Progress
Component: UI – In-game Keywords: patch, simple
Cc: fatherbushido, Krinkle Patch:

Description (last modified by fatherbushido)

There are some rounding issues in the GUI:

  • Trade carts can have very poor income, like 3 per trip. Since resources are floats however, the +10% upgrade might be actually useful in such a situation. So maybe the selection details could show two digits if the income is < 10.
  • Some Hellenic buildings show +1HP due to some ceil(xxx.00002) (f.e. civic center 3301HP, house 1201HP, outpost 881HP, wooden tower 881CP).
  • The ceil call for healthpoints was added so that there couldn't be a unit shown with "0" health points. So maybe we should use a round(HP) || 1 to account for that (r18009).

It seems floor/ceil should be abandoned altogether, but that needs to be investigated thoroughly considering the described edge case.

Attachments (2)

roundup_wipv1.diff (8.4 KB ) - added by elexis 8 years ago.
round_v2.patch (12.8 KB ) - added by Sandarac 8 years ago.
In the summary screen, this only seems to be an obvious issue when calculating Resources Used. I didn't add Round to stats like "Units Trained".

Download all attachments as: .zip

Change History (19)

comment:1 by elexis, 8 years ago

Cc: fatherbushido added

Missing rounds likely introduced in r18500:

  • The translate("%(resourceIcon)s %(minimum)s to %(resourceIcon)s %(maximum)s") part of the wall-tooltip in tooltips.js isn't rounded at all and might show many digits.
  • The summary screen doesn't use any rounding at all. Thanks echotangoecho for discovering and reporting:

http://trac.wildfiregames.com/raw-attachment/ticket/4097/screenshot0001.png

The attached patch doesn't address the summary screen yet and it will have to be added in basically all places there.

by elexis, 8 years ago

Attachment: roundup_wipv1.diff added

comment:2 by elexis, 8 years ago

Keywords: simple added

Adding simple for anyone who wants to add round to all those places in the summary screen.

by Sandarac, 8 years ago

Attachment: round_v2.patch added

In the summary screen, this only seems to be an obvious issue when calculating Resources Used. I didn't add Round to stats like "Units Trained".

comment:3 by fatherbushido, 8 years ago

I tested it and push it in the review queue.

comment:4 by fatherbushido, 8 years ago

Keywords: review added; rfc removed

comment:5 by fatherbushido, 8 years ago

Keywords: rfc added; review removed

see http://trac.wildfiregames.com/ticket/3818#comment:10 for the resources cost part putting it in rfc back as there is still discussion about that

Last edited 8 years ago by fatherbushido (previous) (diff)

comment:6 by fatherbushido, 8 years ago

For the resources cost:

  • r18500 reverted in r18632
  • there are imo missing roundings just after the tech modifications (so the same rounding should occur too in Templates.js) and then we won't need any rounding in the gui (as Cost/Resources are integers).

See for example: http://trac.wildfiregames.com/attachment/ticket/3818/costroundinthesim.diff But in this case, we should take care of roundings in Petra (the above patch is probably wrong for that).

Last edited 8 years ago by fatherbushido (previous) (diff)

comment:7 by fatherbushido, 7 years ago

Another thing : wall has in gui 3001 / 3001 hp (sic). When upgrading it to a gate, we got in gui 3000 / 30001 hp.

comment:8 by elexis, 7 years ago

Milestone: BacklogWork In Progress

Moving to the new WIP milestone.

comment:9 by fatherbushido, 7 years ago

Keywords: rfc removed

I guess comments have been said. I remove rfc.

in reply to:  7 comment:10 by fatherbushido, 7 years ago

Description: modified (diff)

Replying to fatherbushido:

Another thing : wall has in gui 3001 / 3001 hp (sic). When upgrading it to a gate, we got in gui 3000 / 30001 hp.

It was a simulation bug and not a gui one (repairer and healer immediatly try to repair or heal). refs: r19994, r19995, r19996

comment:12 by Imarok, 5 years ago

Component: UI & SimulationIn-game UI

Move tickets to In-game UI as UI & Simulation got some sub components.

comment:13 by Krinkle, 5 years ago

Cc: Krinkle added

comment:14 by Silier, 3 years ago

Keywords: simple removed
severity: simple

comment:15 by Silier, 3 years ago

Keywords: simple added

comment:16 by Freagarach, 2 years ago

In 26236:

Ceil the resource costs for insufficient resources.

A controversial change (refs. #3818, #4099, Phab:D1438), but good to have in some form.

Patch by: @Langbart
Differential revision: https://code.wildfiregames.com/D4332

comment:17 by Langbart, 22 months ago

Duplicated ticket closed #6561

Note: See TracTickets for help on using tickets.