From 7e9fd018793fdbf30c6102fa9f0569eea51b69af Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Tue, 6 Apr 2021 03:31:17 +0200 Subject: [PATCH 1/7] Give names to units The name is empty by default, but can be set by sending unit_rename packets. It is limited to MAX_LEN_NAME which is 48 bytes including the terminating zero. Requested by jwrober in #281. --- client/packhand.cpp | 3 +++ common/networking/packets.def | 7 +++++++ common/unit.h | 1 + server/unithand.cpp | 13 +++++++++++++ server/unittools.cpp | 2 ++ 5 files changed, 26 insertions(+) diff --git a/client/packhand.cpp b/client/packhand.cpp index 372e23f385..a990b6692e 100644 --- a/client/packhand.cpp +++ b/client/packhand.cpp @@ -194,6 +194,8 @@ static struct unit *unpackage_unit(const struct packet_unit_info *packet) unit_tile_set(punit, index_to_tile(&(wld.map), packet->tile)); punit->facing = packet->facing; punit->homecity = packet->homecity; + punit->name = QString::fromUtf8(QByteArray(packet->name, + ARRAY_SIZE(packet->name))); output_type_iterate(o) { punit->upkeep[o] = packet->upkeep[o]; } output_type_iterate_end; punit->moves_left = packet->movesleft; @@ -290,6 +292,7 @@ unpackage_short_unit(const struct packet_unit_short_info *packet) punit->veteran = packet->veteran; punit->hp = packet->hp; punit->activity = static_cast(packet->activity); + punit->name = QString::fromUtf8(packet->name, ARRAY_SIZE(packet->name)); if (packet->activity_tgt == EXTRA_NONE) { punit->activity_target = NULL; diff --git a/common/networking/packets.def b/common/networking/packets.def index 31364901f4..79c428f8b4 100644 --- a/common/networking/packets.def +++ b/common/networking/packets.def @@ -1040,6 +1040,7 @@ PACKET_UNIT_INFO = 63; sc, lsend, is-game-info, cancel(PACKET_UNIT_SHORT_INFO) EXTRA changed_from_tgt; SINT8 battlegroup; + STRING name[MAX_LEN_NAME]; BOOL has_orders; UINT16 orders_length, orders_index; @@ -1059,6 +1060,7 @@ PACKET_UNIT_SHORT_INFO = 64; sc, lsend, is-game-info, force, cancel(PACKET_UNIT_ UINT8 veteran; BOOL occupied, transported; + STRING name[MAX_LEN_NAME]; UINT8 hp, activity; EXTRA activity_tgt; @@ -1152,6 +1154,11 @@ PACKET_UNIT_ACTIONS = 90; sc, dsend ACT_PROB action_probabilities[MAX_NUM_ACTIONS]; end +PACKET_UNIT_RENAME = 91; cs, dsend + UNIT unit_id; + STRING name[MAX_LEN_NAME]; +end + PACKET_UNIT_CHANGE_ACTIVITY = 222; cs, dsend UNIT unit_id; ACTIVITY activity; diff --git a/common/unit.h b/common/unit.h index 79182cbc38..646affa3a4 100644 --- a/common/unit.h +++ b/common/unit.h @@ -127,6 +127,7 @@ struct unit { struct player *nationality; int id; int homecity; + QString name; int upkeep[O_LAST]; // unit upkeep with regards to the homecity diff --git a/server/unithand.cpp b/server/unithand.cpp index b2950236b8..60a094d455 100644 --- a/server/unithand.cpp +++ b/server/unithand.cpp @@ -1940,6 +1940,19 @@ void handle_unit_get_actions(struct connection *pc, const int actor_unit_id, } } +/** + * Handle request to rename a unit. + */ +void handle_unit_rename(player *pplayer, int unit_id, const char *name) +{ + auto unit = game_unit_by_number(unit_id); + fc_assert_ret(unit != nullptr); + fc_assert_ret(pplayer == unit->owner); + + // Use the QByteArray overload to prevent unbounded read + unit->name = QString::fromUtf8(name, MAX_LEN_NAME); +} + /** Try to explain to the player why an action is illegal. diff --git a/server/unittools.cpp b/server/unittools.cpp index 2db673d031..360c2ad835 100644 --- a/server/unittools.cpp +++ b/server/unittools.cpp @@ -2539,6 +2539,7 @@ void package_unit(struct unit *punit, struct packet_unit_info *packet) packet->tile = tile_index(unit_tile(punit)); packet->facing = punit->facing; packet->homecity = punit->homecity; + fc_strlcpy(packet->name, punit->name.toUtf8(), ARRAY_SIZE(packet->name)); output_type_iterate(o) { packet->upkeep[o] = punit->upkeep[o]; } output_type_iterate_end; packet->veteran = punit->veteran; @@ -2624,6 +2625,7 @@ void package_short_unit(struct unit *punit, packet->type = utype_number(unit_type_get(punit)); packet->hp = punit->hp; packet->occupied = (get_transporter_occupancy(punit) > 0); + fc_strlcpy(packet->name, punit->name.toUtf8(), ARRAY_SIZE(packet->name)); if (punit->activity == ACTIVITY_EXPLORE || punit->activity == ACTIVITY_GOTO) { packet->activity = ACTIVITY_IDLE; From bfb0e963be1d511dacdaaffbade6d474da19fa30 Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Tue, 6 Apr 2021 03:37:51 +0200 Subject: [PATCH 2/7] Save unit names See #281. --- server/savegame/savegame3.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/server/savegame/savegame3.cpp b/server/savegame/savegame3.cpp index e20435a2be..4f5810a9f6 100644 --- a/server/savegame/savegame3.cpp +++ b/server/savegame/savegame3.cpp @@ -5668,6 +5668,12 @@ static bool sg_load_player_unit(struct loaddata *loading, struct player *plr, sg_warn_ret_val(secfile_lookup_int(loading->file, &punit->homecity, "%s.homecity", unitstr), false, "%s", secfile_error()); + + if (auto name = secfile_lookup_str(loading->file, "%s.name", unitstr)) { + // Support earlier saves + punit->name = QString::fromUtf8(name); + } + sg_warn_ret_val(secfile_lookup_int(loading->file, &punit->moves_left, "%s.moves", unitstr), false, "%s", secfile_error()); @@ -6206,6 +6212,8 @@ static void sg_save_player_units(struct savedata *saving, struct player *plr) secfile_insert_int(saving->file, punit->veteran, "%s.veteran", buf); secfile_insert_int(saving->file, punit->hp, "%s.hp", buf); secfile_insert_int(saving->file, punit->homecity, "%s.homecity", buf); + secfile_insert_str(saving->file, punit->name.toUtf8(), "%s.name", buf); + secfile_insert_str(saving->file, unit_rule_name(punit), "%s.type_by_name", buf); From 60ad9d703f4bab8f6cca650ad1b66652e46af0e5 Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Tue, 6 Apr 2021 04:20:48 +0200 Subject: [PATCH 3/7] Add a UI to change unit names See #281. --- client/gui-qt/menu.cpp | 40 ++++++++++++++++++++++++++++++++++++++++ client/gui-qt/menu.h | 2 ++ 2 files changed, 42 insertions(+) diff --git a/client/gui-qt/menu.cpp b/client/gui-qt/menu.cpp index 51e49c7872..6e888664f8 100644 --- a/client/gui-qt/menu.cpp +++ b/client/gui-qt/menu.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -836,6 +837,11 @@ void mr_menu::setup_menus() menu_list.insert(DISBAND, act); connect(act, &QAction::triggered, this, &mr_menu::slot_disband); + menu->addSeparator(); + act = menu->addAction(_("Rename...")); + menu_list.insert(RENAME, act); + connect(act, &QAction::triggered, this, &mr_menu::slot_rename); + // Combat Menu menu = this->addMenu(_("Combat")); act = menu->addAction(_("Fortify Unit")); @@ -1866,6 +1872,10 @@ void mr_menu::menus_sensitive() } break; + case RENAME: + i.value()->setEnabled(get_num_units_in_focus() == 1); + break; + case CONNECT_RAIL: proad = road_by_compat_special(ROCO_RAILROAD); if (proad != NULL) { @@ -2178,6 +2188,36 @@ void mr_menu::slot_convert() { key_unit_convert(); } */ void mr_menu::slot_disband() { popup_disband_dialog(get_units_in_focus()); } +/** + * Action "RENAME UNIT" + */ +void mr_menu::slot_rename() +{ + if (get_num_units_in_focus() != 1) { + return; + } + unit_list_iterate(get_units_in_focus(), punit) + { + auto ask = new hud_input_box(king()->central_wdg); + + ask->set_text_title_definput(_("New unit name:"), _("Rename Unit"), + punit->name); + ask->setAttribute(Qt::WA_DeleteOnClose); + + int id = punit->id; + connect(ask, &QDialog::accepted, [ask, id]() { + // Unit might have been removed, make sure it's still there + auto unit = game_unit_by_number(id); + if (unit) { + dsend_packet_unit_rename(&client.conn, id, + ask->input_edit.text().toUtf8()); + } + }); + ask->show(); + } + unit_list_iterate_end; +} + /** Clears delayed orders */ diff --git a/client/gui-qt/menu.h b/client/gui-qt/menu.h index 37ccded049..3034bfad12 100644 --- a/client/gui-qt/menu.h +++ b/client/gui-qt/menu.h @@ -34,6 +34,7 @@ enum munit { UNLOAD, TRANSPORTER, DISBAND, + RENAME, CONVERT, MINE, PLANT, @@ -256,6 +257,7 @@ private slots: void slot_upgrade(); void slot_convert(); void slot_disband(); + void slot_rename(); /*used by combat menu*/ void slot_unit_fortify(); From 4b8df2c4d16e7a870e1048859ba2912373181622 Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Tue, 6 Apr 2021 04:36:44 +0200 Subject: [PATCH 4/7] Show unit names in the UI * In the middle click popup * In the unit orders panel See #281. --- client/gui-qt/hudwidget.cpp | 24 ++++++++++++++++++++---- client/text.cpp | 20 +++++++++++++++----- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/client/gui-qt/hudwidget.cpp b/client/gui-qt/hudwidget.cpp index 4b09c9afe3..bc64b16799 100644 --- a/client/gui-qt/hudwidget.cpp +++ b/client/gui-qt/hudwidget.cpp @@ -604,12 +604,28 @@ void hud_units::update_actions(unit_list *punits) tmp = tileset; tileset = unscaled_tileset; } - text_str = QString(unit_name_translation(punit)); owner = punit->owner; pcity = player_city_by_number(owner, punit->homecity); - if (pcity != NULL) { - text_str = QString(("%1(%2)")) - .arg(unit_name_translation(punit), city_name_get(pcity)); + if (punit->name.isEmpty()) { + if (pcity == nullptr) { + text_str = QString(unit_name_translation(punit)); + } else { + // TRANS: () + text_str = + QString(_("%1 (%2)")) + .arg(unit_name_translation(punit), city_name_get(pcity)); + } + } else { + if (pcity == nullptr) { + // TRANS: "" + text_str = QString(_("%1 \"%2\"")) + .arg(punit->name, unit_name_translation(punit)); + } else { + // TRANS: "" () + text_str = QString(_("%1 \"%2\" (%3)")) + .arg(unit_name_translation(punit), punit->name, + city_name_get(pcity)); + } } text_str = text_str + " "; mp = QString(move_points_text(punit->moves_left, false)); diff --git a/client/text.cpp b/client/text.cpp index e5b986a6c5..4e1d636ba4 100644 --- a/client/text.cpp +++ b/client/text.cpp @@ -372,11 +372,21 @@ const QString popup_info_text(struct tile *ptile) if (!client_player() || owner == client_player()) { struct city *hcity = player_city_by_number(owner, punit->homecity); - // TRANS: "Unit: | ()" - str = str - + QString(_("Unit: %1 | %2 (%3)")) - .arg(utype_name_translation(ptype), username, nation) - + qendl(); + if (punit->name.isEmpty()) { + // TRANS: "Unit: | ()" + str = str + + QString(_("Unit: %1 | %2 (%3)")) + .arg(utype_name_translation(ptype), username, nation) + + qendl(); + } else { + // TRANS: "Unit: "" | ()" + str = str + + QString(_("Unit: %1 \"%2\" | %3 (%4)")) + .arg(utype_name_translation(ptype), punit->name, + username, nation) + + qendl(); + } if (game.info.citizen_nationality && unit_nationality(punit) != unit_owner(punit)) { if (hcity != NULL) { From d2e819346e6b8e95d90087d9d1ce8e8a22b08fbb Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Tue, 6 Apr 2021 04:47:15 +0200 Subject: [PATCH 5/7] Send updated unit when it is renamed See #281. --- server/unithand.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/server/unithand.cpp b/server/unithand.cpp index 60a094d455..676b29b77a 100644 --- a/server/unithand.cpp +++ b/server/unithand.cpp @@ -1951,6 +1951,7 @@ void handle_unit_rename(player *pplayer, int unit_id, const char *name) // Use the QByteArray overload to prevent unbounded read unit->name = QString::fromUtf8(name, MAX_LEN_NAME); + send_unit_info(NULL, unit); } /** From 3ce1b2d45d6dd02cfcb387e8fdca7638af470006 Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Tue, 6 Apr 2021 04:50:25 +0200 Subject: [PATCH 6/7] Update units on the client when their name changes This makes the change instantaneous, it required a reconnect before. See #281. --- client/packhand.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/client/packhand.cpp b/client/packhand.cpp index a990b6692e..dc8c540580 100644 --- a/client/packhand.cpp +++ b/client/packhand.cpp @@ -1707,6 +1707,16 @@ static bool handle_unit_packet_common(struct unit *packet_unit) need_units_report_update = true; } + if (punit->name != packet_unit->name) { + // Name changed + punit->name = packet_unit->name; + + if (unit_is_in_focus(punit)) { + // Update the orders menu -- the name is shown there + need_menus_update = true; + } + } + if (punit->hp != packet_unit->hp) { // hp changed punit->hp = packet_unit->hp; From 2f94c15382344866e88687dc267b52c782b44c12 Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Tue, 6 Apr 2021 05:00:50 +0200 Subject: [PATCH 7/7] Bump network capstring --- utility/fc_version.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utility/fc_version.h b/utility/fc_version.h index ad115ac87f..a0e37631cf 100644 --- a/utility/fc_version.h +++ b/utility/fc_version.h @@ -11,7 +11,7 @@ #define VERSION_STRING "3.0.1337.1-haxx" #endif -#define NETWORK_CAPSTRING "+Freeciv21.21March06" +#define NETWORK_CAPSTRING "+Freeciv21.21April06" #ifndef FOLLOWTAG #define FOLLOWTAG "S_HAXXOR"