Ticket #42290

wonders of the world report sorted by nation, then city

Eröffnet am: 2021-05-16 02:44 Letztes Update: 2023-01-15 12:32

Auswertung:
Verantwortlicher:
Typ:
Status:
Geschlossen
Komponente:
Meilenstein:
Priorität:
5 - Mittel
Schweregrad:
5 - Mittel
Lösung:
Gefixt
Datei:
6

Details

A simple patch applies directly on S2_6 (offset 1line) , S3_0 , S3_1

The wonder of the world report is sorted, by

  • destroyed
  • nations and inside a nation by cities
  • being build

with old fashioned separators (blank lines or '----' or '====')

Ticket-Verlauf (3/26 Historien)

2021-05-16 02:44 Aktualisiert von: alain_bkr
  • New Ticket "wonders of the world report sorted by nation, then city" created
2021-05-16 02:46 Aktualisiert von: alain_bkr
  • File wonders_report_sorted_by_nation_and_cities.png (File ID: 6815) is attached
2021-05-20 07:02 Aktualisiert von: cazfi
Kommentar

Is this wanted?

I myself usually want to know where a specific wonder is, which use-case this makes worse.

What wonders specific player has, can be seen from the intelligence report once https://www.hostedredmine.com/issues/883354 is in.

2021-05-26 07:03 Aktualisiert von: alain_bkr
Kommentar

Reply To cazfi

Is this wanted?

Yes :)

I myself usually want to know where a specific wonder is, which use-case this makes worse.

It is not worse than current, in average there are exactly the same numbers of line to read (find a random element in a list).

At least with my patch (concept) they are sorted by player and city.

What wonders specific player has, can be seen from the intelligence report once https://www.hostedredmine.com/issues/883354 is in.

Great, this proves there is a need for something.

You'll still need to go through all nations reports to find a wonder, this could be annoying (scenario Europe 1901 with 27 nations).

F7 report is one screen for all. I agree it would be nice to sort by translated names, which correspond to your need.

I'll try to improve may patch to sort like this.

2021-05-31 03:42 Aktualisiert von: alain_bkr
  • File 0001-Wonders-report-sorted-twice-by-nations-and-alphabeti.patch (File ID: 6962) is attached
2021-05-31 03:46 Aktualisiert von: alain_bkr
  • File wonders_report_sorted_by_nation_and_cities.png (File ID: 6815) is deleted
2021-05-31 03:48 Aktualisiert von: alain_bkr
Kommentar

The patch 0001-Wonders-report-sorted-by-nations-and-alphabetically.patch

The alpha sorting is done after translations

(Edited, 2021-05-31 05:28 Aktualisiert von: alain_bkr)
2021-05-31 04:20 Aktualisiert von: alain_bkr
  • File 0001-Wonders-report-sorted-twice-by-nations-and-alphabeti.patch (File ID: 6962) is deleted
2021-05-31 06:47 Aktualisiert von: alain_bkr
Kommentar

the second patch adds translations for nations adjective (which was missing in all branches) :

Eiffel Tower in Paris (French)

I tested in zh_TW i can only see that some kind of sorting is done, i guess it is correct. :-)

2022-02-19 15:48 Aktualisiert von: cazfi
Kommentar

Reply To alain_bkr

the second patch adds translations for nations adjective (which was missing in all branches) :

nation_adjective_for_player() already returns translated string, no?

2022-02-19 15:55 Aktualisiert von: cazfi
Kommentar

Your screenshot shows this with "widgets arranged for small display" layout, I think. That means you have plenty of vertical space for the report. I'm worried about usability of this (with increased vertical space used) in normal layout where the message area is just a couple of lines.

2022-04-02 21:46 Aktualisiert von: cazfi
Kommentar

Reply To cazfi

usability of this (with increased vertical space used) in normal layout where the message area is just a couple of lines.

I've come up with possible ways to go forward

1) Produce two different reports, and let the client side decide which one it uses, e.g., depending on the layout in use
2) Rework this patch not to use any more lines than current code: No separators, and definitely not listing all wonders twice; just sort them alphabetically *within each category*

Second option is much more straightforward, and does not require complex interactions between client and server, so I think it would be the way to go. Other opinions?

2022-04-02 21:47 Aktualisiert von: cazfi
2022-06-06 08:10 Aktualisiert von: cazfi
2022-08-05 08:55 Aktualisiert von: cazfi
2022-10-07 09:15 Aktualisiert von: cazfi
2022-12-07 02:55 Aktualisiert von: cazfi
Kommentar

Reply To cazfi

Reply To cazfi I've come up with possible ways to go forward 1) Produce two different reports, and let the client side decide which one it uses, e.g., depending on the layout in use
2) Rework this patch not to use any more lines than current code: No separators, and definitely not listing all wonders twice; just sort them alphabetically *within each category* Second option is much more straightforward, and does not require complex interactions between client and server, so I think it would be the way to go. Other opinions?

As I hope to move more on client feature development after long time working on stability and bugfixes, the first option might be better after all - to provide best report layout for each client and case.

2023-01-08 20:22 Aktualisiert von: cazfi
  • Verantwortlicher Update from (Keine) to cazfi
  • Lösung Update from Keine to Accepted
  • Meilenstein Update from 3.0.6 (closed) to 3.2.0
  • Komponente Update from (Keine) to Server
Kommentar

This ticket (patches here) is now then turning to server-side part only. Will need to open new tickets about clients.

1) I made a copy of existing report_wonders_of_the_world() function in report.c, so it had two of them
2) Applied you patches, which modified one of those copies, and left the other intact
3) Renamed the modified function as report_wonders_of_the_world_long(), did some minor style corrections (mainly removal of trailing spaces)
4) Added new report type, and call to the report_wonders_of_the_world_long() when that report is requested

Resulting patch attached.

2023-01-15 00:44 Aktualisiert von: cazfi
Kommentar

Reply To cazfi

Will need to open new tickets about clients.

Qt-client: #46520
Have also open PR for freeciv-web: https://github.com/freeciv/freeciv-web/pull/597

2023-01-15 12:32 Aktualisiert von: cazfi
  • Status Update from Offen to Geschlossen
  • Lösung Update from Accepted to Gefixt

Bearbeiten

You are not logged in. I you are not logged in, your comment will be treated as an anonymous post. » Anmelden