Project

General

Profile

Feature #856260

Rework city dialog sum breakdowns to remove duplicate descriptions, etc

Added by Jacob Nevins 6 months ago. Updated 6 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Client
Target version:
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

Related issues

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

Blocked by Freeciv - Feature #856256: Expose varargs method for astringClosed

History

#1 Updated by Jacob Nevins 6 months ago

#2 Updated by Jacob Nevins 6 months ago

#3 Updated by Jacob Nevins 6 months ago

  • Status changed from Resolved to Closed

#4 Updated by Jacob Nevins 6 months 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 5 months ago

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

Also available in: Atom PDF