From a34788ef1a972d9fa696bce0e2b6b5f0f5d9431b Mon Sep 17 00:00:00 2001 From: Goober5000 Date: Sat, 22 Aug 2026 00:22:58 -0400 Subject: [PATCH 1/2] a few parsing and infrastructure upgrades - A parse flag to indicate when the mission is being parsed, and a new `ship_class_index()` accessor in the ship registry, both currently unused - Make the ship registry more robust, especially in FRED: properly update it when ships are renamed or deleted --- code/mission/missionparse.cpp | 7 ++ code/mission/missionparse.h | 3 + code/missionui/redalert.cpp | 5 +- code/network/multi_ingame.cpp | 5 ++ code/scripting/api/objs/ship.cpp | 12 ++- code/ship/ship.cpp | 22 ++++++ code/ship/ship.h | 1 + fred2/freddoc.cpp | 10 +-- fred2/fredview.cpp | 4 + fred2/management.cpp | 72 ++++++++++++------ fred2/management.h | 2 +- fred2/shipeditordlg.cpp | 18 ++--- qtfred/src/mission/Editor.cpp | 74 ++++++++++++------- qtfred/src/mission/Editor.h | 2 +- .../ShipEditor/ShipEditorDialogModel.cpp | 16 +--- qtfred/src/ui/widgets/renderwidget.cpp | 4 + 16 files changed, 166 insertions(+), 91 deletions(-) diff --git a/code/mission/missionparse.cpp b/code/mission/missionparse.cpp index 94aa7836c6c..a340ada13b9 100644 --- a/code/mission/missionparse.cpp +++ b/code/mission/missionparse.cpp @@ -178,6 +178,9 @@ SCP_vector Parse_names; SCP_vector Mission_parse_warnings; +// true while a mission is being parsed and post-processed +bool Parsing_mission = false; + // Routes a parse-time auto-correction notice to the right surface for the app: // outside QtFRED, the existing Warning(LOCATION, ...) popup; inside QtFRED, the // Mission_parse_warnings queue so the ErrorChecker can present it without a popup. @@ -7552,6 +7555,8 @@ bool parse_main(const char *mission_name, int flags) Assert(Ship_info.size() <= MAX_SHIP_CLASSES); + Parsing_mission = true; + do { // don't do this for imports if (!(flags & MPF_IMPORT_FSM)) { @@ -7603,6 +7608,8 @@ bool parse_main(const char *mission_name, int flags) } } while (0); + Parsing_mission = false; + if (!Fred_running) strcpy_s(Mission_filename, mission_name); diff --git a/code/mission/missionparse.h b/code/mission/missionparse.h index 40dbf55614e..0fcfd885db1 100644 --- a/code/mission/missionparse.h +++ b/code/mission/missionparse.h @@ -588,6 +588,9 @@ extern SCP_vector Parse_names; // silently buried. Outside of QtFRED these sites still call Warning(LOCATION, ...). extern SCP_vector Mission_parse_warnings; +// true while a mission is being parsed and post-processed +extern bool Parsing_mission; + extern char Player_start_shipname[NAME_LENGTH]; extern int Player_start_shipnum; extern p_object *Player_start_pobject; diff --git a/code/missionui/redalert.cpp b/code/missionui/redalert.cpp index 702e52f8c6c..dd76218f3d8 100644 --- a/code/missionui/redalert.cpp +++ b/code/missionui/redalert.cpp @@ -969,9 +969,8 @@ void red_alert_bash_ship_status() // give the ship its name from the latest wave // (this will make the ship match to the correct red-alert data) wing_bash_ship_name(shipp, wingp, ((rws->latest_wave - 1) * wingp->wave_count) + 1 + pos_in_wing); - // need to update the ship registry too - strcpy_s(Ship_registry[ship_entry_index].name, shipp->ship_name); - Ship_registry_map[shipp->ship_name] = ship_entry_index; + // need to update the ship registry too; keep the old key because the previous name was a real ship name that may still be referenced + ship_registry_rename(ship_entry_index, shipp->ship_name, false); } } } diff --git a/code/network/multi_ingame.cpp b/code/network/multi_ingame.cpp index 4ca24dd5de4..e8bc0c00a2f 100644 --- a/code/network/multi_ingame.cpp +++ b/code/network/multi_ingame.cpp @@ -1332,7 +1332,12 @@ void process_ingame_wings_packet( ubyte *data, header *hinfo ) // kind of stupid, but bash the name since it won't get recreated properly from // the parse_wing_create_ships call. shipp = &Ships[shipnum]; + int ship_entry_index = ship_registry_get_index(shipp->ship_name); + Assertion(ship_entry_index >= 0, "Ship %s must be in the ship registry!", shipp->ship_name); wing_bash_ship_name(shipp, wingp, which_one + 1); + // need to update the ship registry too + if (ship_entry_index >= 0) + ship_registry_rename(ship_entry_index, shipp->ship_name, true); nprintf(("Network", "Created %s\n", shipp->ship_name)); objp = &Objects[shipp->objnum]; diff --git a/code/scripting/api/objs/ship.cpp b/code/scripting/api/objs/ship.cpp index 4be6e475329..36c3dccaeb8 100644 --- a/code/scripting/api/objs/ship.cpp +++ b/code/scripting/api/objs/ship.cpp @@ -343,7 +343,7 @@ ADE_VIRTVAR(ArmorClass, l_Ship, "string", "Current Armor class", "string", "Armo return ade_set_args(L, "s", name); } -ADE_VIRTVAR(Name, l_Ship, "string", "Ship name. This is the actual name of the ship. Use getDisplayString to get the string which should be displayed to the player.", "string", "Ship name, or empty string if handle is invalid") +ADE_VIRTVAR(Name, l_Ship, "string", "Ship name. This is the actual name of the ship. Use getDisplayString to get the string which should be displayed to the player. Beware of setting the name to the name of an existing ship!", "string", "Ship name, or empty string if handle is invalid") { object_h *objh; const char* s = nullptr; @@ -355,10 +355,18 @@ ADE_VIRTVAR(Name, l_Ship, "string", "Ship name. This is the actual name of the s ship *shipp = &Ships[objh->objp()->instance]; - if(ADE_SETTING_VAR && s != nullptr) { + if(ADE_SETTING_VAR && s != nullptr) + { + int ship_entry_index = ship_registry_get_index(shipp->ship_name); + Assertion(ship_entry_index >= 0, "Ship %s must be in the ship registry!", shipp->ship_name); + auto len = sizeof(shipp->ship_name); strncpy(shipp->ship_name, s, len); shipp->ship_name[len - 1] = 0; + + // need to update the ship registry too + if (ship_entry_index >= 0) + ship_registry_rename(ship_entry_index, shipp->ship_name, true); } return ade_set_args(L, "s", shipp->ship_name); diff --git a/code/ship/ship.cpp b/code/ship/ship.cpp index 66984ad0232..ff90d5140ae 100644 --- a/code/ship/ship.cpp +++ b/code/ship/ship.cpp @@ -229,6 +229,19 @@ ship_info* ship_registry_entry::sip() const } } +int ship_registry_entry::ship_class_index() const +{ + if (shipnum >= 0) + return Ships[shipnum].ship_info_index; + else if (pobj_num >= 0) + return Parse_objects[pobj_num].ship_class; + else + { + Assertion(false, "A ship registry entry must have either a parse object or a ship!"); + return -1; + } +} + SCP_vector Ship_registry; SCP_unordered_map Ship_registry_map; @@ -8684,6 +8697,15 @@ void ship_delete( object * obj ) shipp->weapons.primary_bank_external_model_instance[i] = -1; } } + + // In FRED, clean up the registry so that stale references don't stick around. Conversely, + // in FSO, we need to keep the registry entry so that ships will still be known in the debriefing. + if (Fred_running) + { + auto ship_it = Ship_registry_map.find(shipp->ship_name); + if (ship_it != Ship_registry_map.end()) + Ship_registry_map.erase(ship_it); // don't erase the vector entry to avoid clobbering other indexes + } } /** diff --git a/code/ship/ship.h b/code/ship/ship.h index 407cc48b72c..e3703761d64 100644 --- a/code/ship/ship.h +++ b/code/ship/ship.h @@ -1046,6 +1046,7 @@ struct ship_registry_entry ship* shipp_or_null() const; ship_info* sip() const; + int ship_class_index() const; }; extern SCP_vector Ship_registry; diff --git a/fred2/freddoc.cpp b/fred2/freddoc.cpp index 15aafbfa631..77de4badacc 100644 --- a/fred2/freddoc.cpp +++ b/fred2/freddoc.cpp @@ -331,15 +331,7 @@ bool CFREDDoc::load_mission(const char *pathname, int flags) { wing_bash_ship_name(name, Wings[i].name, j + 1); old_name = Ships[Wings[i].ship_index[j]].ship_name; if (stricmp(name, old_name) != 0) { // need to fix name - update_sexp_references(old_name, name); - ai_update_goal_references(sexp_ref_type::SHIP, old_name, name); - update_texture_replacements(old_name, name); - int k = find_item_with_string(Reinforcements, &reinforcements::name, old_name); - if (k >= 0) { - Assert(strlen(name) < NAME_LENGTH); - strcpy_s(Reinforcements[k].name, name); - } - + rename_ship(Wings[i].ship_index[j], name); // bash it again so that we handle display names if needed wing_bash_ship_name(&Ships[Wings[i].ship_index[j]], &Wings[i], j + 1, true); } diff --git a/fred2/fredview.cpp b/fred2/fredview.cpp index c332b7deb4f..9109d0ef1e8 100644 --- a/fred2/fredview.cpp +++ b/fred2/fredview.cpp @@ -1185,6 +1185,10 @@ void CFREDView::OnLButtonUp(UINT nFlags, CPoint point) Assert(objp->type == OBJ_SHIP); ship = objp->instance; Assert(Ships[ship].wingnum == -1); + char new_name[NAME_LENGTH]; + wing_bash_ship_name(new_name, Wings[Duped_wing].name, Wings[Duped_wing].wave_count + 1); + rename_ship(ship, new_name); + // bash it again for the display name wing_bash_ship_name(&Ships[ship], &Wings[Duped_wing], Wings[Duped_wing].wave_count + 1, true); Wings[Duped_wing].ship_index[Wings[Duped_wing].wave_count] = ship; diff --git a/fred2/management.cpp b/fred2/management.cpp index ff5bd36a424..1d339c84d21 100644 --- a/fred2/management.cpp +++ b/fred2/management.cpp @@ -532,11 +532,40 @@ void fix_prop_name(int prop) void fix_ship_name(int ship) { + char old_name[NAME_LENGTH]; + strcpy_s(old_name, Ships[ship].ship_name); + int i = 1; do { sprintf(Ships[ship].ship_name, "U.R.A. Moron %d", i++); } while (query_ship_name_duplicate(ship)); + + // This function is called when a newly created ship duplicates the name of an existing ship. In + // that situation, ship_create() will have overwritten the existing ship's registry entry to point + // to the new ship, so point it back at the ship that legitimately holds the old name. + auto ship_it = Ship_registry_map.find(old_name); + if (ship_it != Ship_registry_map.end() && Ship_registry[ship_it->second].shipnum == ship) + { + int other_shipnum = ship_name_lookup(old_name, 1); + if (other_shipnum >= 0) + { + auto old_entry = &Ship_registry[ship_it->second]; + old_entry->objnum = Ships[other_shipnum].objnum; + old_entry->shipnum = other_shipnum; + } + else + Ship_registry_map.erase(ship_it); // don't erase the vector entry to avoid clobbering other indexes + } + + // add a fresh registry entry for this ship under its new name + ship_registry_entry entry(Ships[ship].ship_name); + entry.status = ShipStatus::PRESENT; + entry.objnum = Ships[ship].objnum; + entry.shipnum = ship; + + Ship_registry.push_back(entry); + Ship_registry_map[Ships[ship].ship_name] = sz2i(Ship_registry.size() - 1); } int create_ship(matrix *orient, vec3d *pos, int ship_type) @@ -1817,7 +1846,7 @@ int get_docking_list(int model_index) } // DA 1/7/99 These ship names are not variables -int rename_ship(int ship, const char *name) +int rename_ship(int ship, const char *name, bool update_display_name) { Assert(ship >= 0); Assert(strlen(name) < NAME_LENGTH); @@ -1835,35 +1864,34 @@ int rename_ship(int ship, const char *name) // keep the ship registry in sync auto reg_it = Ship_registry_map.find(Ships[ship].ship_name); - if (reg_it != Ship_registry_map.end()) { - int reg_idx = reg_it->second; - Ship_registry_map.erase(reg_it); - strcpy_s(Ship_registry[reg_idx].name, name); - Ship_registry_map[name] = reg_idx; - } + if (reg_it != Ship_registry_map.end()) + ship_registry_rename(reg_it->second, name, true); strcpy_s(Ships[ship].ship_name, name); if (ship == cur_ship) Ship_editor_dialog.m_ship_name = _T(name); - // if this name has a hash, create a default display name - if (get_pointer_to_first_hash_symbol(Ships[ship].ship_name)) + if (update_display_name) { - Ships[ship].display_name = Ships[ship].ship_name; - end_string_at_first_hash_symbol(Ships[ship].display_name); - Ships[ship].flags.set(Ship::Ship_Flags::Has_display_name); + // if this name has a hash, create a default display name + if (get_pointer_to_first_hash_symbol(Ships[ship].ship_name)) + { + Ships[ship].display_name = Ships[ship].ship_name; + end_string_at_first_hash_symbol(Ships[ship].display_name); + Ships[ship].flags.set(Ship::Ship_Flags::Has_display_name); - if (ship == cur_ship) - Ship_editor_dialog.m_ship_display_name = _T(Ships[ship].display_name.c_str()); - } - // otherwise reset the display name - else - { - Ships[ship].display_name = ""; - Ships[ship].flags.remove(Ship::Ship_Flags::Has_display_name); + if (ship == cur_ship) + Ship_editor_dialog.m_ship_display_name = _T(Ships[ship].display_name.c_str()); + } + // otherwise reset the display name + else + { + Ships[ship].display_name = ""; + Ships[ship].flags.remove(Ship::Ship_Flags::Has_display_name); - if (ship == cur_ship) - Ship_editor_dialog.m_ship_display_name = _T(""); + if (ship == cur_ship) + Ship_editor_dialog.m_ship_display_name = _T(""); + } } return 0; diff --git a/fred2/management.h b/fred2/management.h index c99cde1c0bc..e224c658f57 100644 --- a/fred2/management.h +++ b/fred2/management.h @@ -98,7 +98,7 @@ int query_initial_orders_conflict(int wing); int query_initial_orders_empty(ai_goal* ai_goals); int set_reinforcement(const char* name, int state); int get_docking_list(int model_index); -int rename_ship(int ship, const char* name); +int rename_ship(int ship, const char* name, bool update_display_name = true); void fix_ship_name(int ship); int internal_integrity_check(); void correct_marking(); diff --git a/fred2/shipeditordlg.cpp b/fred2/shipeditordlg.cpp index 2b7253810fe..6c70de2ea8b 100644 --- a/fred2/shipeditordlg.cpp +++ b/fred2/shipeditordlg.cpp @@ -1063,7 +1063,7 @@ void CShipEditorDlg::initialize_data(int full_update) // Once the error no longer occurs, bypass mode is cleared and data is updated. int CShipEditorDlg::update_data(int redraw) { - char *str, old_name[255]; + char old_name[255]; object *ptr; int i, z, wing; CSingleLock sync(&CS_cur_object_index), sync2(&CS_update); @@ -1136,17 +1136,11 @@ int CShipEditorDlg::update_data(int redraw) if (z) return z; - strcpy_s(old_name, Ships[single_ship].ship_name); - string_copy(Ships[single_ship].ship_name, m_ship_name, NAME_LENGTH - 1, 1); - str = Ships[single_ship].ship_name; - if (strcmp(old_name, str)) { - update_sexp_references(old_name, str); - ai_update_goal_references(sexp_ref_type::SHIP, old_name, str); - update_texture_replacements(old_name, str); - i = find_item_with_string(Reinforcements, &reinforcements::name, old_name); - if (i >= 0) - strcpy_s(Reinforcements[i].name, str); - + char new_name[NAME_LENGTH]; + string_copy(new_name, m_ship_name, NAME_LENGTH - 1, 1); + if (strcmp(Ships[single_ship].ship_name, new_name) != 0) { + // the display name was already handled in update_ship + rename_ship(single_ship, new_name, false); Update_window = 1; } } diff --git a/qtfred/src/mission/Editor.cpp b/qtfred/src/mission/Editor.cpp index b4f147c9d1e..01aea1925ce 100644 --- a/qtfred/src/mission/Editor.cpp +++ b/qtfred/src/mission/Editor.cpp @@ -337,15 +337,7 @@ bool Editor::loadMission(const std::string& mission_name, int flags) { wing_bash_ship_name(name, Wings[i].name, j + 1); old_name = Ships[Wings[i].ship_index[j]].ship_name; if (stricmp(name, old_name) != 0) { // need to fix name - update_sexp_references(old_name, name); - ai_update_goal_references(sexp_ref_type::SHIP, old_name, name); - update_texture_replacements(old_name, name); - int k = find_item_with_string(Reinforcements, &reinforcements::name, old_name); - if (k >= 0) { - Assert(strlen(name) < NAME_LENGTH); - strcpy_s(Reinforcements[k].name, name); - } - + rename_ship(Wings[i].ship_index[j], name); // bash it again so that we handle display names if needed wing_bash_ship_name(&Ships[Wings[i].ship_index[j]], &Wings[i], j + 1, true); } @@ -805,11 +797,40 @@ bool Editor::query_ship_name_duplicate(int ship) { } void Editor::fix_ship_name(int ship) { + char old_name[NAME_LENGTH]; + strcpy_s(old_name, Ships[ship].ship_name); + int i = 1; do { sprintf(Ships[ship].ship_name, "U.R.A. Moron %d", i++); } while (query_ship_name_duplicate(ship)); + + // This function is called when a newly created ship duplicates the name of an existing ship. In + // that situation, ship_create() will have overwritten the existing ship's registry entry to point + // to the new ship, so point it back at the ship that legitimately holds the old name. + auto ship_it = Ship_registry_map.find(old_name); + if (ship_it != Ship_registry_map.end() && Ship_registry[ship_it->second].shipnum == ship) + { + int other_shipnum = ship_name_lookup(old_name, 1); + if (other_shipnum >= 0) + { + auto old_entry = &Ship_registry[ship_it->second]; + old_entry->objnum = Ships[other_shipnum].objnum; + old_entry->shipnum = other_shipnum; + } + else + Ship_registry_map.erase(ship_it); // don't erase the vector entry to avoid clobbering other indexes + } + + // add a fresh registry entry for this ship under its new name + ship_registry_entry entry(Ships[ship].ship_name); + entry.status = ShipStatus::PRESENT; + entry.objnum = Ships[ship].objnum; + entry.shipnum = ship; + + Ship_registry.push_back(entry); + Ship_registry_map[Ships[ship].ship_name] = sz2i(Ship_registry.size() - 1); } void Editor::createNewMission() { @@ -1434,7 +1455,7 @@ void Editor::update_texture_replacements(const char* old_name, const char* new_n strcpy_s(ii->ship_name, new_name); } } -int Editor::rename_ship(int ship, const char* name) { +int Editor::rename_ship(int ship, const char* name, bool update_display_name) { Assert(ship >= 0); Assert(strlen(name) < NAME_LENGTH); @@ -1451,27 +1472,26 @@ int Editor::rename_ship(int ship, const char* name) { // keep the ship registry in sync auto reg_it = Ship_registry_map.find(Ships[ship].ship_name); - if (reg_it != Ship_registry_map.end()) { - int reg_idx = reg_it->second; - Ship_registry_map.erase(reg_it); - strcpy_s(Ship_registry[reg_idx].name, name); - Ship_registry_map[name] = reg_idx; - } + if (reg_it != Ship_registry_map.end()) + ship_registry_rename(reg_it->second, name, true); strcpy_s(Ships[ship].ship_name, name); - // if this name has a hash, create a default display name - if (get_pointer_to_first_hash_symbol(Ships[ship].ship_name)) - { - Ships[ship].display_name = Ships[ship].ship_name; - end_string_at_first_hash_symbol(Ships[ship].display_name); - Ships[ship].flags.set(Ship::Ship_Flags::Has_display_name); - } - // otherwise reset the display name - else + if (update_display_name) { - Ships[ship].display_name = ""; - Ships[ship].flags.remove(Ship::Ship_Flags::Has_display_name); + // if this name has a hash, create a default display name + if (get_pointer_to_first_hash_symbol(Ships[ship].ship_name)) + { + Ships[ship].display_name = Ships[ship].ship_name; + end_string_at_first_hash_symbol(Ships[ship].display_name); + Ships[ship].flags.set(Ship::Ship_Flags::Has_display_name); + } + // otherwise reset the display name + else + { + Ships[ship].display_name = ""; + Ships[ship].flags.remove(Ship::Ship_Flags::Has_display_name); + } } missionChanged(); diff --git a/qtfred/src/mission/Editor.h b/qtfred/src/mission/Editor.h index 6294913f2ba..630736abddf 100644 --- a/qtfred/src/mission/Editor.h +++ b/qtfred/src/mission/Editor.h @@ -221,7 +221,7 @@ class Editor : public QObject { bool rename_wing(int wing, const SCP_string& new_name, bool rename_members = true); // DA 1/7/99 These ship names are not variables - int rename_ship(int ship, const char* name); + int rename_ship(int ship, const char* name, bool update_display_name = true); /** * @brief Delete a whole wing, leaving ships intact but wingless. diff --git a/qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.cpp b/qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.cpp index 5e998d23178..dd8cfa18bcf 100644 --- a/qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.cpp +++ b/qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.cpp @@ -772,22 +772,10 @@ void ShipEditorDialogModel::setShipName(const SCP_string& m_ship_name) } // All validation passed — write the new name - char old_name[NAME_LENGTH]; - strcpy_s(old_name, Ships[_singleShip].ship_name); - strcpy_s(Ships[_singleShip].ship_name, new_name.c_str()); + // (the display name is handled separately in setShipDisplayName) + _editor->rename_ship(_singleShip, new_name.c_str(), false); _shipName = new_name; - if (strcmp(old_name, Ships[_singleShip].ship_name)) { - update_sexp_references(old_name, Ships[_singleShip].ship_name); - _editor->ai_update_goal_references(sexp_ref_type::SHIP, old_name, Ships[_singleShip].ship_name); - _editor->update_texture_replacements(old_name, Ships[_singleShip].ship_name); - int j = find_item_with_string(Reinforcements, &reinforcements::name, old_name); - if (j >= 0) { - Assert(strlen(Ships[_singleShip].ship_name) < NAME_LENGTH); - strcpy_s(Reinforcements[j].name, Ships[_singleShip].ship_name); - } - } - setModified(); _editor->missionChanged(); modelChanged(); diff --git a/qtfred/src/ui/widgets/renderwidget.cpp b/qtfred/src/ui/widgets/renderwidget.cpp index 10f5b22b9e4..c327e82d8df 100644 --- a/qtfred/src/ui/widgets/renderwidget.cpp +++ b/qtfred/src/ui/widgets/renderwidget.cpp @@ -476,6 +476,10 @@ void RenderWidget::mouseReleaseEvent(QMouseEvent* event) { Assert(objp->type == OBJ_SHIP); ship = objp->instance; Assert(Ships[ship].wingnum == -1); + char new_name[NAME_LENGTH]; + wing_bash_ship_name(new_name, Wings[_viewport->Duped_wing].name, Wings[_viewport->Duped_wing].wave_count + 1); + fred->rename_ship(ship, new_name); + // bash it again for the display name wing_bash_ship_name(&Ships[ship], &Wings[_viewport->Duped_wing], Wings[_viewport->Duped_wing].wave_count + 1, true); Wings[_viewport->Duped_wing].ship_index[Wings[_viewport->Duped_wing].wave_count] = ship; From f759019319a201a91769b148cfe3bd4f5cd561d7 Mon Sep 17 00:00:00 2001 From: Goober5000 Date: Sat, 22 Aug 2026 00:22:58 -0400 Subject: [PATCH 2/2] clean up check_sexp_syntax and improve caching - Clean up `check_sexp_syntax` to use `eval_ship` (and incidentally `eval_wing` and `eval_prop`) rather than the old lookups. - Disable SEXP node caching while FRED is running. Also use smart pointers and fix a cache memory leak. --- code/parse/sexp.cpp | 305 +++++++++++++--------------------- code/parse/sexp.h | 7 +- code/parse/sexp_container.cpp | 2 +- 3 files changed, 121 insertions(+), 193 deletions(-) diff --git a/code/parse/sexp.cpp b/code/parse/sexp.cpp index eea938359d5..aa6738e38c4 100644 --- a/code/parse/sexp.cpp +++ b/code/parse/sexp.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include "ai/aigoals.h" #include "ai/ailua.h" @@ -1277,11 +1278,7 @@ int arg_item::is_empty() void clear_cache(int node) { // free anything cached - if (Sexp_nodes[node].cache) - { - delete Sexp_nodes[node].cache; - Sexp_nodes[node].cache = nullptr; - } + Sexp_nodes[node].cache.reset(); // note that cached_variable_index is not reset here because it is a parallel cache (c.f. sexp_get_variable_index) } @@ -1458,8 +1455,9 @@ int alloc_sexp(const char *text, int type, int subtype, int first, int rest) Verify(Sexp_nodes != nullptr); nprintf(("SEXP", "Bumping dynamic sexp node limit from %d to %d...\n", old_size, Num_sexp_nodes)); - // clear all the new sexp nodes we just allocated - memset(&Sexp_nodes[old_size], 0, sizeof(sexp_node) * SEXP_NODE_INCREMENT); //-V512 + // initialize all the new sexp nodes we just allocated + for (int i = old_size; i < Num_sexp_nodes; i++) + new (&Sexp_nodes[i]) sexp_node(); // our new sexp is the first out of the ones we just created node = old_size; @@ -1478,7 +1476,7 @@ int alloc_sexp(const char *text, int type, int subtype, int first, int rest) Sexp_nodes[node].value = SEXP_UNKNOWN; Sexp_nodes[node].flags = SNF_DEFAULT_VALUE; Sexp_nodes[node].op_index = NO_OPERATOR_INDEX_DEFINED; - Sexp_nodes[node].cache = nullptr; + Sexp_nodes[node].cache.reset(); Sexp_nodes[node].cached_variable_index = -1; Sexp_nodes[node].duration_index = -1; @@ -2467,70 +2465,61 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad break; case OPF_SHIP_NOT_PLAYER: - if (node_subtype != SEXP_ATOM_STRING){ + { + if (node_subtype != SEXP_ATOM_STRING) return SEXP_CHECK_TYPE_MISMATCH; - } - if (ship_name_lookup(CTEXT(node), 0) < 0) + auto ship_entry = eval_ship(node); + if (!ship_entry) + return SEXP_CHECK_INVALID_SHIP; + + if (ship_entry->status == ShipStatus::PRESENT) { - if (Fred_running || !mission_check_ship_yet_to_arrive(CTEXT(node))) - { + if (ship_entry->objp()->flags[Object::Object_Flags::Player_ship]) return SEXP_CHECK_INVALID_SHIP; - } } + // if it's not present, just assume it's okay break; + } case OPF_SHIP_OR_NONE: if (node_subtype != SEXP_ATOM_STRING) - { return SEXP_CHECK_TYPE_MISMATCH; - } - if (stricmp(CTEXT(node), SEXP_NONE_STRING) != 0) // none is okay - { - if (ship_name_lookup(CTEXT(node), 1) < 0) - { - if (Fred_running || !mission_check_ship_yet_to_arrive(CTEXT(node))) - { - return SEXP_CHECK_INVALID_SHIP; - } - } - } + if (stricmp(CTEXT(node), SEXP_NONE_STRING) == 0) // none is okay + break; - break; + if (eval_ship(node)) + break; + + return SEXP_CHECK_INVALID_SHIP; case OPF_SHIP: case OPF_SHIP_POINT: - if (node_subtype != SEXP_ATOM_STRING){ + if (node_subtype != SEXP_ATOM_STRING) return SEXP_CHECK_TYPE_MISMATCH; - } - if (ship_name_lookup(CTEXT(node), 1) < 0) { - if (Fred_running || !mission_check_ship_yet_to_arrive(CTEXT(node))) - { - if (desired_argument_type == OPF_SHIP) - { // return invalid ship if not also looking for point - return SEXP_CHECK_INVALID_SHIP; - } + if (!eval_ship(node)) + { + // return invalid ship if not also looking for point + if (desired_argument_type == OPF_SHIP) + return SEXP_CHECK_INVALID_SHIP; - if (find_matching_waypoint(CTEXT(node)) == nullptr) - { - if (verify_vector(CTEXT(node))) // verify return non-zero on invalid point - { - return SEXP_CHECK_INVALID_SHIP_POINT; - } - } + auto ctext = CTEXT(node); + if (!find_matching_waypoint(ctext)) + { + if (verify_vector(ctext)) // verify return non-zero on invalid point + return SEXP_CHECK_INVALID_SHIP_POINT; } } - break; case OPF_PROP: if (node_subtype != SEXP_ATOM_STRING) { return SEXP_CHECK_TYPE_MISMATCH; } - if (prop_name_lookup(CTEXT(node)) < 0) { + if (eval_prop(node) == nullptr) { return SEXP_CHECK_INVALID_PROP; } break; @@ -2539,8 +2528,7 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad if (node_subtype != SEXP_ATOM_STRING){ return SEXP_CHECK_TYPE_MISMATCH; } - - if (wing_name_lookup(CTEXT(node), 1) < 0){ + if (eval_wing(node) == nullptr) { return SEXP_CHECK_INVALID_WING; } @@ -2563,11 +2551,7 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad } // all of these have ships and wings in common - if (ship_name_lookup(CTEXT(node), 1) >= 0 || wing_name_lookup(CTEXT(node), 1) >= 0) { - break; - } - // also check arrival list if we're running the game - if (!Fred_running && mission_check_ship_yet_to_arrive(CTEXT(node))) { + if (eval_ship(node) || eval_wing(node)) { break; } @@ -2606,18 +2590,12 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad if (node_subtype != SEXP_ATOM_STRING) { return SEXP_CHECK_TYPE_MISMATCH; } - if (ship_name_lookup(CTEXT(node), 1) >= 0) { + if (eval_ship(node)) { break; } - if (prop_name_lookup(CTEXT(node)) >= 0) { + if (eval_prop(node)) { break; } - - // also check arrival list if we're running the game - if (!Fred_running && mission_check_ship_yet_to_arrive(CTEXT(node))) { - break; - } - return SEXP_CHECK_INVALID_SHIP_PROP; case OPF_AWACS_SUBSYSTEM: @@ -2627,7 +2605,6 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad case OPF_SUBSYSTEM_OR_NONE: case OPF_SUBSYS_OR_GENERIC: { - int shipnum,ship_class; int ship_node; if (node_subtype != SEXP_ATOM_STRING){ @@ -2723,32 +2700,21 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad } } - auto shipname = CTEXT(ship_node); - shipnum = ship_name_lookup(shipname, 1); - if (shipnum >= 0) - { - ship_class = Ships[shipnum].ship_info_index; - } - else + auto ship_entry = eval_ship(ship_node); + if (!ship_entry) { - // must try to find the ship in the arrival list - p_object *p_objp = mission_parse_get_arrival_ship(shipname); - - if (!p_objp) - { - if (desired_argument_type == OPF_SUBSYSTEM_OR_NONE) - break; - else - { - if (bad_node) - *bad_node = ship_node; - - return SEXP_CHECK_INVALID_SHIP; - } - } + // for subsystem-or-none, the target may legitimately be a wing, waypoint, or , + // in which case there is no ship class to validate the subsystem against + if (desired_argument_type == OPF_SUBSYSTEM_OR_NONE + && (eval_wing(ship_node) || find_matching_waypoint(CTEXT(ship_node)) + || !stricmp(CTEXT(ship_node), SEXP_NONE_STRING))) + break; - ship_class = p_objp->ship_class; + if (bad_node) + *bad_node = ship_node; + return SEXP_CHECK_INVALID_SHIP; } + auto sip = ship_entry->sip(); // check for the special "hull" value if ( (op_const == OP_SABOTAGE_SUBSYSTEM) || (op_const == OP_REPAIR_SUBSYSTEM) || (op_const == OP_SET_SUBSYSTEM_STRNGTH) || (op_const == OP_SET_ARMOR_TYPE) || (op_const == OP_BEAM_FIRE)) { @@ -2763,15 +2729,15 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad } } - for (i=0; in_subsystems; i++) { - if (!subsystem_stricmp(Ship_info[ship_class].subsystems[i].subobj_name, CTEXT(node))) + if (!subsystem_stricmp(sip->subsystems[i].subobj_name, CTEXT(node))) { break; } } - if (i == Ship_info[ship_class].n_subsystems) + if (i == sip->n_subsystems) { return SEXP_CHECK_INVALID_SUBSYS; } @@ -2779,19 +2745,19 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad if(Fred_running) { // if we're checking for an AWACS subsystem and this is not an awacs subsystem - if((desired_argument_type == OPF_AWACS_SUBSYSTEM) && !(Ship_info[ship_class].subsystems[i].flags[Model::Subsystem_Flags::Awacs])) + if((desired_argument_type == OPF_AWACS_SUBSYSTEM) && !(sip->subsystems[i].flags[Model::Subsystem_Flags::Awacs])) { return SEXP_CHECK_INVALID_AWACS_SUBSYS; } // rotating subsystem, like above - Goober5000 - if ((desired_argument_type == OPF_ROTATING_SUBSYSTEM) && !(Ship_info[ship_class].subsystems[i].flags[Model::Subsystem_Flags::Rotates])) + if ((desired_argument_type == OPF_ROTATING_SUBSYSTEM) && !(sip->subsystems[i].flags[Model::Subsystem_Flags::Rotates])) { return SEXP_CHECK_INVALID_ROTATING_SUBSYS; } // translating subsystem, like above - Goober5000 - if ((desired_argument_type == OPF_TRANSLATING_SUBSYSTEM) && !(Ship_info[ship_class].subsystems[i].flags[Model::Subsystem_Flags::Translates])) + if ((desired_argument_type == OPF_TRANSLATING_SUBSYSTEM) && !(sip->subsystems[i].flags[Model::Subsystem_Flags::Translates])) { return SEXP_CHECK_INVALID_TRANSLATING_SUBSYS; } @@ -2803,7 +2769,6 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad case OPF_ANIMATION_NAME: { // OP 1 is always the ship - int shipnum,ship_class; int ship_node; if (node_subtype != SEXP_ATOM_STRING){ @@ -2823,34 +2788,16 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad } } - auto shipname = CTEXT(ship_node); - shipnum = ship_name_lookup(shipname, 1); - if (shipnum >= 0) - { - ship_class = Ships[shipnum].ship_info_index; - } - else + auto ship_entry = eval_ship(ship_node); + if (!ship_entry) { - // must try to find the ship in the arrival list - p_object *p_objp = mission_parse_get_arrival_ship(shipname); - - if (!p_objp) - { - if (desired_argument_type == OPF_SUBSYSTEM_OR_NONE) - break; - else - { - if (bad_node) - *bad_node = ship_node; - - return SEXP_CHECK_INVALID_SHIP; - } - } - - ship_class = p_objp->ship_class; + if (bad_node) + *bad_node = ship_node; + return SEXP_CHECK_INVALID_SHIP; } + auto sip = ship_entry->sip(); - const auto& animSet = Ship_info[ship_class].animations; + const auto& animSet = sip->animations; switch(op_const) { case OP_TRIGGER_ANIMATION_NEW: case OP_STOP_LOOPING_ANIMATION: { @@ -3019,12 +2966,7 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad valid = 1; } - if (ship_name_lookup(CTEXT(node), 1) >= 0) - { - valid = 1; - } - - if (!Fred_running && mission_check_ship_yet_to_arrive(CTEXT(node))) + if (eval_ship(node)) { valid = 1; } @@ -3055,36 +2997,31 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad case OPF_SHIP_WITH_BAY: { - auto name = CTEXT(node); - int shipnum = -1; - if (node_subtype != SEXP_ATOM_STRING) return SEXP_CHECK_TYPE_MISMATCH; - if (!stricmp(name, "")) + if (!stricmp(CTEXT(node), "")) break; - shipnum = ship_name_lookup(name, 1); - if (shipnum < 0) + auto ship_entry = eval_ship(node); + if (ship_entry) { - if (Fred_running) - return SEXP_CHECK_INVALID_SHIP; - - if (!mission_check_ship_yet_to_arrive(name)) - return SEXP_CHECK_INVALID_SHIP; - - // Goober5000 - since we can't check POFs for ships which have yet to arrive - // (not without a bit of work anyway), just assume they're okay - break; + if (ship_entry->status == ShipStatus::PRESENT) + { + // now determine if this ship has a hangar bay + if (model_has_hangar_bay(ship_entry->sip()->model_num)) + break; + else + return SEXP_CHECK_INVALID_SHIP_WITH_BAY; + } + else + { + // we may not have the model paged in yet, so just assume it's okay + break; + } } - // ship exists at this point - - // now determine if this ship has a hangar bay - if (!ship_has_hangar_bay(shipnum)) - return SEXP_CHECK_INVALID_SHIP_WITH_BAY; - - break; + return SEXP_CHECK_INVALID_SHIP; } case OPF_SUPPORT_SHIP_CLASS: @@ -3180,8 +3117,6 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad } if (Fred_running) { - int ship_num, ship2, wing_num = 0; - // if it's the "goals" operator, this is part of initial orders, so we can't grab the ship from it if (op_const == OP_GOALS_ID) { break; @@ -3201,32 +3136,28 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad } } - ship_num = ship_name_lookup(CTEXT(ship_node), 1); // Goober5000 - include players - if (ship_num < 0) { - wing_num = wing_name_lookup(CTEXT(ship_node)); - if (wing_num < 0) { - if (bad_node){ - *bad_node = ship_node; - } - - return SEXP_CHECK_INVALID_SHIP; // should have already been caught earlier, but just in case.. - } + auto ship_entry = eval_ship(ship_node); + auto wingp = eval_wing(ship_node); + if (!ship_entry && !wingp) { + if (bad_node) + *bad_node = ship_node; + return SEXP_CHECK_INVALID_SHIP; // should have already been caught earlier, but just in case.. } Assert(node_subtype == SEXP_ATOM_LIST); z = Sexp_nodes[node].first; Assert(Sexp_nodes[z].subtype != SEXP_ATOM_LIST); z = get_operator_const(z); - if (ship_num >= 0) { - if (!query_sexp_ai_goal_valid(z, ship_num)){ + if (ship_entry) { + if (!query_sexp_ai_goal_valid(z, ship_entry->shipnum)) { if (bad_node) *bad_node = ship_node; return SEXP_CHECK_ORDER_NOT_ALLOWED; } } else { - for (i=0; iwave_count; i++){ + if (!query_sexp_ai_goal_valid(z, wingp->ship_index[i])){ if (bad_node) *bad_node = ship_node; return SEXP_CHECK_ORDER_NOT_ALLOWED; @@ -3235,8 +3166,8 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad } if ((z == OP_AI_DOCK) && (Sexp_nodes[node].rest >= 0)) { - ship2 = ship_name_lookup(CTEXT(Sexp_nodes[node].rest), 1); // Goober5000 - include players - if ((ship_num < 0) || !ship_docking_valid(ship_num, ship2)){ + auto ship_entry2 = eval_ship(Sexp_nodes[node].rest); + if (!ship_entry || !ship_entry2 || !ship_docking_valid(ship_entry->shipnum, ship_entry2->shipnum)){ if (bad_node) *bad_node = ship_node; return SEXP_CHECK_DOCKING_NOT_ALLOWED; @@ -3420,7 +3351,7 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad // This makes massive assumptions about the structure of the SEXP using it. If you add any // new SEXPs that use this OPF, you will probably need to edit this section to accommodate them. if (Fred_running) { - int ship_num, ship_node = -1, model; + int ship_node = -1, model; // Look for the node containing the docker/dockee ship. In most cases, we want // the current SEXP operator, but for ai-dock and the docker, we want its parent. @@ -3494,15 +3425,14 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad } // look for the ship that has this dockpoint - ship_num = ship_name_lookup(CTEXT(ship_node), 1); - if (ship_num < 0) { + auto ship_entry = eval_ship(ship_node); + if (!ship_entry) { if (bad_node) *bad_node = ship_node; - return SEXP_CHECK_INVALID_SHIP; // should have already been caught earlier, but just in case.. } - model = Ship_info[Ships[ship_num].ship_info_index].model_num; + model = ship_entry->sip()->model_num; z = model_get_num_dock_points(model); for (i=0; i") != 0) if (stricmp(CTEXT(node), "") != 0 ) // not a special token? - if ((ship_name_lookup(CTEXT(node), 1) < 0) && (wing_name_lookup(CTEXT(node), 1) < 0)) // is it in the mission? - if (Fred_running || !mission_check_ship_yet_to_arrive(CTEXT(node))) - return SEXP_CHECK_INVALID_MSG_SOURCE; + if (!eval_ship(node) && !eval_wing(node)) // is it a ship or wing? + return SEXP_CHECK_INVALID_MSG_SOURCE; } break; @@ -5857,9 +5786,9 @@ const ship_registry_entry *eval_ship(int node) } if (ship_it != Ship_registry_map.end()) { - // cache the value if it can't change later - if (!is_node_value_dynamic(node)) - Sexp_nodes[node].cache = new sexp_cached_data(OPF_SHIP, -1, ship_it->second); + // cache the value if it can't change later and we're in-game + if (!Fred_running && !is_node_value_dynamic(node)) + Sexp_nodes[node].cache = std::make_unique(OPF_SHIP, -1, ship_it->second); return &Ship_registry[ship_it->second]; } @@ -5900,9 +5829,9 @@ const prop *eval_prop(int node) auto prop_idx = prop_name_lookup(CTEXT(node)); if (prop_idx >= 0) { - // cache the value if it can't change later - if (!is_node_value_dynamic(node)) - Sexp_nodes[node].cache = new sexp_cached_data(OPF_PROP, -1, prop_idx); + // cache the value if it can't change later and we're in-game + if (!Fred_running && !is_node_value_dynamic(node)) + Sexp_nodes[node].cache = std::make_unique(OPF_PROP, -1, prop_idx); return prop_id_lookup(prop_idx); } @@ -5939,12 +5868,13 @@ wing *eval_wing(int node) return eval_wing(arg_node); } - int wing_num = wing_lookup(CTEXT(node)); + auto wing_name = CTEXT(node); + int wing_num = Fred_running ? wing_name_lookup(wing_name) : wing_lookup(wing_name); if (wing_num >= 0) { - // cache the value if it can't change later - if (!is_node_value_dynamic(node)) - Sexp_nodes[node].cache = new sexp_cached_data(OPF_WING, wing_num); + // cache the value if it can't change later and we're in-game + if (!Fred_running && !is_node_value_dynamic(node)) + Sexp_nodes[node].cache = std::make_unique(OPF_WING, wing_num); return &Wings[wing_num]; } @@ -5989,12 +5919,9 @@ int sexp_atoi(int node) int num = atoi(CTEXT(node)); ensure_opf_positive_is_positive(node, num); - if (!Fred_running) - { - // cache the value if it can't change later - if (!is_node_value_dynamic(node)) - Sexp_nodes[node].cache = new sexp_cached_data(OPF_NUMBER, num, -1); - } + // cache the value if it can't change later and we're in-game + if (!Fred_running && !is_node_value_dynamic(node)) + Sexp_nodes[node].cache = std::make_unique(OPF_NUMBER, num, -1); return num; } @@ -25213,9 +25140,9 @@ int sexp_string_to_int(int n) int num = atoi(buf); - // cache the value if it can't change later - if (!is_node_value_dynamic(n)) - Sexp_nodes[n].cache = new sexp_cached_data(OPF_NUMBER, num, -1); + // cache the value if it can't change later and we're in-game + if (!Fred_running && !is_node_value_dynamic(n)) + Sexp_nodes[n].cache = std::make_unique(OPF_NUMBER, num, -1); return num; } diff --git a/code/parse/sexp.h b/code/parse/sexp.h index f5f85a9b020..019b0ba364d 100644 --- a/code/parse/sexp.h +++ b/code/parse/sexp.h @@ -1389,7 +1389,8 @@ struct sexp_cached_data } }; -typedef struct sexp_node { +struct sexp_node +{ char text[TOKEN_LENGTH]; int op_index; // the index in the Operators array for the operator at this node (or -1 if not an operator) int type; // atom, list, or not used @@ -1399,11 +1400,11 @@ typedef struct sexp_node { int value; // known to be true, known to be false, or not known int flags; // Goober5000 - sexp_cached_data *cache; // Goober5000 + std::unique_ptr cache; // Goober5000 int cached_variable_index; // Goober5000 - note, this can be used for special-arg nodes, not just variable nodes int duration_index; // Goober5000 - only used if node is the is-true-for-duration operator -} sexp_node; +}; // Goober5000 #define SNF_ARGUMENT_VALID (1<<0) diff --git a/code/parse/sexp_container.cpp b/code/parse/sexp_container.cpp index 8877ca53596..0e5f3861ea6 100644 --- a/code/parse/sexp_container.cpp +++ b/code/parse/sexp_container.cpp @@ -848,7 +848,7 @@ const char *sexp_container_CTEXT(int node) if (result.front() != sexp_container::DELIM) { if (!Sexp_nodes[node].cache) { - Sexp_nodes[node].cache = new sexp_cached_data(OPF_CONTAINER_NAME, result); + Sexp_nodes[node].cache = std::make_unique(OPF_CONTAINER_NAME, result); } else { Sexp_nodes[node].cache->update_container_CTEXT_result(result); }