Ticket #43426

Load counter information from a savegame

Eröffnet am: 2021-12-19 21:18 Letztes Update: 2022-02-09 19:30

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

Details

#41117 adds saving city counters values. This ticket is about loading them from such a savegame.

Ticket-Verlauf (3/19 Historien)

2021-12-19 21:18 Aktualisiert von: cazfi
  • New Ticket "Load counter information from a savegame" created
2022-01-07 21:25 Aktualisiert von: lachu
Kommentar

Loading works. I save game (server started with -s $FREECIV_SRC_DIRECTORY/saves), modify savegame (uncompress and edit as text), load with ./fcgui & ./fcser -s $FREECIV_SRC_DIRECTORY/saves -f $FREECIV_SRC_DIRECTORY/saves/${OUR_SAVE_NAME_WITHOUT_EXT}.sav . Save and open save with ark and text editor. My changes was preserved.

2022-01-07 21:49 Aktualisiert von: cazfi
Kommentar

Promising!

- Remove empty line after the function header
- Use SIZE_T_PRINTF instead of "%ld" to print size_t, for portability
- Do not ignore the failure. Should probably use sg_failure_ret() instead of log_error() & break
- Memory for counters has already been allocated when the virtual city was created. Do not allocate again
- Check that there was actual city was found by game_city_by_number() (is not NULL)
- You assume that counters in the savegame are in the same order as they currently are in the memory. Use city_counters_order instead
- I don't think you own what you get from secfile_lookup_int_vec() but it's still part of the secfile. If so, you should not free(city_count)
- ++i; -> i++;

2022-01-09 04:23 Aktualisiert von: lachu
Kommentar

Reply To cazfi

Promising! - Remove empty line after the function header
- Use SIZE_T_PRINTF instead of "%ld" to print size_t, for portability
- Do not ignore the failure. Should probably use sg_failure_ret() instead of log_error() & break
- Memory for counters has already been allocated when the virtual city was created. Do not allocate again
- Check that there was actual city was found by game_city_by_number() (is not NULL)
- You assume that counters in the savegame are in the same order as they currently are in the memory. Use city_counters_order instead
- I don't think you own what you get from secfile_lookup_int_vec() but it's still part of the secfile. If so, you should not free(city_count)
- ++i; -> i++;

I made these changes in https://osdn.net/ticket/download.php?group_id=12505&tid=43426&file_id=8284 . Additionally, I introduced two other patches: https://osdn.net/ticket/download.php?group_id=12505&tid=43426&file_id=8285 https://osdn.net/ticket/download.php?group_id=12505&tid=43426&file_id=8286

2022-01-09 04:29 Aktualisiert von: lachu
Kommentar

How I test changes: 1. I introduced new counter, called loyalty. 2. I saw my changes do not broke savefile loading, so I introduce second patch 3. I rearrange Loyalty with owned, save game and checked values created matrix with opposite numbers 4. I load previous game and save it 5. I checked 3 and 4 contains the same values for counters.

2022-01-13 01:11 Aktualisiert von: cazfi
Kommentar

You previous version was a good base. Maybe it's better to go back to it than to try to fix latest version.

- See how loading of other items using "orders" vectors is implemented, and do this in a similar way
- Really use SIZE_T_PRINTF. It's set correctly for each platform by configure. Not all platforms support "%zu" either.

2022-02-06 22:57 Aktualisiert von: lachu
Kommentar

I send patch.

You can test it by applying special patch, so you can change order in savegame and test it was preserved.

2022-02-06 22:58 Aktualisiert von: lachu
Kommentar

Reply To cazfi

You previous version was a good base. Maybe it's better to go back to it than to try to fix latest version. - See how loading of other items using "orders" vectors is implemented, and do this in a similar way
- Really use SIZE_T_PRINTF. It's set correctly for each platform by configure. Not all platforms support "%zu" either.

Sorry you must wait too long. Can you look at my patches?

2022-02-06 23:48 Aktualisiert von: cazfi
Kommentar

I applied "0001-Added-city-counter-load-routines.patch" + "0002-Properly-load-savegame-with-counter-info.patch". I think the code is correct now, just a couple of style issues to fix.

- Last sg_failure_ret() -line has a trailing space
- Remove empty line between sg_load_counters() function header and the function itself
- "static void sg_load_counters (struct loaddata * loading)" -> "static void sg_load_counters(struct loaddata *loading)" (two spaces in wrong places removed)

Can you also provide both of these as a single patch.

2022-02-07 23:03 Aktualisiert von: lachu
Kommentar

Reply To cazfi

I applied "0001-Added-city-counter-load-routines.patch" + "0002-Properly-load-savegame-with-counter-info.patch". I think the code is correct now, just a couple of style issues to fix. - Last sg_failure_ret() -line has a trailing space
- Remove empty line between sg_load_counters() function header and the function itself
- "static void sg_load_counters (struct loaddata * loading)" -> "static void sg_load_counters(struct loaddata *loading)" (two spaces in wrong places removed)
Can you also provide both of these as a single patch.

Done.

2022-02-07 23:12 Aktualisiert von: cazfi
  • Verantwortlicher Update from (Keine) to cazfi
  • Lösung Update from Keine to Accepted
2022-02-09 19:30 Aktualisiert von: cazfi
  • Status Update from Offen to Geschlossen
  • Lösung Update from Accepted to Gefixt

Dateianhangliste

Bearbeiten

Please login to add comment to this ticket » Anmelden