From 48f9c0a66c9b4568ed2139f0e9b95a8b533b5721 Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Thu, 21 Jul 2022 18:24:52 +0200 Subject: [PATCH 1/4] Use QString in tileset reading functions This makes crashes less likely since no pointer is used. --- client/tilespec.cpp | 39 +++++++++++++++++---------------------- client/tilespec.h | 7 ++++--- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/client/tilespec.cpp b/client/tilespec.cpp index 0f74d267cd..9b9545e7be 100644 --- a/client/tilespec.cpp +++ b/client/tilespec.cpp @@ -367,7 +367,7 @@ int focus_unit_state = 0; static bool tileset_update = false; -static struct tileset *tileset_read_toplevel(const char *tileset_name, +static struct tileset *tileset_read_toplevel(const QString &tileset_name, bool verbose, int topology_id); static bool load_river_sprites(struct tileset *t, @@ -923,12 +923,12 @@ void tileset_free(struct tileset *t) Returns TRUE iff tileset with suggested tileset_name was loaded. */ -bool tilespec_try_read(const char *tileset_name, bool verbose, int topo_id, - bool global_default) +bool tilespec_try_read(const QString &tileset_name, bool verbose, + int topo_id, bool global_default) { bool original; - if (tileset_name == nullptr + if (tileset_name.isEmpty() || !(tileset = tileset_read_toplevel(tileset_name, verbose, topo_id))) { QVector *list = fileinfolist(get_data_dirs(), TILESPEC_SUFFIX); @@ -985,32 +985,27 @@ bool tilespec_try_read(const char *tileset_name, bool verbose, int topo_id, Returns TRUE iff new tileset has been succesfully loaded. */ -bool tilespec_reread(const char *new_tileset_name, - bool game_fully_initialized) +bool tilespec_reread(const QString &name, bool game_fully_initialized) { int id; - struct tile *center_tile; enum client_states state = client_state(); - const char *name = new_tileset_name ? new_tileset_name : tileset->name; bool new_tileset_in_use; - // Make local copies since these values may be freed down below - QString tileset_name = name; - QString old_name = tileset->name; - - qInfo(_("Loading tileset \"%s\"."), qUtf8Printable(tileset_name)); + qInfo(_("Loading tileset \"%s\"."), qUtf8Printable(name)); /* Step 0: Record old data. * * We record the current mapcanvas center, etc. */ - center_tile = get_center_tile_mapcanvas(); + auto center_tile = tileset ? get_center_tile_mapcanvas() : nullptr; /* Step 1: Cleanup. * * Free old tileset. */ + const char *old_name = nullptr; if (tileset) { + old_name = tileset->name; tileset_free(tileset); } @@ -1018,14 +1013,14 @@ bool tilespec_reread(const char *new_tileset_name, * * We read in the new tileset. This should be pretty straightforward. */ - tileset = tileset_read_toplevel(qUtf8Printable(tileset_name), false, -1); + tileset = tileset_read_toplevel(name, false, -1); if (tileset != nullptr) { new_tileset_in_use = true; } else { new_tileset_in_use = false; - if (!(tileset = - tileset_read_toplevel(qUtf8Printable(old_name), false, -1))) { + if (old_name + && !(tileset = tileset_read_toplevel(old_name, false, -1))) { // Always fails. fc_assert_exit_msg(nullptr != tileset, "Failed to re-read the currently loaded tileset."); @@ -1160,10 +1155,10 @@ void tilespec_reread_callback(struct option *poption) See tilespec_reread() for details. */ -void tilespec_reread_frozen_refresh(const char *tname) +void tilespec_reread_frozen_refresh(const QString &name) { tileset_update = true; - tilespec_reread(tname, true); + tilespec_reread(name, true); tileset_update = false; menus_init(); } @@ -1594,7 +1589,7 @@ static void tileset_add_layer(struct tileset *t, mapview_layer layer) intro files. topology_id of -1 means any topology is acceptable. */ -static struct tileset *tileset_read_toplevel(const char *tileset_name, +static struct tileset *tileset_read_toplevel(const QString &tileset_name, bool verbose, int topology_id) { struct section_file *file; @@ -1618,7 +1613,7 @@ static struct tileset *tileset_read_toplevel(const char *tileset_name, fname = tilespec_fullname(tileset_name); if (!fname) { if (verbose) { - qCritical("Can't find tileset \"%s\".", tileset_name); + qCritical("Can't find tileset \"%s\".", qUtf8Printable(tileset_name)); } return nullptr; } @@ -1692,7 +1687,7 @@ static struct tileset *tileset_read_toplevel(const char *tileset_name, t->for_ruleset = nullptr; } - sz_strlcpy(t->name, tileset_name); + sz_strlcpy(t->name, tileset_name.toUtf8().data()); if (!secfile_lookup_int(file, &t->priority, "tilespec.priority") || !secfile_lookup_bool(file, &is_hex, "tilespec.is_hex")) { qCritical("Tileset \"%s\" invalid: %s", t->name, secfile_error()); diff --git a/client/tilespec.h b/client/tilespec.h index 40d7c1dfdc..6d06e4f7f8 100644 --- a/client/tilespec.h +++ b/client/tilespec.h @@ -117,11 +117,12 @@ bool tileset_has_error(const struct tileset *t); void finish_loading_sprites(struct tileset *t); -bool tilespec_try_read(const char *tileset_name, bool verbose, int topo_id, +bool tilespec_try_read(const QString &name, bool verbose, int topo_id, bool global_default); -bool tilespec_reread(const char *tileset_name, bool game_fully_initialized); +bool tilespec_reread(const QString &tileset_name, + bool game_fully_initialized); void tilespec_reread_callback(struct option *poption); -void tilespec_reread_frozen_refresh(const char *tname); +void tilespec_reread_frozen_refresh(const QString &name); void tileset_setup_specialist_type(struct tileset *t, Specialist_type_id id); void tileset_setup_unit_type(struct tileset *t, struct unit_type *punittype); From 9d4df8f38d7355f02df14185f7f211c6d20d1677 Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Thu, 21 Jul 2022 18:37:59 +0200 Subject: [PATCH 2/4] Remove default_tileset_name The last used tileset was saved in the options file, but that value wasn't shown anywhere in the interface. It was then used *instead* of the tileset configured in the settings when the topology matched. This was a source of confusion. Get rid of this behavior by removing the hidden option. --- client/client_main.cpp | 6 ++---- client/options.cpp | 21 +++------------------ client/options.h | 5 ++--- client/tilespec.cpp | 10 +--------- client/tilespec.h | 3 +-- 5 files changed, 9 insertions(+), 36 deletions(-) diff --git a/client/client_main.cpp b/client/client_main.cpp index 1bd1525986..4b28cd8264 100644 --- a/client/client_main.cpp +++ b/client/client_main.cpp @@ -585,17 +585,15 @@ int client_main(int argc, char *argv[]) boot_help_texts(client_current_nation_set(), tileset_help(tileset)); fill_topo_ts_default(); - if (!forced_tileset_name.isEmpty()) { - if (!tilespec_try_read(qUtf8Printable(forced_tileset_name), true, -1, - true)) { + if (!tilespec_try_read(forced_tileset_name, true, -1)) { qCritical(_("Can't load requested tileset %s!"), qUtf8Printable(forced_tileset_name)); client_exit(); return EXIT_FAILURE; } } else { - tilespec_try_read(gui_options.default_tileset_name, false, -1, true); + tilespec_try_read(QString(), false, -1); } audio_real_init(sound_set_name, music_set_name, sound_plugin_name); diff --git a/client/options.cpp b/client/options.cpp index f8ab84f64e..1f3d265132 100644 --- a/client/options.cpp +++ b/client/options.cpp @@ -88,7 +88,6 @@ struct client_options gui_options = { /** Migrations **/ false, //.first_boot = - "\0", //.default_tileset_name = "\0", //.default_tileset_overhead_name = "\0", //.default_tileset_iso_name = false, //.gui_qt_migrated_from_2_5 = @@ -4418,11 +4417,6 @@ void options_load() secfile_lookup_bool_default(sf, gui_options.gui_qt_migrated_from_2_5, "%s.migration_qt_from_2_5", prefix); - str = - secfile_lookup_str_default(sf, nullptr, "client.default_tileset_name"); - if (str != nullptr) { - sz_strlcpy(gui_options.default_tileset_name, str); - } str = secfile_lookup_str_default(sf, nullptr, "client.default_tileset_overhead_name"); if (str != nullptr) { @@ -4531,11 +4525,6 @@ void options_save(option_save_log_callback log_cb) client_options_iterate_all(poption) { client_option_save(poption, sf); } client_options_iterate_all_end; - if (gui_options.default_tileset_name[0] != '\0') { - secfile_insert_str(sf, gui_options.default_tileset_name, - "client.default_tileset_name"); - } - message_options_save(sf, "client"); options_dialogs_save(sf); @@ -4878,10 +4867,6 @@ const char *tileset_name_for_topology(int topology_id) break; } - if (tsn == nullptr) { - tsn = gui_options.default_tileset_name; - } - return tsn; } @@ -4957,15 +4942,15 @@ void fill_topo_ts_default() sizeof(gui_options.default_tileset_square_name)); } else { log_debug("Setting tileset for square topologies."); - tilespec_try_read(nullptr, false, 0, false); + tilespec_try_read(QString(), false, 0); } } if (is_ts_option_unset("default_tileset_hex_name")) { log_debug("Setting tileset for hex topology."); - tilespec_try_read(nullptr, false, TF_HEX, false); + tilespec_try_read(QString(), false, TF_HEX); } if (is_ts_option_unset("default_tileset_isohex_name")) { log_debug("Setting tileset for isohex topology."); - tilespec_try_read(nullptr, false, TF_ISO | TF_HEX, false); + tilespec_try_read(QString(), false, TF_ISO | TF_HEX); } } diff --git a/client/options.h b/client/options.h index 4311080d48..9d7a80e00d 100644 --- a/client/options.h +++ b/client/options.h @@ -62,9 +62,8 @@ struct client_options { bool save_options_on_exit; /** Migrations **/ - bool first_boot; /* There was no earlier options saved. - * This affects some migrations, but not all. */ - char default_tileset_name[512]; // pre-2.6 had just this one tileset name + bool first_boot; /* There was no earlier options saved. + * This affects some migrations, but not all. */ char default_tileset_overhead_name[512]; /* 2.6 had separate tilesets for ... */ char default_tileset_iso_name[512]; // ...overhead and iso topologies. diff --git a/client/tilespec.cpp b/client/tilespec.cpp index 9b9545e7be..24862b5e47 100644 --- a/client/tilespec.cpp +++ b/client/tilespec.cpp @@ -924,7 +924,7 @@ void tileset_free(struct tileset *t) Returns TRUE iff tileset with suggested tileset_name was loaded. */ bool tilespec_try_read(const QString &tileset_name, bool verbose, - int topo_id, bool global_default) + int topo_id) { bool original; @@ -965,10 +965,6 @@ bool tilespec_try_read(const QString &tileset_name, bool verbose, } option_set_default_ts(tileset); - if (global_default) { - sz_strlcpy(gui_options.default_tileset_name, tileset_basename(tileset)); - } - return original; } @@ -1138,10 +1134,6 @@ void tilespec_reread_callback(struct option *poption) tileset_name = option_str_get(poption); - /* As it's going to be 'current' tileset, make it global default if - * options saved. */ - sz_strlcpy(gui_options.default_tileset_name, tileset_name); - fc_assert_ret(nullptr != tileset_name && tileset_name[0] != '\0'); tileset_update = true; tilespec_reread(tileset_name, client.conn.established); diff --git a/client/tilespec.h b/client/tilespec.h index 6d06e4f7f8..aed0726ebe 100644 --- a/client/tilespec.h +++ b/client/tilespec.h @@ -117,8 +117,7 @@ bool tileset_has_error(const struct tileset *t); void finish_loading_sprites(struct tileset *t); -bool tilespec_try_read(const QString &name, bool verbose, int topo_id, - bool global_default); +bool tilespec_try_read(const QString &name, bool verbose, int topo_id); bool tilespec_reread(const QString &tileset_name, bool game_fully_initialized); void tilespec_reread_callback(struct option *poption); From 4158cb27be534b60dc9a590068196ae3484814e4 Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Thu, 21 Jul 2022 18:47:48 +0200 Subject: [PATCH 3/4] Don't try to set the ruleset based on the tileset The tileset in pregame isn't well defined because the topology isn't known yet. Always start servers with the default ruleset, and load a tileset based on that. If the user want to spawn a client with a given ruleset, they can use the -r option. --- client/connectdlg_common.cpp | 39 +----------------------------------- client/tilespec.cpp | 16 --------------- client/tilespec.h | 1 - 3 files changed, 1 insertion(+), 55 deletions(-) diff --git a/client/connectdlg_common.cpp b/client/connectdlg_common.cpp index 71e7512d1c..e077e69ac8 100644 --- a/client/connectdlg_common.cpp +++ b/client/connectdlg_common.cpp @@ -200,7 +200,7 @@ static int find_next_free_port(int starting_port, int highest_port) bool client_start_server(const QString &user_name) { QStringList arguments; - QString trueFcser, ruleset, storage, port_buf, savesdir, scensdir; + QString trueFcser, storage, port_buf, savesdir, scensdir; char buf[512]; int connect_tries = 0; @@ -233,8 +233,6 @@ bool client_start_server(const QString &user_name) return false; } - ruleset = QString::fromUtf8(tileset_what_ruleset(tileset)); - // Set up the command-line parameters. port_buf = QString::number(internal_server_port); savesdir = QStringLiteral("%1/saves").arg(storage); @@ -256,9 +254,6 @@ bool client_start_server(const QString &user_name) if (savefile.isEmpty()) { arguments << QStringLiteral("--file") << savefile; } - if (ruleset != nullptr) { - arguments << QStringLiteral("--ruleset") << ruleset; - } // Look for a server binary const QString server_name = QStringLiteral("freeciv21-server"); @@ -330,38 +325,6 @@ bool client_start_server(const QString &user_name) return false; } - /* We set the topology to match the view. - * - * When a typical player launches a game, he wants the map orientation to - * match the tileset orientation. So if you use an isometric tileset you - * get an iso-map and for a classic tileset you get a classic map. In - * both cases the map wraps in the X direction by default. - * - * This works with hex maps too now. A hex map always has - * tileset_is_isometric(tileset) return TRUE. An iso-hex map has - * tileset_hex_height(tileset) != 0, while a non-iso hex map - * has tileset_hex_width(tileset) != 0. - * - * Setting the option here is a bit of a hack, but so long as the client - * has sufficient permissions to do so (it doesn't have HACK access yet) it - * is safe enough. Note that if you load a savegame the topology will be - * set but then overwritten during the load. - * - * Don't send it now, it will be sent to the server when receiving the - * server setting infos. */ - { - char topobuf[16]; - - fc_strlcpy(topobuf, "WRAPX", sizeof(topobuf)); - if (tileset_is_isometric(tileset) && 0 == tileset_hex_height(tileset)) { - fc_strlcat(topobuf, "|ISO", sizeof(topobuf)); - } - if (0 < tileset_hex_width(tileset) || 0 < tileset_hex_height(tileset)) { - fc_strlcat(topobuf, "|HEX", sizeof(topobuf)); - } - desired_settable_option_update("topology", topobuf, false); - } - return true; } diff --git a/client/tilespec.cpp b/client/tilespec.cpp index 24862b5e47..feb1cd96d2 100644 --- a/client/tilespec.cpp +++ b/client/tilespec.cpp @@ -300,8 +300,6 @@ struct tileset { std::vector log; - char *for_ruleset; - std::vector> layers; struct { freeciv::layer_special *background, *middleground, *foreground; @@ -892,10 +890,8 @@ static void tileset_free_toplevel(struct tileset *t) delete[] t->summary; delete[] t->description; - delete[] t->for_ruleset; t->summary = nullptr; t->description = nullptr; - t->for_ruleset = nullptr; } /** @@ -1672,13 +1668,6 @@ static struct tileset *tileset_read_toplevel(const QString &tileset_name, } } - tstr = secfile_lookup_str_default(file, nullptr, "tilespec.for_ruleset"); - if (tstr != nullptr) { - t->for_ruleset = fc_strdup(tstr); - } else { - t->for_ruleset = nullptr; - } - sz_strlcpy(t->name, tileset_name.toUtf8().data()); if (!secfile_lookup_int(file, &t->priority, "tilespec.priority") || !secfile_lookup_bool(file, &is_hex, "tilespec.is_hex")) { @@ -5669,11 +5658,6 @@ const char *tileset_summary(struct tileset *t) { return t->summary; } */ const char *tileset_description(struct tileset *t) { return t->description; } -/** - Return what ruleset this tileset is primarily meant for - */ -char *tileset_what_ruleset(struct tileset *t) { return t->for_ruleset; } - /** Return tileset topology index */ diff --git a/client/tilespec.h b/client/tilespec.h index aed0726ebe..ece2ab6fe0 100644 --- a/client/tilespec.h +++ b/client/tilespec.h @@ -318,6 +318,5 @@ const char *tileset_name_get(struct tileset *t); const char *tileset_version(struct tileset *t); const char *tileset_summary(struct tileset *t); const char *tileset_description(struct tileset *t); -char *tileset_what_ruleset(struct tileset *t); int tileset_topo_index(struct tileset *t); help_item *tileset_help(struct tileset *t); From f28c8c6b7f3b4a0f3f9b8b059ae156a097b9f939 Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Thu, 21 Jul 2022 19:00:32 +0200 Subject: [PATCH 4/4] Remove for_ruleset from shipped tilesets It's no longer supported. --- data/alio.tilespec | 3 --- data/amplio.tilespec | 3 --- data/amplio2.tilespec | 3 --- data/cimpletoon.tilespec | 3 --- data/hex2t.tilespec | 3 --- data/hexemplio.tilespec | 3 --- data/isophex.tilespec | 3 --- data/isotrident.tilespec | 3 --- data/toonhex.tilespec | 3 --- data/trident.tilespec | 3 --- 10 files changed, 30 deletions(-) diff --git a/data/alio.tilespec b/data/alio.tilespec index 8e9edb64d9..0b438724ab 100644 --- a/data/alio.tilespec +++ b/data/alio.tilespec @@ -24,9 +24,6 @@ Tileset containing graphics suitable for playing with alien ruleset.\ ; TODO: add more overall information fields on tiles, ; eg, authors, colors, etc. -; What is the primary ruleset this tileset is meant for. -for_ruleset = "alien" - ; Basic tile sizes: normal_tile_width = 126 normal_tile_height = 64 diff --git a/data/amplio.tilespec b/data/amplio.tilespec index 89dcb0ca78..59138bba2a 100644 --- a/data/amplio.tilespec +++ b/data/amplio.tilespec @@ -18,9 +18,6 @@ summary = _("Older variant of the current default tileset, Amplio2") ; TODO: add more overall information fields on tiles, ; eg, authors, colors, etc. -; What is the primary ruleset this tileset is meant for. -;for_ruleset = "" - ; Basic tile sizes: normal_tile_width = 96 normal_tile_height = 48 diff --git a/data/amplio2.tilespec b/data/amplio2.tilespec index 12a70cf061..28424e0711 100644 --- a/data/amplio2.tilespec +++ b/data/amplio2.tilespec @@ -18,9 +18,6 @@ summary = _("Large isometric tileset.") ; TODO: add more overall information fields on tiles, ; eg, authors, colors, etc. -; What is the primary ruleset this tileset is meant for. -;for_ruleset = "" - ; Basic tile sizes: normal_tile_width = 96 normal_tile_height = 48 diff --git a/data/cimpletoon.tilespec b/data/cimpletoon.tilespec index fe1d0ca975..04d7305783 100644 --- a/data/cimpletoon.tilespec +++ b/data/cimpletoon.tilespec @@ -21,9 +21,6 @@ direction the unit is facing.\ ; TODO: add more overall information fields on tiles, ; eg, authors, colors, etc. -; What is the primary ruleset this tileset is meant for. -;for_ruleset = "" - ; Basic tile sizes: normal_tile_width = 96 normal_tile_height = 48 diff --git a/data/hex2t.tilespec b/data/hex2t.tilespec index 117d3fadae..7121ad8da6 100644 --- a/data/hex2t.tilespec +++ b/data/hex2t.tilespec @@ -18,9 +18,6 @@ summary = _("Small hex tileset.") ; TODO: add more overall information fields on tiles, ; eg, authors, colors, etc. -; What is the primary ruleset this tileset is meant for. -;for_ruleset = "" - ; Basic tile sizes: normal_tile_width = 40 normal_tile_height = 72 diff --git a/data/hexemplio.tilespec b/data/hexemplio.tilespec index 2c4e4a7029..a92ea0752f 100644 --- a/data/hexemplio.tilespec +++ b/data/hexemplio.tilespec @@ -20,9 +20,6 @@ summary = _("Large iso-hex tileset, similar to Amplio.") ; TODO: add more overall information fields on tiles, ; eg, authors, colors, etc. -; What is the primary ruleset this tileset is meant for. -;for_ruleset = "" - ; Basic tile sizes: normal_tile_width = 126 normal_tile_height = 64 diff --git a/data/isophex.tilespec b/data/isophex.tilespec index 1e28afbe89..01906fc954 100644 --- a/data/isophex.tilespec +++ b/data/isophex.tilespec @@ -18,9 +18,6 @@ summary = _("Small iso-hex tileset.") ; TODO: add more overall information fields on tiles, ; eg, authors, colors, etc. -; What is the primary ruleset this tileset is meant for. -;for_ruleset = "" - ; Basic tile sizes: normal_tile_width = 64 normal_tile_height = 32 diff --git a/data/isotrident.tilespec b/data/isotrident.tilespec index ce8738b8cb..d10820a85f 100644 --- a/data/isotrident.tilespec +++ b/data/isotrident.tilespec @@ -18,9 +18,6 @@ summary = _("Isometric tileset based on Trident tileset.") ; TODO: add more overall information fields on tiles, ; eg, authors, colors, etc. -; What is the primary ruleset this tileset is meant for. -;for_ruleset = "" - ; Basic tile sizes: normal_tile_width = 64 normal_tile_height = 32 diff --git a/data/toonhex.tilespec b/data/toonhex.tilespec index d9ca51597d..000b568dfd 100644 --- a/data/toonhex.tilespec +++ b/data/toonhex.tilespec @@ -22,9 +22,6 @@ and Cimpletoon's units with orientation.") ; TODO: add more overall information fields on tiles, ; eg, authors, colors, etc. -; What is the primary ruleset this tileset is meant for. -;for_ruleset = "" - ; Basic tile sizes: normal_tile_width = 126 normal_tile_height = 64 diff --git a/data/trident.tilespec b/data/trident.tilespec index dc6aea0e4d..3a278c9774 100644 --- a/data/trident.tilespec +++ b/data/trident.tilespec @@ -17,9 +17,6 @@ summary = _("Small overhead tileset.") ; TODO: add more overall information fields on tiles, ; eg, authors, colors, etc. -; What is the primary ruleset this tileset is meant for. -;for_ruleset = "" - ; Basic tile sizes: normal_tile_width = 30 normal_tile_height = 30