From 246213a4bef19e2914a426e3a41fc5769ad5d858 Mon Sep 17 00:00:00 2001 From: Goober5000 Date: Wed, 13 May 2026 00:44:07 -0400 Subject: [PATCH 1/4] first part of comm node fix Fix #4089, part 1: - give the `my_replacement` and `i_replace` fields clearer names - for submodels with the `Is_damaged` flag (representing -destroyed variants or debris), initialize them to blown-off (the true fix for #3078, fixing a regression introduced in the model instance refactor) - roll back the de-parenting of live debris from #3089 (726ce953c73a20ba8ad6de2ea8e322e947cdd61a), because it was a wrong fix to the initialization bug and introduced its own subtle bug when positioning live debris - roll back the `Can_move` flag skipping from #4138 (063d7afcfe230ad790d4ea55d0a9667bd1659fda), because it was another wrong fix to the initialization bug - add some clarifying comments --- code/model/model.h | 8 ++--- code/model/modelread.cpp | 60 +++++++++++++++++++++---------------- code/model/modelreplace.cpp | 4 +-- 3 files changed, 41 insertions(+), 31 deletions(-) diff --git a/code/model/model.h b/code/model/model.h index 3cff8481624..fee1de9bb3b 100644 --- a/code/model/model.h +++ b/code/model/model.h @@ -136,7 +136,7 @@ struct submodel_instance float shift_accel = 0.0f; TIMESTAMP stepped_translation_started; - bool blown_off = false; // If set, this subobject is blown off + bool blown_off = false; // If set, this subobject is not rendered or used for collision // These fields are the true standard reference for submodel rotation. They should seldom be read directly // and should almost never be written directly. In most cases, coders should prefer cur_angle and prev_angle. @@ -433,7 +433,7 @@ class bsp_info public: bsp_info() : bsp_data_size(0), bsp_data(nullptr), collision_tree_index(-1), - rad(0.0f), my_replacement(-1), i_replace(-1), num_live_debris(0), + rad(0.0f), next_form(-1), prev_form(-1), num_live_debris(0), parent(-1), num_children(0), first_child(-1), next_sibling(-1), num_details(0), outline_buffer(nullptr), n_verts_outline(0), render_sphere_radius(0.0f), use_render_box(0), use_render_sphere(0) { @@ -484,8 +484,8 @@ class bsp_info vec3d max; // The max point of this object's geometry vec3d bounding_box[8]; // calculated fron min/max - int my_replacement; // If not -1 this subobject is what should get rendered instead of this one - int i_replace; // If this is not -1, then this subobject will replace i_replace when it is damaged + int next_form; // If not -1, this submodel can transform into it + int prev_form; // If not -1, another submodel that can transform into this one int num_live_debris; // num live debris models assocaiated with a submodel int live_debris[MAX_LIVE_DEBRIS]; // array of live debris submodels for a submodel diff --git a/code/model/modelread.cpp b/code/model/modelread.cpp index 44015e877d0..176ab4307eb 100644 --- a/code/model/modelread.cpp +++ b/code/model/modelread.cpp @@ -3333,22 +3333,20 @@ int model_load(const char* filename, ship_info* sip, ErrorType error_type, bool // Set up the default values for (i=0; in_models; i++ ) { - pm->submodel[i].my_replacement = -1; // assume nothing replaces this - pm->submodel[i].i_replace = -1; // assume this doesn't replaces anything + pm->submodel[i].next_form = -1; // assume nothing replaces this + pm->submodel[i].prev_form = -1; // assume this doesn't replace anything } // Search for models that have destroyed versions for (i=0; in_models; i++ ) { - int j; char destroyed_name[128]; - strcpy_s( destroyed_name, pm->submodel[i].name ); strcat_s( destroyed_name, "-destroyed" ); - for (j=0; jn_models; j++ ) { - if ( !stricmp( pm->submodel[j].name, destroyed_name )) { - pm->submodel[i].my_replacement = j; - pm->submodel[j].i_replace = i; - } + + int j = find_item_with_string(pm->submodel.get(), i2sz(pm->n_models), &bsp_info::name, destroyed_name); + if (j >= 0) { + pm->submodel[i].next_form = j; + pm->submodel[j].prev_form = i; } // Search for models with live debris @@ -3366,12 +3364,8 @@ int model_load(const char* filename, ship_info* sip, ErrorType error_type, bool Assert(pm->submodel[i].num_live_debris < MAX_LIVE_DEBRIS); pm->submodel[i].live_debris[pm->submodel[i].num_live_debris++] = j; pm->submodel[j].flags.set(Model::Submodel_flags::Is_live_debris); - - // make sure live debris doesn't have a parent - pm->submodel[j].parent = -1; } } - } // maybe generate vertex buffers @@ -3554,9 +3548,17 @@ int model_create_instance(int objnum, int model_num) } pmi->id = open_slot; - if (pm->n_models > 0) + if (pm->n_models > 0) { pmi->submodel = new submodel_instance[pm->n_models]; + // "damaged" submodels (like -destroyed variants, or debris) are blown-off by default + for (int i = 0; i < pm->n_models; i++) { + if (pm->submodel[i].flags[Model::Submodel_flags::Is_damaged]) { + pmi->submodel[i].blown_off = true; + } + } + } + // add intrinsic_motion instances if this model is intrinsic-moving if (pm->flags & PM_FLAG_HAS_INTRINSIC_MOTION) { intrinsic_motion motion(objnum >= 0, open_slot); @@ -4836,8 +4838,9 @@ void model_get_moving_submodel_list(SCP_vector &submodel_vector, const obje const auto& child_submodel = pm->submodel[submodel]; const auto& child_submodel_instance = pmi->submodel[submodel]; - // Don't check it or its children if it is destroyed or it is a replacement (non-moving) - if (child_submodel.flags[Model::Submodel_flags::No_collisions] || child_submodel_instance.blown_off || child_submodel.i_replace != -1) { + // Don't check it or its children if it is destroyed or it is a replacement + // (we currently assume replacements are -destroyed versions of submodels that might otherwise move) + if (child_submodel.flags[Model::Submodel_flags::No_collisions] || child_submodel_instance.blown_off || child_submodel.prev_form != -1) { skipChildren = true; return; } @@ -4964,7 +4967,10 @@ void model_set_up_techroom_instance(ship_info *sip, int model_instance_num) model_iterate_submodel_tree(pm, pm->detail[0], [&](int submodel, int /*level*/, bool /*isLeaf*/) { - model_replicate_submodel_instance(pm, pmi, submodel, empty); + auto sm = &pm->submodel[submodel]; + + if (sm->flags[Model::Submodel_flags::Can_move]) + model_replicate_submodel_instance(pm, pmi, submodel, empty); }); } @@ -4985,17 +4991,21 @@ void model_replicate_submodel_instance_sub(polymodel *pm, polymodel_instance *pm submodel_instance *smi = &pmi->submodel[submodel_num]; bsp_info *sm = &pm->submodel[submodel_num]; - - // Set the "blown out" flags. + + // Set the "blown off" flags if ( flags[Ship::Subsystem_Flags::No_disappear] ) { smi->blown_off = false; } else if ( copy_from ) { smi->blown_off = copy_from->blown_off; } + // In the future, we could expand the submodel instance to have a "blown_off_index" + // to indicate which form is currently visible, but for now, we'll follow the retail + // convention of having just two forms, the second of which is opposite from the first. + if ( smi->blown_off ) { - if ( sm->my_replacement >= 0 && !(flags[Ship::Subsystem_Flags::No_replace]) ) { - auto r_smi = &pmi->submodel[sm->my_replacement]; + if ( sm->next_form >= 0 && !(flags[Ship::Subsystem_Flags::No_replace]) ) { + auto r_smi = &pmi->submodel[sm->next_form]; r_smi->blown_off = false; if ( copy_from ) { r_smi->cur_angle = copy_from->cur_angle; @@ -5016,10 +5026,10 @@ void model_replicate_submodel_instance_sub(polymodel *pm, polymodel_instance *pm } } } else { - // If submodel isn't yet blown off and has a -destroyed replacement model, we prevent - // the replacement model from being drawn by marking it as having been blown off - if ( sm->my_replacement >= 0 && sm->my_replacement != submodel_num) { - auto r_smi = &pmi->submodel[sm->my_replacement]; + // If submodel isn't yet blown off and has a next form (like a -destroyed replacement model), + // we prevent the replacement model from being drawn by marking it as having been blown off + if ( sm->next_form >= 0 && sm->next_form != submodel_num) { + auto r_smi = &pmi->submodel[sm->next_form]; r_smi->blown_off = true; } } diff --git a/code/model/modelreplace.cpp b/code/model/modelreplace.cpp index f929b997337..d8c5c1e5fa3 100644 --- a/code/model/modelreplace.cpp +++ b/code/model/modelreplace.cpp @@ -267,11 +267,11 @@ CHANGE_HELPER(change_submodel_numbers, bsp_info, int) for (auto& detail : input.details) REPLACE_IF_EQ(detail); REPLACE_IF_EQ(input.first_child); - REPLACE_IF_EQ(input.i_replace); + REPLACE_IF_EQ(input.prev_form); for (auto& debris : input.live_debris) REPLACE_IF_EQ(debris); REPLACE_IF_EQ(input.look_at_submodel); - REPLACE_IF_EQ(input.my_replacement); + REPLACE_IF_EQ(input.next_form); REPLACE_IF_EQ(input.next_sibling); REPLACE_IF_EQ(input.parent); CHANGE_HELPER_END From bed223c5c7b1b2f0ae91c6e9885aa21488d8b4f9 Mon Sep 17 00:00:00 2001 From: Goober5000 Date: Wed, 13 May 2026 00:53:05 -0400 Subject: [PATCH 2/4] second part of comm node fix Fix #4089, part 2: - properly link special-point subsystems with -destroyed submodels, if such a relationship exists and a game_settings.tbl flag is set - add some helper functions for -destroyed variants of submodels (and now subsystems) --- code/mod_table/mod_table.cpp | 6 ++++++ code/mod_table/mod_table.h | 1 + code/model/model.h | 7 ++++++- code/model/modelread.cpp | 30 ++++++++++++++++++++++++------ code/parse/sexp.cpp | 3 +++ code/ship/shiphit.cpp | 28 ++++++++++++++++++++++++++++ code/ship/shiphit.h | 4 ++++ freespace2/freespace.cpp | 2 +- 8 files changed, 73 insertions(+), 8 deletions(-) diff --git a/code/mod_table/mod_table.cpp b/code/mod_table/mod_table.cpp index 2a7176fd829..dde809ec526 100644 --- a/code/mod_table/mod_table.cpp +++ b/code/mod_table/mod_table.cpp @@ -184,6 +184,7 @@ float Shield_percent_skips_damage; float Min_radius_for_persistent_debris; bool Zero_radius_explosions_skip_fireballs; bool Render_insignias_as_decals; +bool Link_subsystems_to_destroyed_submodels; #ifdef WITH_DISCORD @@ -1649,6 +1650,10 @@ void parse_mod_table(const char *filename) stuff_boolean(&Zero_radius_explosions_skip_fireballs); } + if (optional_string("$Link subsystems to -destroyed submodels:")) { + stuff_boolean(&Link_subsystems_to_destroyed_submodels); + } + // end of options ---------------------------------------- // if we've been through once already and are at the same place, force a move @@ -1897,6 +1902,7 @@ void mod_table_reset() Min_radius_for_persistent_debris = 50.0f; Zero_radius_explosions_skip_fireballs = false; Render_insignias_as_decals = false; + Link_subsystems_to_destroyed_submodels = false; } void mod_table_set_version_flags() diff --git a/code/mod_table/mod_table.h b/code/mod_table/mod_table.h index af0470548fd..433461d63d8 100644 --- a/code/mod_table/mod_table.h +++ b/code/mod_table/mod_table.h @@ -206,6 +206,7 @@ extern float Shield_percent_skips_damage; extern float Min_radius_for_persistent_debris; extern bool Zero_radius_explosions_skip_fireballs; extern bool Render_insignias_as_decals; +extern bool Link_subsystems_to_destroyed_submodels; void mod_table_init(); void mod_table_post_process(); diff --git a/code/model/model.h b/code/model/model.h index fee1de9bb3b..109db237da6 100644 --- a/code/model/model.h +++ b/code/model/model.h @@ -1137,7 +1137,6 @@ extern int model_find_2d_bound_min(int model_num,matrix *orient, vec3d * pos,int // rect. int submodel_find_2d_bound_min(int model_num,int submodel, matrix *orient, vec3d * pos,int *x1, int *y1, int *x2, int *y2); - // Returns zero is x1,y1,x2,y2 are valid // Returns 2 for point offscreen. // note that x1,y1,x2,y2 aren't clipped to 2d screen coordinates! @@ -1145,6 +1144,12 @@ int submodel_find_2d_bound_min(int model_num,int submodel, matrix *orient, vec3d // bounding box won't change depending on the obj's orient. int subobj_find_2d_bound(float radius, matrix *orient, vec3d * pos,int *x1, int *y1, int *x2, int *y2); +// Returns the index of the -destroyed version of a submodel name, if it exists +int submodel_find_destroyed_form(int model_num, const char *name_stem); + +// Returns whether this submodel name is a -destroyed version +bool submodel_is_destroyed_form(const char *name); + // stats variables #ifndef NDEBUG extern int modelstats_num_polys; diff --git a/code/model/modelread.cpp b/code/model/modelread.cpp index 176ab4307eb..c25752e9d69 100644 --- a/code/model/modelread.cpp +++ b/code/model/modelread.cpp @@ -2088,7 +2088,7 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename, sm->flags.set(Model::Submodel_flags::Nocollide_this_only); } - sm->flags.set(Model::Submodel_flags::Is_damaged, in(sm->name, "-destroyed")); + sm->flags.set(Model::Submodel_flags::Is_damaged, submodel_is_destroyed_form(sm->name)); break; } @@ -3339,11 +3339,7 @@ int model_load(const char* filename, ship_info* sip, ErrorType error_type, bool // Search for models that have destroyed versions for (i=0; in_models; i++ ) { - char destroyed_name[128]; - strcpy_s( destroyed_name, pm->submodel[i].name ); - strcat_s( destroyed_name, "-destroyed" ); - - int j = find_item_with_string(pm->submodel.get(), i2sz(pm->n_models), &bsp_info::name, destroyed_name); + int j = submodel_find_destroyed_form(pm->id, pm->submodel[i].name); if (j >= 0) { pm->submodel[i].next_form = j; pm->submodel[j].prev_form = i; @@ -3979,6 +3975,28 @@ int subobj_find_2d_bound(float radius ,matrix * /*orient*/, vec3d * pos,int *x1, return 0; } +int submodel_find_destroyed_form(int model_num, const char *name_stem) +{ + const auto pm = model_get(model_num); + Assertion(pm, "model_num must be valid!"); + + SCP_string destroyed_name(name_stem); + destroyed_name += "-destroyed"; + + return find_item_with_string(pm->submodel.get(), i2sz(pm->n_models), &bsp_info::name, destroyed_name.c_str()); +} + +bool submodel_is_destroyed_form(const char *name) +{ + constexpr auto suffix = "-destroyed"; + constexpr auto suffix_len = std::char_traits::length(suffix); + + auto len = strlen(name); + if (len <= suffix_len) + return false; + + return stricmp(name + len - suffix_len, suffix) == 0; +} // Given a rotating submodel, find the local and world axes of rotation. void model_get_rotating_submodel_axis(vec3d *model_axis, vec3d *world_axis, const polymodel *pm, const polymodel_instance *pmi, int submodel_num, const matrix *objorient) diff --git a/code/parse/sexp.cpp b/code/parse/sexp.cpp index 3c3cda2c890..7d97160754a 100644 --- a/code/parse/sexp.cpp +++ b/code/parse/sexp.cpp @@ -15755,6 +15755,9 @@ void set_subsys_strength_and_maybe_ancestors(ship *shipp, ship_subsys *ss, polym if (ss->submodel_instance_2) ss->submodel_instance_2->blown_off = false; + // special case for subsystems that don't correspond to a submodel + check_subsystem_submodel_link(shipp, ss, false); + // see if we are handling ancestors and if this subsystem has a submodel int subobj = ss->system_info->subobj_num; if (repair_ancestors && subobj >= 0) diff --git a/code/ship/shiphit.cpp b/code/ship/shiphit.cpp index 541a3b9c272..0c7d936f1ce 100755 --- a/code/ship/shiphit.cpp +++ b/code/ship/shiphit.cpp @@ -114,6 +114,31 @@ static bool is_subsys_destroyed(ship *shipp, int submodel) return false; } +void check_subsystem_submodel_link(const ship *shipp, const ship_subsys *subsys, bool was_destroyed) +{ + if (!Link_subsystems_to_destroyed_submodels) + return; + + Assertion(shipp && subsys, "the ship and subsystem must exist!"); + auto pmi = model_get_instance(shipp->model_instance_num); + Assertion(pmi, "the ship's model instance must exist!"); + + // check subsystem-submodel link, but only for special-point subsystems + // (not subsystems corresponding to a submodel) + auto psub = subsys->system_info; + if (psub->subobj_num >= 0) + return; + int j = submodel_find_destroyed_form(pmi->model_num, psub->subobj_name); + if (j < 0) + return; + + // show the submodel, or not, depending on what happened to the subsystem + if (was_destroyed) + pmi->submodel[j].blown_off = false; + else + pmi->submodel[j].blown_off = true; +} + // do_subobj_destroyed_stuff is called when a subobject for a ship is killed. Separated out // to separate function on 10/15/97 by MWA for easy multiplayer access. It does all of the // cool things like blowing off the model (if applicable, writing the logs, etc) @@ -346,6 +371,9 @@ void do_subobj_destroyed_stuff( ship *ship_p, ship_subsys *subsys, const vec3d* if ((psub->subobj_num != psub->turret_gun_sobj) && (psub->turret_gun_sobj >= 0)) { subsys->submodel_instance_2->blown_off = true; } + + // special case for subsystems that don't correspond to a submodel + check_subsystem_submodel_link(ship_p, subsys, true); } if (notify && !no_explosion) { diff --git a/code/ship/shiphit.h b/code/ship/shiphit.h index ba7b2f6292a..e2d6ee3542a 100644 --- a/code/ship/shiphit.h +++ b/code/ship/shiphit.h @@ -32,6 +32,10 @@ constexpr float DEATHROLL_ROTVEL_CAP = 6.3f; // maximum added deathroll rotve // of whoever is calling these functions. These functions are strictly // for damaging ship's hulls, shields, and subsystems. Nothing more. +// handle a -destroyed submodel in the special case where it is related to a special-point subsystem +// (the usual submodel-submodel case is handled through the normal code paths) +void check_subsystem_submodel_link(const ship *shipp, const ship_subsys *subsys, bool was_destroyed); + // function to destroy a subsystem. Called internally and from multiplayer messaging code extern void do_subobj_destroyed_stuff( ship *ship_p, ship_subsys *subsys, const vec3d *hitpos, bool no_explosion = false ); diff --git a/freespace2/freespace.cpp b/freespace2/freespace.cpp index 10de4806fae..b983df5bf5b 100644 --- a/freespace2/freespace.cpp +++ b/freespace2/freespace.cpp @@ -6732,7 +6732,7 @@ void game_spew_pof_info_sub(int model_num, polymodel *pm, int sm, CFILE *out, in // find the # of faces for this _individual_ object total = submodel_get_num_polys(model_num, sm); - if(strstr(pm->submodel[sm].name, "-destroyed")){ + if (submodel_is_destroyed_form(pm->submodel[sm].name)) { sub_total_destroyed = total; } From 8f916174bef3d2ce1d1bd7a884616a7b750dcac0 Mon Sep 17 00:00:00 2001 From: Goober5000 Date: Thu, 14 May 2026 03:30:53 -0400 Subject: [PATCH 3/4] clang --- code/model/modelread.cpp | 2 +- code/ship/shiphit.cpp | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/code/model/modelread.cpp b/code/model/modelread.cpp index c25752e9d69..95de185af92 100644 --- a/code/model/modelread.cpp +++ b/code/model/modelread.cpp @@ -3983,7 +3983,7 @@ int submodel_find_destroyed_form(int model_num, const char *name_stem) SCP_string destroyed_name(name_stem); destroyed_name += "-destroyed"; - return find_item_with_string(pm->submodel.get(), i2sz(pm->n_models), &bsp_info::name, destroyed_name.c_str()); + return find_item_with_string(pm->submodel.get(), i2sz(pm->n_models), &bsp_info::name, destroyed_name); } bool submodel_is_destroyed_form(const char *name) diff --git a/code/ship/shiphit.cpp b/code/ship/shiphit.cpp index 0c7d936f1ce..96fb5ddac76 100755 --- a/code/ship/shiphit.cpp +++ b/code/ship/shiphit.cpp @@ -133,10 +133,7 @@ void check_subsystem_submodel_link(const ship *shipp, const ship_subsys *subsys, return; // show the submodel, or not, depending on what happened to the subsystem - if (was_destroyed) - pmi->submodel[j].blown_off = false; - else - pmi->submodel[j].blown_off = true; + pmi->submodel[j].blown_off = !was_destroyed; } // do_subobj_destroyed_stuff is called when a subobject for a ship is killed. Separated out From c3b8922fe3fe36e656d7cde098bf73928edb7107 Mon Sep 17 00:00:00 2001 From: Goober5000 Date: Thu, 14 May 2026 23:35:00 -0400 Subject: [PATCH 4/4] address feedback --- code/mod_table/mod_table.cpp | 8 ++++---- code/mod_table/mod_table.h | 2 +- code/ship/shiphit.cpp | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/code/mod_table/mod_table.cpp b/code/mod_table/mod_table.cpp index dde809ec526..b5fd84f2c6d 100644 --- a/code/mod_table/mod_table.cpp +++ b/code/mod_table/mod_table.cpp @@ -184,7 +184,7 @@ float Shield_percent_skips_damage; float Min_radius_for_persistent_debris; bool Zero_radius_explosions_skip_fireballs; bool Render_insignias_as_decals; -bool Link_subsystems_to_destroyed_submodels; +bool Link_special_point_subsystems_to_destroyed_submodels; #ifdef WITH_DISCORD @@ -1650,8 +1650,8 @@ void parse_mod_table(const char *filename) stuff_boolean(&Zero_radius_explosions_skip_fireballs); } - if (optional_string("$Link subsystems to -destroyed submodels:")) { - stuff_boolean(&Link_subsystems_to_destroyed_submodels); + if (optional_string("$Link special-point subsystems to -destroyed submodels:")) { + stuff_boolean(&Link_special_point_subsystems_to_destroyed_submodels); } // end of options ---------------------------------------- @@ -1902,7 +1902,7 @@ void mod_table_reset() Min_radius_for_persistent_debris = 50.0f; Zero_radius_explosions_skip_fireballs = false; Render_insignias_as_decals = false; - Link_subsystems_to_destroyed_submodels = false; + Link_special_point_subsystems_to_destroyed_submodels = false; } void mod_table_set_version_flags() diff --git a/code/mod_table/mod_table.h b/code/mod_table/mod_table.h index 433461d63d8..022d6e34fc3 100644 --- a/code/mod_table/mod_table.h +++ b/code/mod_table/mod_table.h @@ -206,7 +206,7 @@ extern float Shield_percent_skips_damage; extern float Min_radius_for_persistent_debris; extern bool Zero_radius_explosions_skip_fireballs; extern bool Render_insignias_as_decals; -extern bool Link_subsystems_to_destroyed_submodels; +extern bool Link_special_point_subsystems_to_destroyed_submodels; void mod_table_init(); void mod_table_post_process(); diff --git a/code/ship/shiphit.cpp b/code/ship/shiphit.cpp index 96fb5ddac76..2721bd0ab00 100755 --- a/code/ship/shiphit.cpp +++ b/code/ship/shiphit.cpp @@ -116,7 +116,7 @@ static bool is_subsys_destroyed(ship *shipp, int submodel) void check_subsystem_submodel_link(const ship *shipp, const ship_subsys *subsys, bool was_destroyed) { - if (!Link_subsystems_to_destroyed_submodels) + if (!Link_special_point_subsystems_to_destroyed_submodels) return; Assertion(shipp && subsys, "the ship and subsystem must exist!");