diff --git a/code/asteroid/asteroid.cpp b/code/asteroid/asteroid.cpp index a1acbc6f6c..9c59ee6940 100644 --- a/code/asteroid/asteroid.cpp +++ b/code/asteroid/asteroid.cpp @@ -1439,9 +1439,9 @@ int asteroid_check_collision(object *pasteroid, object *other_obj, vec3d *hitpos // everything above was calculated relative to the heavy's position vec3d actual_world_hit_pos = mc.hit_point_world + heavy_obj->pos; if ((shipp->is_arriving()) && (shipp->warpin_effect != nullptr)) - warp_effect = shipp->warpin_effect; + warp_effect = shipp->warpin_effect.get(); else if ((shipp->flags[Ship::Ship_Flags::Depart_warp]) && (shipp->warpout_effect != nullptr)) - warp_effect = shipp->warpout_effect; + warp_effect = shipp->warpout_effect.get(); if (warp_effect != nullptr && point_is_clipped_by_warp(&actual_world_hit_pos, warp_effect)) mc_ret_val = 0; diff --git a/code/debris/debris.cpp b/code/debris/debris.cpp index 24b26e1cdf..7d45fa0ee7 100644 --- a/code/debris/debris.cpp +++ b/code/debris/debris.cpp @@ -1152,9 +1152,9 @@ int debris_check_collision(object *pdebris, object *other_obj, vec3d *hitpos, co // everything above was calculated relative to the heavy's position vec3d actual_world_hit_pos = mc.hit_point_world + heavy_obj->pos; if ((shipp->is_arriving()) && (shipp->warpin_effect != nullptr)) - warp_effect = shipp->warpin_effect; + warp_effect = shipp->warpin_effect.get(); else if ((shipp->flags[Ship::Ship_Flags::Depart_warp]) && (shipp->warpout_effect != nullptr)) - warp_effect = shipp->warpout_effect; + warp_effect = shipp->warpout_effect.get(); if (warp_effect != nullptr && point_is_clipped_by_warp(&actual_world_hit_pos, warp_effect)) mc_ret_val = 0; diff --git a/code/graphics/shadows.cpp b/code/graphics/shadows.cpp index 7a649b1c02..6a47b4fab3 100644 --- a/code/graphics/shadows.cpp +++ b/code/graphics/shadows.cpp @@ -537,8 +537,8 @@ static bool shadow_obj_clip_plane(const object* objp, shadow_render_list::clip_p if (dfi.maintained_variables.bool_value) { auto* dship = &Ships[dfi.maintained_variables.objp_value->instance]; - WarpEffect* warp_effect = dship->is_arriving(ship::warpstage::BOTH, true) - ? dship->warpin_effect : dship->warpout_effect; + auto* warp_effect = (dship->is_arriving(ship::warpstage::BOTH, true) + ? dship->warpin_effect : dship->warpout_effect).get(); model_render_params dummy; if (warp_effect->warpShipClip(&dummy)) { clip->normal = dummy.get_clip_plane_normal(); diff --git a/code/model/modelrender.cpp b/code/model/modelrender.cpp index 9e7b1e9432..549ad74681 100644 --- a/code/model/modelrender.cpp +++ b/code/model/modelrender.cpp @@ -1662,11 +1662,11 @@ void model_render_glowpoint_bitmap(int point_num, const vec3d *pos, const matrix if ((shipp->is_arriving()) && (shipp->warpin_effect != nullptr) && Warp_params[shipp->warpin_params_index].warp_type != WT_HYPERSPACE) { - warp_effect = shipp->warpin_effect; + warp_effect = shipp->warpin_effect.get(); } else if ((shipp->flags[Ship::Ship_Flags::Depart_warp]) && (shipp->warpout_effect != nullptr) && Warp_params[shipp->warpout_params_index].warp_type != WT_HYPERSPACE) { - warp_effect = shipp->warpout_effect; + warp_effect = shipp->warpout_effect.get(); } if (warp_effect != nullptr && point_is_clipped_by_warp(&world_pnt, warp_effect)) @@ -1821,10 +1821,10 @@ void model_render_glowpoint_add_light(int point_num, const vec3d *pos, const mat WarpEffect* warp_effect = nullptr; if ((shipp->is_arriving()) && Warp_params[shipp->warpin_params_index].warp_type != WT_HYPERSPACE) { - warp_effect = shipp->warpin_effect; + warp_effect = shipp->warpin_effect.get(); } else if ((shipp->flags[Ship::Ship_Flags::Depart_warp]) && Warp_params[shipp->warpout_params_index].warp_type != WT_HYPERSPACE) { - warp_effect = shipp->warpout_effect; + warp_effect = shipp->warpout_effect.get(); } if (warp_effect != nullptr && point_is_clipped_by_warp(&world_pnt, warp_effect)) @@ -2234,11 +2234,11 @@ void model_queue_render_thrusters(const model_render_params *interp, const polym if ((shipp->is_arriving()) && (shipp->warpin_effect != nullptr) && Warp_params[shipp->warpin_params_index].warp_type != WT_HYPERSPACE) { - warp_effect = shipp->warpin_effect; + warp_effect = shipp->warpin_effect.get(); } else if ((shipp->flags[Ship::Ship_Flags::Depart_warp]) && (shipp->warpout_effect != nullptr) && Warp_params[shipp->warpout_params_index].warp_type != WT_HYPERSPACE) { - warp_effect = shipp->warpout_effect; + warp_effect = shipp->warpout_effect.get(); } if (warp_effect != nullptr && point_is_clipped_by_warp(&world_pnt, warp_effect)) diff --git a/code/object/collideshipship.cpp b/code/object/collideshipship.cpp index 1fd1d69cfa..e62a057274 100644 --- a/code/object/collideshipship.cpp +++ b/code/object/collideshipship.cpp @@ -358,9 +358,9 @@ int ship_ship_check_collision(collision_info_struct *ship_ship_hit_info) vec3d actual_world_hit_pos = mc.hit_point_world + heavy_obj->pos; if (heavy_shipp->flags[Ship::Ship_Flags::Depart_warp] && heavy_shipp->warpout_effect != nullptr) - warp_effect = heavy_shipp->warpout_effect; + warp_effect = heavy_shipp->warpout_effect.get(); else if (heavy_shipp->flags[Ship::Ship_Flags::Arriving_stage_2] && heavy_shipp->warpin_effect != nullptr) - warp_effect = heavy_shipp->warpin_effect; + warp_effect = heavy_shipp->warpin_effect.get(); bool heavy_warp_no_collide = false; if (warp_effect != nullptr) @@ -368,9 +368,9 @@ int ship_ship_check_collision(collision_info_struct *ship_ship_hit_info) warp_effect = nullptr; if (light_shipp->flags[Ship::Ship_Flags::Depart_warp] && light_shipp->warpout_effect != nullptr) - warp_effect = light_shipp->warpout_effect; + warp_effect = light_shipp->warpout_effect.get(); else if (light_shipp->flags[Ship::Ship_Flags::Arriving_stage_2] && light_shipp->warpin_effect != nullptr) - warp_effect = light_shipp->warpin_effect; + warp_effect = light_shipp->warpin_effect.get(); bool light_warp_no_collide = false; if (warp_effect != nullptr) diff --git a/code/object/collideshipweapon.cpp b/code/object/collideshipweapon.cpp index 5d31296c84..3c85e3752d 100644 --- a/code/object/collideshipweapon.cpp +++ b/code/object/collideshipweapon.cpp @@ -435,9 +435,9 @@ static std::tuple ship_weapon_check_coll WarpEffect* warp_effect = nullptr; if (shipp->flags[Ship::Ship_Flags::Depart_warp] && shipp->warpout_effect != nullptr) - warp_effect = shipp->warpout_effect; + warp_effect = shipp->warpout_effect.get(); else if (shipp->flags[Ship::Ship_Flags::Arriving_stage_2] && shipp->warpin_effect != nullptr) - warp_effect = shipp->warpin_effect; + warp_effect = shipp->warpin_effect.get(); bool hull_no_collide, shield_no_collide; hull_no_collide = shield_no_collide = false; diff --git a/code/prop/prop.cpp b/code/prop/prop.cpp index 20eccc5e1b..a901e5b714 100644 --- a/code/prop/prop.cpp +++ b/code/prop/prop.cpp @@ -1094,9 +1094,9 @@ int prop_check_collision(object* prop_obj, object* other_obj, vec3d* hitpos, col // everything above was calculated relative to the heavy's position vec3d actual_world_hit_pos = mc.hit_point_world + heavy_obj->pos; if ((shipp->is_arriving()) && (shipp->warpin_effect != nullptr)) - warp_effect = shipp->warpin_effect; + warp_effect = shipp->warpin_effect.get(); else if ((shipp->flags[Ship::Ship_Flags::Depart_warp]) && (shipp->warpout_effect != nullptr)) - warp_effect = shipp->warpout_effect; + warp_effect = shipp->warpout_effect.get(); if (warp_effect != nullptr && point_is_clipped_by_warp(&actual_world_hit_pos, warp_effect)) mc_ret_val = 0; diff --git a/code/ship/ship.cpp b/code/ship/ship.cpp index f665b96049..e8d9b59576 100644 --- a/code/ship/ship.cpp +++ b/code/ship/ship.cpp @@ -6790,6 +6790,13 @@ vec3d get_submodel_offset(int model, int submodel){ } +// Defaulted here rather than in ship.h because the unique_ptr +// members need the complete WarpEffect type at the point of definition. +ship::ship() = default; +ship::~ship() = default; +ship::ship(ship &&) = default; // NOLINT(performance-noexcept-move-constructor) +ship &ship::operator=(ship &&) = default; // NOLINT(performance-noexcept-move-constructor) + // Reset all ship values to empty/unused. void ship::clear() { @@ -6824,12 +6831,8 @@ void ship::clear() really_final_death_time = timestamp(-1); deathroll_rotvel = vmd_zero_vector; - if (warpin_effect != nullptr) - delete warpin_effect; - if (warpout_effect != nullptr) - delete warpout_effect; - warpin_effect = nullptr; - warpout_effect = nullptr; + warpin_effect.reset(); + warpout_effect.reset(); warpin_params_index = -1; warpout_params_index = -1; @@ -16553,13 +16556,8 @@ void ship_close() for (i=0; iwarpin_effect != NULL) - delete shipp->warpin_effect; - shipp->warpin_effect = NULL; - - if(shipp->warpout_effect != NULL) - delete shipp->warpout_effect; - shipp->warpout_effect = NULL; + shipp->warpin_effect.reset(); + shipp->warpout_effect.reset(); } // free this too! -- Goober5000 diff --git a/code/ship/ship.h b/code/ship/ship.h index d2ee5e47fe..972dcb02f4 100644 --- a/code/ship/ship.h +++ b/code/ship/ship.h @@ -450,6 +450,64 @@ public: void clear(); }; +// The head sentinel of a ship's intrusive subsystem list. The sentinel's +// address is meaningful: the first node's prev and the last node's next point +// back at it, and an empty list is self-referential (after list_init) or +// null-linked (after construction or ship::clear). Moving a ship would +// otherwise leave the bookend nodes pointing at the old sentinel address, so +// the move operations re-link them. Copying is deleted: a copied sentinel +// would alias another list's nodes. +struct ship_subsys_sentinel : public ship_subsys +{ + ship_subsys_sentinel() = default; + + ship_subsys_sentinel(const ship_subsys_sentinel &) = delete; + ship_subsys_sentinel &operator=(const ship_subsys_sentinel &) = delete; + + ship_subsys_sentinel(ship_subsys_sentinel &&other) noexcept + { + take_links_from(other); + } + + ship_subsys_sentinel &operator=(ship_subsys_sentinel &&other) noexcept + { + if (this != &other) + { + Assertion(next == nullptr || next == this, "Move-assigning over a sentinel whose subsystem list is not empty! The destination's subsystems would be orphaned."); + take_links_from(other); + } + return *this; + } + +private: + void take_links_from(ship_subsys_sentinel &other) noexcept + { + if (other.next == nullptr) + { + // other was never list_init'd; match that state + next = nullptr; + prev = nullptr; + } + else if (other.next == &other) + { + // other is an initialized empty list + next = this; + prev = this; + } + else + { + // take ownership of the chain and re-point the bookends + next = other.next; + prev = other.prev; + next->prev = this; + prev->next = this; + } + // leave other as an initialized empty list, which is safe to iterate + other.next = &other; + other.prev = &other; + } +}; + // structure for subsystems which tells us the total count of a particular type of subsystem (i.e. // we might have 3 engines), and the relative strength of the subsystem. The #defines in model.h // for SUBSYSTEM_xxx will be used as indices into this array. @@ -592,8 +650,8 @@ public: int really_final_death_time; // Time until ship breaks up and disappears vec3d deathroll_rotvel; // Desired death rotational velocity - WarpEffect *warpin_effect; - WarpEffect *warpout_effect; + std::unique_ptr warpin_effect; + std::unique_ptr warpout_effect; int warpin_params_index; int warpout_params_index; @@ -655,7 +713,7 @@ public: // of a particular subsystem, like engines). The subsys_info struct is information for particular // types of subsystems. (i.e. the list might contain 3 engines. There will be one subsys_info entry // describing the state of all engines combined) -- MWA 4/1/97 - ship_subsys subsys_list; // linked list of subsystems for this ship. + ship_subsys_sentinel subsys_list; // linked list of subsystems for this ship. std::unique_ptr subsys_list_indexer; // provides random-access lookup to the linked list ship_subsys *last_targeted_subobject[MAX_PLAYERS]; // Last subobject that has been targeted. NULL if none;(player specific) ship_subsys_info subsys_info[SUBSYSTEM_MAX]; // info on particular generic types of subsystems @@ -851,6 +909,28 @@ public: BOTH, }; + // Ships support moving but not copying. The defaulted moves are correct + // because the members carry the smarts: the subsys_list sentinel re-links + // its bookend nodes, and the owning pointers are unique_ptrs. Callers must + // still fix up external back-references (Objects[].instance, + // Ai_info[].shipnum, etc.) -- see reassign_ship_slot in + // missioneditor/common.cpp. + // All the defaulted special member functions are defaulted in ship.cpp + // rather than here because they need the complete WarpEffect type. + // The moves are not declared noexcept because the implicit exception + // specification differs by standard library: MSVC and clang compute + // nothrow, but libstdc++ computes potentially-throwing for some members, + // making an explicit noexcept ill-formed there (a defaulted redeclaration + // may not strengthen the implicit specification). The NOLINTs suppress + // clang-tidy's performance-noexcept-move-constructor, which only sees + // clang's computation. + ship(); + ~ship(); + ship(const ship &) = delete; + ship &operator=(const ship &) = delete; + ship(ship &&); // NOLINT(performance-noexcept-move-constructor) + ship &operator=(ship &&); // NOLINT(performance-noexcept-move-constructor) + // reset to a completely blank ship void clear(); diff --git a/code/ship/shipfx.cpp b/code/ship/shipfx.cpp index c7379d7615..011fa38909 100644 --- a/code/ship/shipfx.cpp +++ b/code/ship/shipfx.cpp @@ -1264,9 +1264,9 @@ void shipfx_emit_spark( int n, int sn ) WarpEffect* warp_effect = nullptr; if ((shipp->is_arriving()) && (shipp->warpin_effect)) - warp_effect = shipp->warpin_effect; + warp_effect = shipp->warpin_effect.get(); else if ((shipp->flags[Ship::Ship_Flags::Depart_warp]) && (shipp->warpout_effect)) - warp_effect = shipp->warpout_effect; + warp_effect = shipp->warpout_effect.get(); if (warp_effect != nullptr && point_is_clipped_by_warp(&outpnt, warp_effect)) return; @@ -2950,50 +2950,44 @@ void ship_set_warp_effects(object *objp) if (warpout_type & WT_DEFAULT_WITH_FIREBALL) warpout_type = WT_DEFAULT; - if (shipp->warpin_effect != nullptr) - delete shipp->warpin_effect; - switch (warpin_type) { case WT_DEFAULT: case WT_KNOSSOS: case WT_DEFAULT_THEN_KNOSSOS: - shipp->warpin_effect = new WE_Default(objnum, WarpDirection::WARP_IN); + shipp->warpin_effect = std::make_unique(objnum, WarpDirection::WARP_IN); break; case WT_IN_PLACE_ANIM: - shipp->warpin_effect = new WE_BSG(objnum, WarpDirection::WARP_IN); + shipp->warpin_effect = std::make_unique(objnum, WarpDirection::WARP_IN); break; case WT_SWEEPER: - shipp->warpin_effect = new WE_Homeworld(objnum, WarpDirection::WARP_IN); + shipp->warpin_effect = std::make_unique(objnum, WarpDirection::WARP_IN); break; case WT_HYPERSPACE: - shipp->warpin_effect = new WE_Hyperspace(objnum, WarpDirection::WARP_IN); + shipp->warpin_effect = std::make_unique(objnum, WarpDirection::WARP_IN); break; default: - shipp->warpin_effect = new WarpEffect(); + shipp->warpin_effect = std::make_unique(); } - if (shipp->warpout_effect != nullptr) - delete shipp->warpout_effect; - switch (warpout_type) { case WT_DEFAULT: case WT_KNOSSOS: case WT_DEFAULT_THEN_KNOSSOS: - shipp->warpout_effect = new WE_Default(objnum, WarpDirection::WARP_OUT); + shipp->warpout_effect = std::make_unique(objnum, WarpDirection::WARP_OUT); break; case WT_IN_PLACE_ANIM: - shipp->warpout_effect = new WE_BSG(objnum, WarpDirection::WARP_OUT); + shipp->warpout_effect = std::make_unique(objnum, WarpDirection::WARP_OUT); break; case WT_SWEEPER: - shipp->warpout_effect = new WE_Homeworld(objnum, WarpDirection::WARP_OUT); + shipp->warpout_effect = std::make_unique(objnum, WarpDirection::WARP_OUT); break; case WT_HYPERSPACE: - shipp->warpout_effect = new WE_Hyperspace(objnum, WarpDirection::WARP_OUT); + shipp->warpout_effect = std::make_unique(objnum, WarpDirection::WARP_OUT); break; default: - shipp->warpout_effect = new WarpEffect(); + shipp->warpout_effect = std::make_unique(); } } diff --git a/code/weapon/beam.cpp b/code/weapon/beam.cpp index 3c6d77d792..0466130d36 100644 --- a/code/weapon/beam.cpp +++ b/code/weapon/beam.cpp @@ -3216,9 +3216,9 @@ int beam_collide_ship(obj_pair *pair) WarpEffect* warp_effect = nullptr; if (shipp->flags[Ship::Ship_Flags::Depart_warp] && shipp->warpout_effect != nullptr) - warp_effect = shipp->warpout_effect; + warp_effect = shipp->warpout_effect.get(); else if (shipp->flags[Ship::Ship_Flags::Arriving_stage_2] && shipp->warpin_effect != nullptr) - warp_effect = shipp->warpin_effect; + warp_effect = shipp->warpin_effect.get(); bool hull_no_collide, shield_no_collide;