Project

Profile

Help

HostedRedmine.com has moved to the Planio platform. All logins and passwords remained the same. All users will be able to login and use Redmine just as before. Read more...

Feature #856260

Rework city dialog sum breakdowns to remove duplicate descriptions, etc

Added by Jacob Nevins over 1 year ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Client
Sprint/Milestone:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

A thing I noticed with the attached experimental savegame was that the plague risk breakdown had two lines with identical descriptions, "Bonus from Aqueduct".

This is because there are two separate effects clauses in effect (one is an additional bonus for Aqueduct before Industrialization), but the effect summary text doesn't mention Industrialization (and nor should it try, probably).

The attached rework merges such duplicate entries, by accumulating the breakdown in a "city_sum" structure. Other benefits of this:
  • Gets rid of the duplicate running-total logic that's been a source of bugs (e.g. bug #854306).
  • Centralises logic for spotting when the client has incomplete knowledge (added in gna bug #21442).

I've also made it so that the plague risk breakdown shows the bonus value from effects, like e.g. Gold (e.g. "-4.7% : Bonus from Aqueduct+Sewer System (-20%)". (This does show two separate percentages, but I think we are saved from confusion by the second one typically being obviously round numbers matching what you see in the online help.)

One possibly contentious point is the use of two __format(__printf__) qualifiers on a single function with two format strings. My GCC (8.3.0) accepts this and does the right thing with it, but I haven't checked whether it might be a portability hazard.

This touches quite a few strings.

26_exp_hugecity.sav.bz2 (15.7 KB) 26_exp_hugecity.sav.bz2 Example 2.6 experimental savegame Jacob Nevins, 2020-01-16 01:37 AM
before.png (25 KB) before.png Plague sum before patch Jacob Nevins, 2020-01-16 01:37 AM
after.png (35.4 KB) after.png Plague sum after patch Jacob Nevins, 2020-01-16 01:37 AM
m-citysum.patch (24.1 KB) m-citysum.patch master Jacob Nevins, 2020-01-16 01:43 AM
30-citysum.patch (24.1 KB) 30-citysum.patch S3_0 Jacob Nevins, 2020-01-16 01:43 AM
26-citysum.patch (23.9 KB) 26-citysum.patch S2_6 Jacob Nevins, 2020-01-16 01:43 AM
250
250

Related issues

Related to Freeciv - Bug #859209: citydlg_common.c uses va_list, but does not include stdarg.hClosed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>
Blocked by Freeciv - Feature #856256: Expose varargs method for astringClosed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>

History

#1 Updated by Jacob Nevins over 1 year ago

#2 Updated by Jacob Nevins over 1 year ago

#3 Updated by Jacob Nevins over 1 year ago

  • Status changed from Resolved to Closed

#4 Updated by Jacob Nevins over 1 year ago

One possibly contentious point is the use of two __format(__printf__) qualifiers on a single function

(I mentioned this to a friend and they confirmed that Clang does the right thing with this too.)

#5 Updated by Jacob Nevins over 1 year ago

  • Related to Bug #859209: citydlg_common.c uses va_list, but does not include stdarg.h added

Also available in: Atom PDF