From 1dc68b28c2316526974fc0eba00b5bf9b0ebba6b Mon Sep 17 00:00:00 2001 From: Goober5000 Date: Wed, 29 Oct 2025 00:41:59 -0400 Subject: [PATCH] add `waypoint_stuff_name` A consistent friendly way to get a waypoint name. Follow-up to #7103. --- code/object/waypoint.cpp | 43 +++++++++++++++++++ code/object/waypoint.h | 22 ++++++++++ fred2/calcrelativecoordsdlg.cpp | 13 +++--- fred2/fredview.cpp | 13 +++--- fred2/management.cpp | 11 ++--- fred2/orienteditor.cpp | 6 +-- fred2/sexp_tree.cpp | 6 +-- fred2/ship_select.cpp | 9 ++-- fred2/waypointpathdlg.cpp | 8 ++-- qtfred/src/mission/Editor.cpp | 11 +++-- .../dialogs/ObjectOrientEditorDialogModel.cpp | 6 +-- .../mission/dialogs/SelectionDialogModel.cpp | 9 +--- .../dialogs/WaypointEditorDialogModel.cpp | 8 ++-- qtfred/src/mission/object.cpp | 6 +-- qtfred/src/ui/widgets/sexp_tree.cpp | 6 +-- 15 files changed, 105 insertions(+), 72 deletions(-) diff --git a/code/object/waypoint.cpp b/code/object/waypoint.cpp index a3ae7e55f03..955d29deb2c 100644 --- a/code/object/waypoint.cpp +++ b/code/object/waypoint.cpp @@ -393,6 +393,49 @@ void waypoint_find_unique_name(char *dest_name, int start_index) } while (collision != NULL); } +void waypoint_stuff_name(char *dest, const char *waypoint_list_name, int waypoint_num) +{ + constexpr size_t name_max_len = NAME_LENGTH - 3 - 1 - 1; // a colon, three digits, and a null terminator + + if (waypoint_num < 1) + { + Assertion(LOCATION, "A waypoint number must be at least 1!"); + *dest = 0; + return; + } + if (waypoint_num >= 1000) + { + Error(LOCATION, "This waypoint number has more than three digits! If you actually need this, first convince a coder that you are sane and then ask for the limit to be increased."); + *dest = 0; + return; + } + + strncpy(dest, waypoint_list_name, name_max_len); + sprintf(dest + name_max_len, ":%d", waypoint_num); +} + +void waypoint_stuff_name(SCP_string &dest, const char *waypoint_list_name, int waypoint_num) +{ + constexpr size_t name_max_len = NAME_LENGTH - 3 - 1 - 1; // a colon, three digits, and a null terminator + + if (waypoint_num < 1) + { + Assertion(LOCATION, "A waypoint number must be at least 1!"); + dest = ""; + return; + } + if (waypoint_num >= 1000) + { + Error(LOCATION, "This waypoint number has more than three digits! If you actually need this, first convince a coder that you are sane and then ask for the limit to be increased."); + dest = ""; + return; + } + + dest.assign(waypoint_list_name, name_max_len); + dest += ":"; + dest.append(std::to_string(waypoint_num)); +} + void waypoint_add_list(const char *name, const SCP_vector &vec_list) { Assert(name != NULL); diff --git a/code/object/waypoint.h b/code/object/waypoint.h index 77ee0e8a3bc..b7ace498476 100644 --- a/code/object/waypoint.h +++ b/code/object/waypoint.h @@ -92,6 +92,28 @@ int find_index_of_waypoint(const waypoint_list *wp_list, const waypoint *wpt); // Find a name that doesn't conflict with any current waypoint list void waypoint_find_unique_name(char *dest_name, int start_index); +// Write a waypoint name to a string buffer. Note that waypoint_num is written verbatim, i.e. not adding or subtracting 1. The buffer size must be at least NAME_LENGTH. +void waypoint_stuff_name(char *dest, const char *waypoint_list_name, int waypoint_num); + +// Write a waypoint name to a string buffer. Note that waypoint_num is written verbatim, i.e. not adding or subtracting 1. +void waypoint_stuff_name(SCP_string &dest, const char *waypoint_list_name, int waypoint_num); + +template +void waypoint_stuff_name(STR &dest, const waypoint &wpt) +{ + waypoint_stuff_name(dest, wpt.get_parent_list()->get_name(), wpt.get_index() + 1); +} + +template +void waypoint_stuff_name(STR &dest, int waypoint_instance) +{ + int wl_index, wp_index; + calc_waypoint_indexes(waypoint_instance, wl_index, wp_index); + Assertion(wl_index >= 0, "Waypoint list must exist!"); + Assertion(Waypoint_lists.in_bounds(wp_index), "Waypoint index must be in bounds!"); + waypoint_stuff_name(dest, Waypoint_lists[wl_index].get_name(), wp_index + 1); +} + // Add a new list of waypoints. Called from mission parsing. void waypoint_add_list(const char *name, const SCP_vector &vec_list); diff --git a/fred2/calcrelativecoordsdlg.cpp b/fred2/calcrelativecoordsdlg.cpp index 4ee159b8160..1091f0b8784 100644 --- a/fred2/calcrelativecoordsdlg.cpp +++ b/fred2/calcrelativecoordsdlg.cpp @@ -65,14 +65,11 @@ BOOL calc_relative_coords_dlg::OnInitDialog() } else if (ptr->type == OBJ_WAYPOINT) { - SCP_string text; - int waypoint_num; - - auto wp_list = find_waypoint_list_with_instance(ptr->instance, &waypoint_num); - Assert(wp_list != nullptr); - sprintf(text, "%s:%d", wp_list->get_name(), waypoint_num + 1); - m_origin_list.AddString(text.c_str()); - m_satellite_list.AddString(text.c_str()); + char text[NAME_LENGTH]; + waypoint_stuff_name(text, ptr->instance); + + m_origin_list.AddString(text); + m_satellite_list.AddString(text); added = true; } diff --git a/fred2/fredview.cpp b/fred2/fredview.cpp index 81b9ae2d343..8b95c7fcb33 100644 --- a/fred2/fredview.cpp +++ b/fred2/fredview.cpp @@ -1463,12 +1463,11 @@ void CFREDView::OnContextMenu(CWnd* /*pWnd*/, CPoint point) str.Format("Edit %s", jnp->GetName()); } else if (Objects[objnum].type == OBJ_WAYPOINT) { - int idx; - waypoint_list *wp_list = find_waypoint_list_with_instance(Objects[objnum].instance, &idx); - Assert(wp_list != NULL); + char text[NAME_LENGTH]; + waypoint_stuff_name(text, Objects[objnum].instance); id = ID_EDITORS_WAYPOINT; - str.Format("Edit %s:%d", wp_list->get_name(), idx + 1); + str.Format("Edit %s", text); } else if (Objects[objnum].type == OBJ_POINT) { return; @@ -2574,7 +2573,7 @@ int CFREDView::global_error_check() return internal_error("Object references an illegal waypoint number in path"); } - sprintf(buf, "%s:%d", wp_list->get_name(), waypoint_num + 1); + waypoint_stuff_name(buf, i); names[obj_count] = new char[strlen(buf) + 1]; strcpy(names[obj_count], buf); flags[obj_count] = 1; @@ -2900,8 +2899,8 @@ int CFREDView::global_error_check() } } - for (j = 0; (uint) j < ii.get_waypoints().size(); j++) { - sprintf(buf, "%s:%d", ii.get_name(), j + 1); + for (const auto &jj: ii.get_waypoints()) { + waypoint_stuff_name(buf, jj); for (z=0; zget_parent_list(); - int index = calc_waypoint_list_index(Objects[obj].instance); - int count = (int) wp_list->get_waypoints().size(); + auto count = wp_list->get_waypoints().size(); // we'll end up deleting the path, so check for path references if (count == 1) { @@ -1213,7 +1212,7 @@ int common_object_delete(int obj) } // check for waypoint references - sprintf(msg, "%s:%d", wp_list->get_name(), index + 1); + waypoint_stuff_name(msg, *wpt); name = msg; r = reference_handler(name, sexp_ref_type::WAYPOINT, obj); if (r) @@ -2225,8 +2224,6 @@ int sexp_reference_handler(int node, sexp_src source, int source_index, char *ms char *object_name(int obj) { static char text[80]; - waypoint_list *wp_list; - int waypoint_num; if (!query_valid_object(obj)) return "*none*"; @@ -2237,9 +2234,7 @@ char *object_name(int obj) return Ships[Objects[obj].instance].ship_name; case OBJ_WAYPOINT: - wp_list = find_waypoint_list_with_instance(Objects[obj].instance, &waypoint_num); - Assert(wp_list != NULL); - sprintf(text, "%s:%d", wp_list->get_name(), waypoint_num + 1); + waypoint_stuff_name(text, Objects[obj].instance); return text; case OBJ_POINT: diff --git a/fred2/orienteditor.cpp b/fred2/orienteditor.cpp index a3d93a1e8a7..a5a4c4f181c 100644 --- a/fred2/orienteditor.cpp +++ b/fred2/orienteditor.cpp @@ -129,11 +129,7 @@ BOOL orient_editor::OnInitDialog() index[total++] = objnum; } else if (ptr->type == OBJ_WAYPOINT) { - int waypoint_num; - waypoint_list *wp_list = find_waypoint_list_with_instance(ptr->instance, &waypoint_num); - Assert(wp_list != NULL); - sprintf(text, "%s:%d", wp_list->get_name(), waypoint_num + 1); - + waypoint_stuff_name(text, ptr->instance); box->AddString(text); index[total++] = objnum; diff --git a/fred2/sexp_tree.cpp b/fred2/sexp_tree.cpp index 437886ca36b..8199fa6be7a 100644 --- a/fred2/sexp_tree.cpp +++ b/fred2/sexp_tree.cpp @@ -6448,14 +6448,14 @@ sexp_list_item *sexp_tree::get_listing_opf_subsystem_type(int parent_node) sexp_list_item *sexp_tree::get_listing_opf_point() { - char buf[NAME_LENGTH+8]; + char buf[NAME_LENGTH]; sexp_list_item head; for (const auto &ii: Waypoint_lists) { - for (int j = 0; (uint) j < ii.get_waypoints().size(); ++j) + for (const auto &jj: ii.get_waypoints()) { - sprintf(buf, "%s:%d", ii.get_name(), j + 1); + waypoint_stuff_name(buf, jj); head.add_data(buf); } } diff --git a/fred2/ship_select.cpp b/fred2/ship_select.cpp index d033573d803..a40c4f6a116 100644 --- a/fred2/ship_select.cpp +++ b/fred2/ship_select.cpp @@ -162,7 +162,6 @@ BOOL ship_select::OnInitDialog() void ship_select::create_list() { - SCP_string text; object *ptr; update_status(true); @@ -213,11 +212,9 @@ void ship_select::create_list() { if (ptr->type == OBJ_WAYPOINT) { - int waypoint_num; - waypoint_list *wp_list = find_waypoint_list_with_instance(ptr->instance, &waypoint_num); - Assert(wp_list != NULL); - sprintf(text, "%s:%d", wp_list->get_name(), waypoint_num + 1); - m_ship_list.AddString(text.c_str()); + char text[NAME_LENGTH]; + waypoint_stuff_name(text, ptr->instance); + m_ship_list.AddString(text); obj_index.push_back(OBJ_INDEX(ptr)); if (ptr->flags[Object::Object_Flags::Temp_marked]) m_ship_list.SetSel((int)obj_index.size()); diff --git a/fred2/waypointpathdlg.cpp b/fred2/waypointpathdlg.cpp index ce7aab3da76..58060677725 100644 --- a/fred2/waypointpathdlg.cpp +++ b/fred2/waypointpathdlg.cpp @@ -272,10 +272,10 @@ int waypoint_path_dlg::update_data(int redraw) ai_update_goal_references(sexp_ref_type::WAYPOINT_PATH, old_name, str); for (auto &wpt: cur_waypoint_list->get_waypoints()) { - char old_buf[NAME_LENGTH + 8]; - char new_buf[NAME_LENGTH + 8]; - sprintf(old_buf, "%s:%d", old_name, wpt.get_index() + 1); - sprintf(new_buf, "%s:%d", str, wpt.get_index() + 1); + char old_buf[NAME_LENGTH]; + char new_buf[NAME_LENGTH]; + waypoint_stuff_name(old_buf, old_name, wpt.get_index() + 1); + waypoint_stuff_name(new_buf, str, wpt.get_index() + 1); update_sexp_references(old_buf, new_buf); ai_update_goal_references(sexp_ref_type::WAYPOINT, old_buf, new_buf); } diff --git a/qtfred/src/mission/Editor.cpp b/qtfred/src/mission/Editor.cpp index f27b730aa48..82d6b7a2146 100644 --- a/qtfred/src/mission/Editor.cpp +++ b/qtfred/src/mission/Editor.cpp @@ -1006,8 +1006,7 @@ int Editor::common_object_delete(int obj) { waypoint* wpt = find_waypoint_with_instance(Objects[obj].instance); Assert(wpt != NULL); waypoint_list* wp_list = wpt->get_parent_list(); - int index = calc_waypoint_list_index(Objects[obj].instance); - int count = (int) wp_list->get_waypoints().size(); + auto count = wp_list->get_waypoints().size(); // we'll end up deleting the path, so check for path references if (count == 1) { @@ -1019,7 +1018,7 @@ int Editor::common_object_delete(int obj) { } // check for waypoint references - sprintf(msg, "%s:%d", wp_list->get_name(), index + 1); + waypoint_stuff_name(msg, *wpt); name = msg; r = reference_handler(name, sexp_ref_type::WAYPOINT, obj); if (r) { @@ -2094,7 +2093,7 @@ int Editor::global_error_check_impl() { return internal_error("Object references an illegal waypoint number in path"); } - sprintf(buf, "%s:%d", wp_list->get_name(), waypoint_num + 1); + waypoint_stuff_name(buf, i); names[obj_count] = new char[strlen(buf) + 1]; strcpy(names[obj_count], buf); err_flags[obj_count] = 1; @@ -2436,8 +2435,8 @@ int Editor::global_error_check_impl() { } } - for (j = 0; (uint) j < ii.get_waypoints().size(); j++) { - sprintf(buf, "%s:%d", ii.get_name(), j + 1); + for (const auto &jj: ii.get_waypoints()) { + waypoint_stuff_name(buf, jj); for (z = 0; z < obj_count; z++) { if (names[z]) { if (!stricmp(names[z], buf)) { diff --git a/qtfred/src/mission/dialogs/ObjectOrientEditorDialogModel.cpp b/qtfred/src/mission/dialogs/ObjectOrientEditorDialogModel.cpp index 457580ffed5..80bfdf3da06 100644 --- a/qtfred/src/mission/dialogs/ObjectOrientEditorDialogModel.cpp +++ b/qtfred/src/mission/dialogs/ObjectOrientEditorDialogModel.cpp @@ -40,11 +40,7 @@ void ObjectOrientEditorDialogModel::initializeData() _pointToObjectList.emplace_back(ObjectEntry(Ships[ptr->instance].ship_name, OBJ_INDEX(ptr))); break; case OBJ_WAYPOINT: { - int waypoint_num; - waypoint_list* wp_list = find_waypoint_list_with_instance(ptr->instance, &waypoint_num); - Assertion(wp_list != nullptr, "Waypoint list was nullptr!"); - sprintf(text, "%s:%d", wp_list->get_name(), waypoint_num + 1); - + waypoint_stuff_name(text, ptr->instance); _pointToObjectList.emplace_back(ObjectEntry(text, OBJ_INDEX(ptr))); break; } diff --git a/qtfred/src/mission/dialogs/SelectionDialogModel.cpp b/qtfred/src/mission/dialogs/SelectionDialogModel.cpp index d0dd4430c70..795153b87bc 100644 --- a/qtfred/src/mission/dialogs/SelectionDialogModel.cpp +++ b/qtfred/src/mission/dialogs/SelectionDialogModel.cpp @@ -97,15 +97,8 @@ void SelectionDialogModel::updateObjectList() { auto ptr = GET_FIRST(&obj_used_list); while (ptr != END_OF_LIST(&obj_used_list)) { if (ptr->type == OBJ_WAYPOINT) { - int waypoint_num; - waypoint_list* wp_list = find_waypoint_list_with_instance(ptr->instance, &waypoint_num); - Assert(wp_list != NULL); - - SCP_string text; - sprintf(text, "%s:%d", wp_list->get_name(), waypoint_num + 1); - ListEntry entry; - entry.name = text; + waypoint_stuff_name(entry.name, ptr->instance); entry.id = OBJ_INDEX(ptr); entry.selected = ptr->flags[Object::Object_Flags::Temp_marked]; diff --git a/qtfred/src/mission/dialogs/WaypointEditorDialogModel.cpp b/qtfred/src/mission/dialogs/WaypointEditorDialogModel.cpp index 64473be61b7..f0d240f5c8c 100644 --- a/qtfred/src/mission/dialogs/WaypointEditorDialogModel.cpp +++ b/qtfred/src/mission/dialogs/WaypointEditorDialogModel.cpp @@ -32,10 +32,10 @@ bool WaypointEditorDialogModel::apply() _editor->ai_update_goal_references(sexp_ref_type::WAYPOINT_PATH, old_name, str); for (auto &wpt : _editor->cur_waypoint_list->get_waypoints()) { - char old_buf[NAME_LENGTH + 12]; - char new_buf[NAME_LENGTH + 12]; - sprintf(old_buf, "%s:%d", old_name, wpt.get_index() + 1); - sprintf(new_buf, "%s:%d", str, wpt.get_index() + 1); + char old_buf[NAME_LENGTH]; + char new_buf[NAME_LENGTH]; + waypoint_stuff_name(old_buf, old_name, wpt.get_index() + 1); + waypoint_stuff_name(new_buf, str, wpt.get_index() + 1); update_sexp_references(old_buf, new_buf); _editor->ai_update_goal_references(sexp_ref_type::WAYPOINT, old_buf, new_buf); } diff --git a/qtfred/src/mission/object.cpp b/qtfred/src/mission/object.cpp index a276a1bc6f3..b70cfcf6032 100644 --- a/qtfred/src/mission/object.cpp +++ b/qtfred/src/mission/object.cpp @@ -50,8 +50,6 @@ bool query_valid_object(int index) const char* object_name(int obj) { static char text[80]; - waypoint_list *wp_list; - int waypoint_num; if (!query_valid_object(obj)) return "*none*"; @@ -62,9 +60,7 @@ const char* object_name(int obj) { return Ships[Objects[obj].instance].ship_name; case OBJ_WAYPOINT: - wp_list = find_waypoint_list_with_instance(Objects[obj].instance, &waypoint_num); - Assert(wp_list != NULL); - sprintf(text, "%s:%d", wp_list->get_name(), waypoint_num + 1); + waypoint_stuff_name(text, Objects[obj].instance); return text; case OBJ_POINT: diff --git a/qtfred/src/ui/widgets/sexp_tree.cpp b/qtfred/src/ui/widgets/sexp_tree.cpp index 3562760b57a..788ee80645b 100644 --- a/qtfred/src/ui/widgets/sexp_tree.cpp +++ b/qtfred/src/ui/widgets/sexp_tree.cpp @@ -4347,12 +4347,12 @@ sexp_list_item* sexp_tree::get_listing_opf_subsystem_type(int parent_node) { } sexp_list_item* sexp_tree::get_listing_opf_point() { - char buf[NAME_LENGTH + 8]; + char buf[NAME_LENGTH]; sexp_list_item head; for (const auto &ii: Waypoint_lists) { - for (int j = 0; (uint) j < ii.get_waypoints().size(); ++j) { - sprintf(buf, "%s:%d", ii.get_name(), j + 1); + for (const auto &jj: ii.get_waypoints()) { + waypoint_stuff_name(buf, jj); head.add_data(buf); } }