Update 25.0.1 with four PRs (#7392)

* Fix stencil attachment for RTT textures and enable depth/stencil for cockpit RTT (#7386)

* Fix stale subsystem model_num references across mission boundaries (#3147)

Fixes the long-standing intermittent bug where the GTD Orion#Repulse in
Silent Threat: Reborn's "Forced Hand" mission spawns with all subsystems
unlinked, leaving the ship untargetable and undisableable.  The same root
cause has been reported since at least 2015 (HLP topic 89403) and was
provisionally closed in #3147 as fixed by #3858, but #3858 was an unrelated
parse_ship_values memory leak fix and never actually addressed this
behavior.  Recently the underlying check at ship.cpp ~8043 was promoted
from Warning to Error (#5591), which is why Release players are now
hitting a hard error dialog instead of just seeing missing subsystems.

Root cause
----------
model_unload() walks Ship_info and resets si.model_num = -1 on every
ship_info that referenced the model, but does not touch
si.subsystems[k].model_num.  Subsystem entries are left pointing at the
freed model id.  When a subsequent mission reloads the same pof under a
new id, ship variants that share the model rely on
ship_copy_subsystem_fixup() to re-link their subsystem arrays, but the
fixup matches by name via model_copy_subsystems() and any unmatched
destination subsystem stays at -1.  The mismatch case in
model_copy_subsystems() was guarded only by Int3(), which is a no-op in
Release builds, so the failure was completely silent until subsys_set()
errored out much later with no useful context.

The bug is intermittent because it depends on three independent ordering
factors: which prior mission was last played, which Orion variant that
mission used, and the iteration order of Ship_info during the next
mission's ship_page_in().

Changes
-------
* model_unload(): when clearing si.model_num for an unloaded model, also
  clear every si.subsystems[k].model_num that pointed at the same id.
  This is the structural fix and eliminates the entire class of stale
  subsystem pointers surviving across mission boundaries.

* model_copy_subsystems(): replace the silent Int3() in the name-mismatch
  branch with Error(LOCATION, ...) naming the unmatched subsystem.  This
  surfaces a failure mode that has been invisible in Release builds for
  over a decade and gives modders an actionable diagnostic when sibling
  ship classes that share a model have inconsistent subsystem names.

* ship.cpp: introduce verify_ship_subsystems_linked() helper that checks
  every model_subsystem on a ship_info is linked into the same model the
  ship is using, and fails with Error(LOCATION, ...) naming both
  polymodels if not.  This is the canonical post-condition for both
  ship_copy_subsystem_fixup() and model_load() (which is supposed to
  link subsystems via do_new_subsystem()).

* ship_create(): call verify_ship_subsystems_linked() after
  ship_copy_subsystem_fixup() so that any failure to link a subsystem
  is surfaced with full context (ship name, subsystem name, both model
  filenames) instead of propagating to a generic error in subsys_set()
  much later.  This closes a long-standing asymmetry where ship_page_in()
  had a guard (added by zookeeper in 2015) but ship_create() did not.

* ship_page_in(): three post-fixup verifications were gated by
  #ifndef NDEBUG with Warning() or Assertion() calls, both of which are
  no-ops in Release builds.  Replace each with a verify_ship_subsystems_linked()
  call so they fire in Release as well.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix two timestamp bugs

A timestamp set to 1 means an immediate timestamp, and so the `int` version of `timestamp_elapsed` should account for that, just as it does in the `TIMESTAMP` and `UI_TIMESTAMP` versions.  Normally the number comparison means the check will still work, but during mission initialization the timestamp code is not running.  This caused destroy-before-mission ships to be added to the arrival list when they shouldn't have been.

Also fix logic in `timestamp_elapsed_safe`, where the `TIMESTAMP` and `UI_TIMESTAMP` versions were correct, but the `int` version had a bug dating back to retail that caused it to return true for uninitialized (zero) timestamps rather than false.  Practically speaking, this fix has no effect because the function is only used in multiplayer and none of the instances use integer-0 to indicate uninitialized, but this will at least fix any uses in the future.

* Fix dirty modelcollide state in shipfx and harden with assert (#7390)

---------

Co-authored-by: Goober5000 <[email protected]>
Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>

code/ai/aicode.cpp

@@ -13705,7 +13705,7 @@ int ai_acquire_emerge_path(object *pl_objp, int parent_objnum, int allowed_path_
 	ship_info* parent_sip = &Ship_info[parent_shipp->ship_info_index];
 
 	polymodel *pm = model_get( parent_sip->model_num );
-	ship_bay *bay = pm->ship_bay;
+	const auto& bay = pm->ship_bay;
 
 	if ( bay == nullptr ) {
 		Warning(LOCATION, "Ship %s was set to arrive from fighter bay on object %s, but no fighter bay exists on that ships' model (%s).\n", shipp->ship_name, parent_shipp->ship_name, pm->filename);
@@ -13920,9 +13920,8 @@ int ai_find_closest_depart_path(ai_info *aip, polymodel *pm, int allowed_path_ma
 	float		dist, min_dist, min_free_dist;
 	vec3d		*source;
 	model_path	*mp;
-	ship_bay	*bay;
 
-	bay = pm->ship_bay;
+	const auto& bay = pm->ship_bay;
 
 	best_free_path = best_path = -1;
 	min_free_dist = min_dist = 1e20f;
@@ -13999,9 +13998,9 @@ int ai_acquire_depart_path(object *pl_objp, int parent_objnum, int allowed_path_
 
 	object *parent_objp = &Objects[parent_objnum];
 	polymodel *pm = model_get(Ship_info[Ships[parent_objp->instance].ship_info_index].model_num );
-	ship_bay *bay = pm->ship_bay;
+	const auto& bay = pm->ship_bay;
 
-	if ( bay == NULL ) 
+	if ( bay == nullptr )
 		return -1;
 	if ( bay->num_paths <= 0 ) 
 		return -1;
@@ -14092,11 +14091,10 @@ void ai_bay_depart()
 
 		// Volition bay code
 		polymodel	*pm;
-		ship_bay	*bay;
 
 		pm = model_get(Ship_info[Ships[Objects[aip->goal_objnum].instance].ship_info_index].model_num);
-		bay = pm->ship_bay;
-		if ( bay != NULL ) {
+		const auto& bay = pm->ship_bay;
+		if ( bay != nullptr ) {
 			bay->depart_flags &= ~(1<<aip->submode_parm0);
 		}
 

code/ai/aiturret.cpp

@@ -2042,7 +2042,7 @@ bool turret_fire_weapon(int weapon_num,
 						}
 						//spawn particle effect
 						auto particleSource = particle::ParticleManager::get()->createSource(wip->muzzle_effect);
-						particleSource->setHost(make_unique<EffectHostTurret>(&Objects[parent_ship->objnum], turret->system_info->turret_gun_sobj, turret->turret_next_fire_pos));
+						particleSource->setHost(make_unique<EffectHostTurret>(&Objects[parent_ship->objnum], turret->system_info->turret_gun_sobj, turret->turret_next_fire_pos, false));
 						particleSource->setTriggerRadius(objp->radius * radius_mult);
 						particleSource->setTriggerVelocity(vm_vec_mag_quick(&objp->phys_info.vel));
 						particleSource->finishCreation();
@@ -2163,7 +2163,7 @@ void turret_swarm_fire_from_turret(turret_swarm_info *tsi)
 		if (Weapon_info[tsi->weapon_class].muzzle_effect.isValid()) {
 			//spawn particle effect
 			auto particleSource = particle::ParticleManager::get()->createSource(Weapon_info[tsi->weapon_class].muzzle_effect);
-			particleSource->setHost(make_unique<EffectHostTurret>(&Objects[tsi->parent_objnum], tsi->turret->system_info->turret_gun_sobj, tsi->turret->turret_next_fire_pos - 1));
+			particleSource->setHost(make_unique<EffectHostTurret>(&Objects[tsi->parent_objnum], tsi->turret->system_info->turret_gun_sobj, tsi->turret->turret_next_fire_pos - 1, false));
 			particleSource->setTriggerRadius(Objects[weapon_objnum].radius);
 			particleSource->finishCreation();
 		}

code/asteroid/asteroid.cpp

@@ -1315,9 +1315,9 @@ int asteroid_check_collision(object *pasteroid, object *other_obj, vec3d *hitpos
 				model_get_moving_submodel_list(submodel_vector, heavy_obj);
 
 				// turn off all moving submodels, collide against only 1 at a time.
-				// turn off collision detection for all moving submodels
+				mc.collision_checked.assign(pm->n_models, 0);
 				for (auto submodel: submodel_vector) {
-					pmi->submodel[submodel].collision_checked = true;
+					mc.collision_checked[submodel] = true;
 				}
 
 				// Only check single submodel now, since children of moving submodels are handled as moving as well
@@ -1325,10 +1325,8 @@ int asteroid_check_collision(object *pasteroid, object *other_obj, vec3d *hitpos
 
 				// check each submodel in turn
 				for (auto submodel: submodel_vector) {
-					auto smi = &pmi->submodel[submodel];
-
 					// turn on just one submodel for collision test
-					smi->collision_checked = false;
+					mc.collision_checked[submodel] = false;
 
 					// find the start and end positions of the sphere in submodel RF
 					model_instance_global_to_local_point(&p0, &light_obj->last_pos, pm, pmi, submodel, &heavy_obj->last_orient, &heavy_obj->last_pos, true);
@@ -1360,9 +1358,12 @@ int asteroid_check_collision(object *pasteroid, object *other_obj, vec3d *hitpos
 						}
 					}
 					// Don't look at this submodel again
-					smi->collision_checked = true;
+					mc.collision_checked[submodel] = true;
 				}
 
+				// Clear collision_checked before base model pass so it auto-inits fresh
+				mc.collision_checked.clear();
+
 			}
 
 			// Now complete base model collision checks that do not take into account rotating submodels.

code/bmpman/bmpman.h

@@ -135,6 +135,7 @@ void bm_close();
 #define BMP_FLAG_RENDER_TARGET_DYNAMIC		(1<<1)
 #define BMP_FLAG_CUBEMAP					(1<<2)
 #define BMP_FLAG_RENDER_TARGET_MIPMAP		(1<<3)
+#define BMP_FLAG_RENDER_TARGET_DEPTH_ATTACHMENT	(1<<4)
 
 /**
  * @brief Allocates memory for the given handle.

code/debris/debris.cpp

@@ -1030,9 +1030,9 @@ int debris_check_collision(object *pdebris, object *other_obj, vec3d *hitpos, co
 				model_get_moving_submodel_list(submodel_vector, heavy_obj);
 
 				// turn off all moving submodels, collide against only 1 at a time.
-				// turn off collision detection for all moving submodels
+				mc.collision_checked.assign(pm->n_models, 0);
 				for (auto submodel : submodel_vector) {
-					pmi->submodel[submodel].collision_checked = true;
+					mc.collision_checked[submodel] = true;
 				}
 
 				// Only check single submodel now, since children of moving submodels are handled as moving as well
@@ -1044,10 +1044,8 @@ int debris_check_collision(object *pdebris, object *other_obj, vec3d *hitpos, co
 
 				// check each submodel in turn
 				for (auto submodel: submodel_vector) {
-					auto smi = &pmi->submodel[submodel];
-
 					// turn on just one submodel for collision test
-					smi->collision_checked = false;
+					mc.collision_checked[submodel] = false;
 
 					// find the start and end positions of the sphere in submodel RF
 					model_instance_global_to_local_point(&p0, &light_obj->last_pos, pm, pmi, submodel, &heavy_obj->last_orient, &heavy_obj->last_pos, true);
@@ -1081,8 +1079,11 @@ int debris_check_collision(object *pdebris, object *other_obj, vec3d *hitpos, co
 					}
 
 					// Don't look at this submodel again
-					smi->collision_checked = true;
+					mc.collision_checked[submodel] = true;
 				}
+
+				// Clear collision_checked before base model pass so it auto-inits fresh
+				mc.collision_checked.clear();
 			}
 
 			// Now complete base model collision checks that do not take into account rotating submodels.

code/decals/decals.cpp

@@ -185,6 +185,10 @@ Decal::Decal() {
 	vm_vec_make(&scale, 1.f, 1.f, 1.f);
 }
 
+void Decal::markForDeletion() {
+	orig_obj_type = OBJ_NONE;
+}
+
 bool Decal::isValid() const  {
 	if (!object.isValid()) {
 		return false;
@@ -193,6 +197,11 @@ bool Decal::isValid() const  {
 		return false;
 	}
 
+	if (orig_obj_type == OBJ_NONE) {
+		//Decal should be cleared
+		return false;
+	}
+
 	if (orig_obj_type != object.objp()->type) {
 		mprintf(("Decal object type for object %d has changed from %s to %s. Please let m!m know about this\n",
 				object.objnum, Object_type_names[orig_obj_type], Object_type_names[object.objp()->type]));
@@ -536,4 +545,13 @@ void addSingleFrameDecal(Decal&& info) {
 	active_single_frame_decals.push_back(info);
 }
 
+void invalidateForShip(const ship* shipp) {
+	int objnum = shipp->objnum;
+	for (Decal& decal : active_decals) {
+		if (decal.object.objnum == objnum) {
+			decal.markForDeletion();
+		}
+	}
+}
+
 }

code/decals/decals.h

@@ -66,6 +66,8 @@ struct Decal {
 
 	Decal();
 
+	void markForDeletion();
+
 	bool isValid() const;
 };
 
@@ -160,4 +162,6 @@ void addDecal(creation_info& info,
 
 void addSingleFrameDecal(Decal&& info);
 
+void invalidateForShip(const ship* shipp);
+
 }

code/globalincs/type_traits.h

@@ -46,6 +46,30 @@ struct is_dereferenceable_pointer<T, typename std::enable_if_t<std::is_pointer_v
 template<typename T>
 inline constexpr bool is_dereferenceable_pointer_v = is_dereferenceable_pointer<T>::value;
 
+template<typename T, typename Enable = void>
+struct is_smart_pointer : std::false_type {};
+
+template<typename T>
+struct is_smart_pointer<T, std::enable_if_t<std::is_same_v<std::remove_cv_t<T>, std::shared_ptr<typename T::element_type>>>> : std::true_type {};
+
+template<typename T>
+struct is_smart_pointer<T, std::enable_if_t<std::is_same_v<std::remove_cv_t<T>, std::shared_ptr<typename T::element_type[]>>>> : std::true_type {};
+
+template<typename T>
+struct is_smart_pointer<T, std::enable_if_t<std::is_same_v<std::remove_cv_t<T>, std::unique_ptr<typename T::element_type>>>> : std::true_type {};
+
+template<typename T>
+struct is_smart_pointer<T, std::enable_if_t<std::is_same_v<std::remove_cv_t<T>, std::unique_ptr<typename T::element_type[]>>>> : std::true_type {};
+
+template<typename T>
+struct is_smart_pointer<T, std::enable_if_t<std::is_same_v<std::remove_cv_t<T>, std::weak_ptr<typename T::element_type>>>> : std::true_type {};
+
+template<typename T>
+struct is_smart_pointer<T, std::enable_if_t<std::is_same_v<std::remove_cv_t<T>, std::weak_ptr<typename T::element_type[]>>>> : std::true_type {};
+
+template<typename T>
+inline constexpr bool is_smart_pointer_v = is_smart_pointer<T>::value;
+
 template<class T, template<class...> class U>
 inline constexpr bool is_instance_of_v = std::false_type{};
 

code/graphics/2d.h

@@ -615,9 +615,8 @@ public:
 };
 
 struct indexed_vertex_source {
-	void* Vertex_list = nullptr;
-	void* Index_list = nullptr;
-
+	std::shared_ptr<uint8_t[]> Vertex_list = nullptr;
+	std::shared_ptr<uint8_t[]> Index_list = nullptr;
 	gr_buffer_handle Vbuffer_handle;
 	size_t Vertex_offset = 0;
 	size_t Base_vertex_offset = 0;

code/graphics/opengl/gropengl.cpp

@@ -286,12 +286,19 @@ void gr_opengl_print_screen(const char *filename)
 	//Reading from the front buffer here seems to no longer work correctly; that just reads back all zeros
 	glReadBuffer(Cmdline_window_res ? GL_COLOR_ATTACHMENT0 : GL_FRONT);
 
+	// Clamp float values to [0,1] when reading from the GL_RGBA16F back framebuffer.
+	// The default GL_FIXED_ONLY only clamps fixed-point FBOs, leaving float FBO reads
+	// with out-of-range values (HDR > 1.0) producing undefined behavior when converted
+	// to integer types, which manifests as rainbow artifacts on bright areas.
+	glClampColor(GL_CLAMP_READ_COLOR, GL_TRUE);
+
 	// now for the data
 	if (Use_PBOs) {
 		Assert( !pbo );
 		glGenBuffers(1, &pbo);
 
 		if ( !pbo ) {
+			glClampColor(GL_CLAMP_READ_COLOR, GL_FIXED_ONLY);
 			return;
 		}
 
@@ -306,6 +313,7 @@ void gr_opengl_print_screen(const char *filename)
 		pixels = (GLubyte*) vm_malloc(gr_screen.max_w * gr_screen.max_h * 4, memory::quiet_alloc);
 
 		if (pixels == NULL) {
+			glClampColor(GL_CLAMP_READ_COLOR, GL_FIXED_ONLY);
 			return;
 		}
 
@@ -313,10 +321,20 @@ void gr_opengl_print_screen(const char *filename)
 		glFlush();
 	}
 
+	glClampColor(GL_CLAMP_READ_COLOR, GL_FIXED_ONLY);
+
+	// Force alpha to fully opaque so screenshots are not saved with transparency.
+	// The framebuffer alpha can be < 255 due to blending operations affecting the alpha
+	// channel as a side effect, even though the rendered image itself is opaque.
+	int num_pixels = gr_screen.max_w * gr_screen.max_h;
+	for (int i = 0; i < num_pixels; i++) {
+		pixels[i * 4 + 3] = 255;
+	}
+
 	if (!png_write_bitmap(os_get_config_path(tmp).c_str(), gr_screen.max_w, gr_screen.max_h, true, pixels)) {
 		ReleaseWarning(LOCATION, "Failed to write screenshot to \"%s\".", os_get_config_path(tmp).c_str());
 	}
-	
+
 	if (pbo) {
 		glUnmapBuffer(GL_PIXEL_PACK_BUFFER);
 		pixels = NULL;

code/graphics/opengl/gropengldraw.cpp

@@ -52,6 +52,7 @@ GLuint Scene_stencil_buffer;
 GLuint Back_framebuffer;
 GLuint Back_texture;
 GLuint Back_depth_texture;
+GLuint Back_stencil_buffer;
 
 GLuint Distortion_framebuffer = 0;
 GLuint Distortion_texture[2];
@@ -551,6 +552,11 @@ void opengl_setup_scene_textures()
 		glFramebufferTexture2D(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_TEXTURE_2D, Back_depth_texture, 0);
 		gr_zbuffer_set(GR_ZBUFF_FULL);
 		glClear(GL_DEPTH_BUFFER_BIT);
+
+		glGenRenderbuffers(1, &Back_stencil_buffer);
+		glBindRenderbuffer(GL_RENDERBUFFER, Back_stencil_buffer);
+		glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8, gr_screen.max_w, gr_screen.max_h);
+		glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_RENDERBUFFER, Back_stencil_buffer);
 	}
 
 	//Setup thruster distortion framebuffer

code/graphics/opengl/gropengltexture.cpp

@@ -1866,11 +1866,13 @@ int opengl_make_render_target( int handle, int *w, int *h, int *bpp, int *mm_lvl
 
 	GL_state.Texture.Enable(0);
 
-	// render buffer
-//	glGenRenderbuffers(1, &new_fbo.renderbuffer_id);
-//	glBindRenderbuffer(GL_RENDERBUFFER, new_fbo.renderbuffer_id);
-//	glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH_COMPONENT24, *w, *h);
-	glBindRenderbuffer(GL_RENDERBUFFER, 0);
+	// render buffer (depth)
+	if (flags & BMP_FLAG_RENDER_TARGET_DEPTH_ATTACHMENT) {
+		glGenRenderbuffers(1, &new_fbo->renderbuffer_id);
+		glBindRenderbuffer(GL_RENDERBUFFER, new_fbo->renderbuffer_id);
+		glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8, *w, *h);
+		glBindRenderbuffer(GL_RENDERBUFFER, 0);
+	}
 
 	// frame buffer
 	glGenFramebuffers(1, &new_fbo->framebuffer_id);
@@ -1883,7 +1885,9 @@ int opengl_make_render_target( int handle, int *w, int *h, int *bpp, int *mm_lvl
 		glFramebufferTextureLayer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, ts->texture_id, 0, 0);
 	}
 
-//	glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, new_fbo.renderbuffer_id);
+	if (flags & BMP_FLAG_RENDER_TARGET_DEPTH_ATTACHMENT) {
+		glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_RENDERBUFFER, new_fbo->renderbuffer_id);
+	}
 
 	if ( opengl_check_framebuffer() ) {
 		// Oops!!  reset everything and then bail
@@ -1898,7 +1902,10 @@ int opengl_make_render_target( int handle, int *w, int *h, int *bpp, int *mm_lvl
 
 		glDeleteFramebuffers(1, &new_fbo->framebuffer_id);
 
-	//	glDeleteRenderbuffersEXT(1, &new_fbo.renderbuffer_id);
+		if (new_fbo->renderbuffer_id) {
+			glDeleteRenderbuffers(1, &new_fbo->renderbuffer_id);
+			new_fbo->renderbuffer_id = 0;
+		}
 
 		opengl_set_texture_target();
 		opengl_free_fbo_slot(fbo_id);

code/graphics/opengl/gropengltnl.cpp

@@ -822,7 +822,10 @@ void opengl_tnl_set_model_material(model_material *material_info)
 	opengl_tnl_set_material(material_info, false, false);
 
 	if ( GL_state.CullFace() ) {
-		GL_state.FrontFaceValue(GL_CW);
+		// Model rendering uses CW front faces in the default backbuffer path.
+		// Render-to-texture flips the projection vertically, which flips winding,
+		// so we must invert the front-face definition there.
+		GL_state.FrontFaceValue(gr_screen.rendering_to_texture != -1 ? GL_CCW : GL_CW);
 	}
 
 	gr_set_center_alpha(material_info->get_center_alpha());

code/hud/hudreticle.cpp

@@ -1037,7 +1037,7 @@ void HudGaugeThreatIndicator::renderLaserThreat(bool config)
 	}
 
 	int frame_offset;
-	if ( Player->threat_flags & THREAT_DUMBFIRE ) {
+	if (!config && Player->threat_flags & THREAT_DUMBFIRE) {
 		if ( timestamp_elapsed(laser_warn_timer) ) {
 			laser_warn_timer = timestamp(THREAT_DUMBFIRE_FLASH);
 			laser_warn_frame++;
@@ -1072,7 +1072,7 @@ void HudGaugeThreatIndicator::renderLockThreat(bool config)
 	}
 
 	int frame_offset;
-	if ( Player->threat_flags & (THREAT_LOCK | THREAT_ATTEMPT_LOCK) ) {
+	if (!config && Player->threat_flags & (THREAT_LOCK | THREAT_ATTEMPT_LOCK)) {
 		if ( timestamp_elapsed(lock_warn_timer) ) {
 			if ( Player->threat_flags & THREAT_LOCK )  {
 				lock_warn_timer = timestamp(fl2i(THREAT_LOCK_FLASH/2.0f));

code/hud/hudshield.cpp

@@ -700,12 +700,10 @@ void HudGaugeShield::showShields(const object *objp, ShieldGaugeType mode, bool
 			int mi = -1;
 			if (!config) {
 				mi = sp->model_instance_num;
-			}else{
-				mi = model_load(sip, false);
+				render_info.set_replacement_textures(model_get_instance(mi)->texture_replace);
 			}
 
 			render_info.set_flags(MR_NO_LIGHTING | MR_AUTOCENTER | MR_NO_FOGGING);
-			render_info.set_replacement_textures(model_get_instance(mi)->texture_replace);
 			render_info.set_detail_level_lock(1);
 			render_info.set_object_number(OBJ_INDEX(objp));
 

code/io/joy-sdl.cpp

@@ -558,7 +558,7 @@ bool device_event_handler(const SDL_Event &evt)
 		// Else, Find if any of our joysticks were lost
 		for (short i = CID_JOY0; i < CID_JOY_MAX; ++i)
 		{
-			if (joyDeviceEvent.which == pJoystick[i]->getID())
+			if (pJoystick[i] != nullptr && joyDeviceEvent.which == pJoystick[i]->getID())
 			{
 				// We just lost our joystick, reset the data
 				mprintf(("Joystick %i disconnected\n", i));

code/io/timer.cpp

@@ -489,6 +489,9 @@ bool timestamp_elapsed(int stamp) {
 	if (stamp == 0) {
 		return false;
 	}
+	if (stamp == 1) {
+		return true;
+	}
 
 	return timestamp_ms() >= stamp;
 }
@@ -539,6 +542,9 @@ bool ui_timestamp_elapsed_last_frame(UI_TIMESTAMP ui_stamp) {
 
 bool timestamp_elapsed_safe(int a, int b) {
 	if (a == 0) {
+		return false;
+	}
+	if (a == 1) {
 		return true;
 	}
 

code/lab/dialogs/lab_ui.cpp

@@ -478,6 +478,10 @@ void LabUi::show_render_options()
 					for (const auto &s : profile_list) {
 						if (Button(s.c_str(), ImVec2(-FLT_MIN, GetTextLineHeight() * 2))) {
 							ltp::switch_to(s);
+
+							// Avoid immediately overwriting the selected profile's values with
+							// stale slider state captured before the profile switch.
+							skip_setting_light_options_this_frame = true;
 						}
 					}
 				}
@@ -1064,6 +1068,9 @@ void LabUi::maybe_show_animation_category(const SCP_vector<animation::ModelAnima
 					case animation::ModelAnimationTriggerType::Docked:
 						button_label += "Trigger Docked Animation " + std::to_string(count++);
 						break;
+					case animation::ModelAnimationTriggerType::Scripted:
+						button_label += "Trigger Scripted Animation " + std::to_string(count++);
+						break;
 					default:
 						// We really shouldn't be here, but just in case
 						Assertion(false, "Unexpected animation trigger type %d", static_cast<int>(trigger_type));

code/lab/renderer/lab_renderer.cpp

@@ -411,6 +411,10 @@ void LabRenderer::useBackground(const SCP_string& mission_name) {
 				ltp::switch_to(ltp_name);
 		}
 
+		// Mission headers include additional fields between lighting profile and the
+		// background section. If we stopped at the lighting profile, we need to seek again.
+		skip_to_start_of_string("#Background bitmaps");
+
 		if (optional_string("#Background bitmaps")) {
 			required_string("$Num stars:");
 			stuff_int(&Num_stars);

code/mission/missioncampaign.cpp

@@ -928,15 +928,11 @@ void mission_campaign_eval_next_mission()
  */
 void mission_campaign_store_goals_and_events()
 {
-	int cur;
-	cmission *mission_obj;
-
 	if (!(Game_mode & GM_CAMPAIGN_MODE) || (Campaign.current_mission < 0))
 		return;
 
-	cur = Campaign.current_mission;
-
-	mission_obj = &Campaign.missions[cur];
+	int cur = Campaign.current_mission;
+	auto mission_obj = &Campaign.missions[cur];
 
 	// first we must save the status of the current missions goals in the campaign mission structure.
 	// After that, we can determine which mission is tagged as the next mission.  Finally, we
@@ -992,66 +988,54 @@ void mission_campaign_store_goals_and_events()
 
 void mission_campaign_store_variables(int persistence_type, bool store_red_alert)
 {
-	int cur, i, j;
-	cmission *mission_obj;
-
 	if (!(Game_mode & GM_CAMPAIGN_MODE) || (Campaign.current_mission < 0))
 		return;
 
-	cur = Campaign.current_mission;
-	mission_obj = &Campaign.missions[cur];
+	int cur = Campaign.current_mission;
+	auto mission_obj = &Campaign.missions[cur];
 
-	// handle variables that are saved on mission victory -------------------------------------
 	mission_obj->variables.clear();
 
-	int num_mission_variables = sexp_campaign_file_variable_count();
-
-	if (num_mission_variables > 0) {
-		
-		if (store_red_alert) {
-			for (auto& current_rav : Campaign.red_alert_variables) {
-				Campaign.persistent_variables.push_back(current_rav);
-			}
+	int num_sexp_variables = sexp_variable_count();
+	for (int i = 0; i < num_sexp_variables; i++) {
+		if (!(Sexp_variables[i].type & persistence_type)) {
+			continue;
 		}
 
-		for (i = 0; i < sexp_variable_count(); i++) {
-			if (!(Sexp_variables[i].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE)) {
-				if (Sexp_variables[i].type & persistence_type) {
-					bool add_it = true;
-
-					// see if we already have a variable with this name
-					for (j = 0; j < (int)Campaign.persistent_variables.size(); j++) {
-						if (!(stricmp(Sexp_variables[i].variable_name, Campaign.persistent_variables[j].variable_name))) {
-							add_it = false;
-							Campaign.persistent_variables[j].type = Sexp_variables[i].type;
-							strcpy_s(Campaign.persistent_variables[j].text, Sexp_variables[i].text);
-							break;
-						}
-					}
-
-					// new variable
-					if (add_it) {
-						Campaign.persistent_variables.push_back(Sexp_variables[i]);
-					}
-				}
+		// player-persistent (aka "eternal")
+		if (Sexp_variables[i].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE) {
+			// see if we already have a variable with this name
+			int j = find_item_with_string(Player->variables, &sexp_variable::variable_name, Sexp_variables[i].variable_name);
+			if (j >= 0) {
+				Player->variables[j].type = Sexp_variables[i].type;
+				strcpy_s(Player->variables[j].text, Sexp_variables[i].text);
 			}
-			// we might need to save some eternal variables
-			else if ((persistence_type & SEXP_VARIABLE_SAVE_ON_MISSION_PROGRESS) && (Sexp_variables[i].type & persistence_type) && (Sexp_variables[i].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE)) {
-				bool add_it = true;
+			// new variable
+			else {
+				Player->variables.push_back(Sexp_variables[i]);
+			}
+		}
+		// campaign-persistent
+		else {
+			// see if we already have a variable with this name
+			int j = find_item_with_string(Campaign.persistent_variables, &sexp_variable::variable_name, Sexp_variables[i].variable_name);
+			if (j >= 0) {
+				Campaign.persistent_variables[j].type = Sexp_variables[i].type;
+				strcpy_s(Campaign.persistent_variables[j].text, Sexp_variables[i].text);
+			}
+			// new variable
+			else {
+				Campaign.persistent_variables.push_back(Sexp_variables[i]);
+			}
+		}
+	}
 
-				for (j = 0; j < (int)Player->variables.size(); j++) {
-					if (!(stricmp(Sexp_variables[i].variable_name, Player->variables[j].variable_name))) {
-						Player->variables[j] = Sexp_variables[i];
-
-						add_it = false;
-						break;
-					}
-				}
-
-				// if not found then add new entry
-				if (add_it) {
-					Player->variables.push_back(Sexp_variables[i]);
-				}
+	if (store_red_alert) {
+		for (const auto& current_rav : Campaign.red_alert_variables) {
+			if (find_item_with_string(Campaign.persistent_variables, &sexp_variable::variable_name, current_rav.variable_name) < 0) {
+				Campaign.persistent_variables.push_back(current_rav);
+			} else {
+				Warning(LOCATION, "A red alert variable has the same name as a persistent variable!");
 			}
 		}
 	}
@@ -1063,40 +1047,17 @@ void mission_campaign_store_containers(ContainerType persistence_type, bool stor
 	if (!(Game_mode & GM_CAMPAIGN_MODE) || (Campaign.current_mission < 0))
 		return;
 
-	if (!sexp_container_has_persistent_non_eternal_containers()) {
-		// nothing to do
-		return;
-	}
-
-	if (store_red_alert) {
-		for (const auto& current_con : Campaign.red_alert_containers) {
-			Campaign.persistent_containers.emplace_back(current_con);
-		}
-	}
-
 	for (const auto &container : get_all_sexp_containers()) {
-		if (!container.is_eternal()) {
-			if (any(container.type & persistence_type)) {
-				// see if we already have a container with this name
-				auto cpc_it = std::find_if(Campaign.persistent_containers.begin(),
-					Campaign.persistent_containers.end(),
-					[container](const sexp_container &cpc) {
-						return cpc.name_matches(container);
-					});
+		if (none(container.type & persistence_type)) {
+			continue;
+		}
 
-				if (cpc_it != Campaign.persistent_containers.end()) {
-					*cpc_it = container;
-				} else {
-					// new container
-					Campaign.persistent_containers.emplace_back(container);
-				}
-			}
-		} else if (any(persistence_type & ContainerType::SAVE_ON_MISSION_PROGRESS) &&
-				   any(container.type & persistence_type) && container.is_eternal()) {
-			// we might need to save some eternal player-persistent containers
+		// player-persistent (aka "eternal")
+		if (container.is_eternal()) {
+			// see if we already have a container with this name
 			auto ppc_it = std::find_if(Player->containers.begin(),
 				Player->containers.end(),
-				[container](const sexp_container &ppc) {
+				[&container](const sexp_container& ppc) {
 					return ppc.name_matches(container);
 				});
 
@@ -1104,17 +1065,50 @@ void mission_campaign_store_containers(ContainerType persistence_type, bool stor
 				*ppc_it = container;
 			} else {
 				// new player-persistent container
-				Player->containers.emplace_back(container);
+				Player->containers.push_back(container);
+			}
+		}
+		// campaign-persistent
+		else {
+			// see if we already have a container with this name
+			auto cpc_it = std::find_if(Campaign.persistent_containers.begin(),
+				Campaign.persistent_containers.end(),
+				[&container](const sexp_container& cpc) {
+					return cpc.name_matches(container);
+				});
+
+			if (cpc_it != Campaign.persistent_containers.end()) {
+				*cpc_it = container;
+			} else {
+				// new container
+				Campaign.persistent_containers.push_back(container);
+			}
+		}
+	}
+
+	if (store_red_alert) {
+		for (const auto& container : Campaign.red_alert_containers) {
+			// see if we already have a container with this name
+			auto cpc_it = std::find_if(Campaign.persistent_containers.begin(),
+				Campaign.persistent_containers.end(),
+				[&container](const sexp_container& cpc) {
+					return cpc.name_matches(container);
+				});
+
+			if (cpc_it == Campaign.persistent_containers.end()) {
+				Campaign.persistent_containers.push_back(container);
+			} else {
+				Warning(LOCATION, "A red alert container has the same name as a persistent container!");
 			}
 		}
 	}
 }
 
-void mission_campaign_store_goals_and_events_and_variables()
+void mission_campaign_store_goals_and_events_and_variables(bool store_red_alert_data)
 {
 	mission_campaign_store_goals_and_events();
-	mission_campaign_store_variables(SEXP_VARIABLE_SAVE_ON_MISSION_PROGRESS);
-	mission_campaign_store_containers(ContainerType::SAVE_ON_MISSION_PROGRESS);
+	mission_campaign_store_variables(SEXP_VARIABLE_SAVE_ON_MISSION_PROGRESS, store_red_alert_data);
+	mission_campaign_store_containers(ContainerType::SAVE_ON_MISSION_PROGRESS, store_red_alert_data);
 }
 
 /**
@@ -1146,9 +1140,6 @@ void mission_campaign_mission_over(bool do_next_mission)
 		Campaign.weapons_allowed[Granted_weapons[i]] = 1;	
 	}
 
-	// Goober5000 - player-persistent variables are handled when the mission is
-	// over, not necessarily when the mission is accepted
-
 	// update campaign.mission stats (used to allow backout inRedAlert)
 	// .. but we don't do this if we are inside of the prev/current loop hack
 	if ( Campaign.prev_mission != Campaign.current_mission ) {
@@ -1682,6 +1673,9 @@ void mission_campaign_end_init()
 
 void mission_campaign_end_do()
 {
+	mission_campaign_store_variables(SEXP_VARIABLE_SAVE_ON_MISSION_PROGRESS, false);
+	mission_campaign_store_containers(ContainerType::SAVE_ON_MISSION_PROGRESS, false);
+
 	// close out the mission
 	event_music_level_close();
 	mission_goal_fail_incomplete();
@@ -1728,7 +1722,7 @@ void mission_campaign_skip_to_next()
 	mission_goal_mark_events_complete();
 
 	// store
-	mission_campaign_store_goals_and_events_and_variables();
+	mission_campaign_store_goals_and_events_and_variables(false);
 
 	// now set the next mission
 	mission_campaign_eval_next_mission();
@@ -1845,84 +1839,6 @@ bool mission_campaign_jump_to_mission(const char* filename, bool no_skip)
 	}
 }
 
-// Goober5000
-void mission_campaign_save_on_close_variables()
-{
-	int i;
-
-	// make sure we are actually playing a single-player campaign
-	if (!(Game_mode & GM_CAMPAIGN_MODE) || (Campaign.type != CAMPAIGN_TYPE_SINGLE) || (Campaign.current_mission < 0))
-		return;
-
-	// now save variables
-	for (i = 0; i < sexp_variable_count(); i++) {
-		// we only want the on mission close type. On campaign progress type are dealt with elsewhere
-		if ( !(Sexp_variables[i].type & SEXP_VARIABLE_SAVE_ON_MISSION_CLOSE) ) {
-			continue;
-		}
-
-		bool found = false;
-
-		// deal with eternals 
-		if ((Sexp_variables[i].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE)) {
-			// check if variable already exists and updated it
-			for (auto& current_variable : Player->variables) {
-				if (!(stricmp(Sexp_variables[i].variable_name, current_variable.variable_name))) {
-					current_variable = Sexp_variables[i];
-
-					found = true;
-					break;
-				}
-			}
-
-			// if not found then add new entry
-			if (!found) {
-				Player->variables.push_back(Sexp_variables[i]);
-			}
-		}
-
-	}
-
-	// store any non-eternal on mission close variables
-	mission_campaign_store_variables(SEXP_VARIABLE_SAVE_ON_MISSION_CLOSE, false);
-}
-
-// jg18 - adapted from mission_campaign_save_on_close_variables()
-void mission_campaign_save_on_close_containers()
-{
-	// make sure we are actually playing a single-player campaign
-	if (!(Game_mode & GM_CAMPAIGN_MODE) || (Campaign.type != CAMPAIGN_TYPE_SINGLE) || (Campaign.current_mission < 0))
-		return;
-
-	// now save containers
-	for (const auto &container : get_all_sexp_containers()) {
-		// we only want the on mission close type. On campaign progress type are dealt with elsewhere
-		if (none(container.type & ContainerType::SAVE_ON_MISSION_CLOSE)) {
-			continue;
-		}
-
-		// deal with eternals
-		if (container.is_eternal()) {
-			// check if container already exists and update it
-			auto ppc_it = std::find_if(Player->containers.begin(),
-				Player->containers.end(),
-				[container](const sexp_container &ppc) {
-					return ppc.name_matches(container);
-				});
-
-			if (ppc_it != Player->containers.end()) {
-				*ppc_it = container;
-			} else {
-				// if not found then add new entry
-				Player->containers.emplace_back(container);
-			}
-		}
-	}
-
-	// store any non-eternal on mission close containers
-	mission_campaign_store_containers(ContainerType::SAVE_ON_MISSION_CLOSE, false);
-}
-
 void mission_campaign_load_failure_popup()
 {
 	if (Campaign_load_failure == 0) {

code/mission/missioncampaign.h

@@ -226,13 +226,13 @@ int mission_load_up_campaign(bool fall_back_from_current = false);
 void mission_campaign_store_goals_and_events();
 
 // stores variables which will be saved only on mission progression
-void mission_campaign_store_variables(int persistence_type, bool store_red_alert = true);
+void mission_campaign_store_variables(int persistence_type, bool store_red_alert);
 
 // stores containers which will be saved only on mission progression
-void mission_campaign_store_containers(ContainerType persistence_type, bool store_red_alert = true);
+void mission_campaign_store_containers(ContainerType persistence_type, bool store_red_alert);
 
 // does all three of the above
-void mission_campaign_store_goals_and_events_and_variables();
+void mission_campaign_store_goals_and_events_and_variables(bool store_red_alert_data);
 
 // evaluates next mission and possible loop mission
 void mission_campaign_eval_next_mission();
@@ -254,12 +254,6 @@ void mission_campaign_end_init();
 void mission_campaign_end_close();
 void mission_campaign_end_do();
 
-// save eternal variables
-extern void mission_campaign_save_on_close_variables();
-
-// save eternal containers
-extern void mission_campaign_save_on_close_containers();
-
 extern void mission_campaign_load_failure_popup();
 
 SCP_string mission_campaign_get_name(const char* filename);

code/mission/missionparse.cpp

@@ -6222,7 +6222,7 @@ void parse_asteroid_fields(mission *pm)
 
 void parse_variables()
 {
-	int i, j, num_variables = 0;
+	int j, num_variables = 0;
 
 	if (! optional_string("#Sexp_variables") ) {
 		return;
@@ -6236,56 +6236,41 @@ void parse_variables()
 		return;
 	}
 
-	// Goober5000 - now set the default value, if it's a variable saved on mission progress
+	// Goober5000 - now set the default value, if it's a persistent variable
+
 	// loop through the current mission's variables
 	for (j = 0; j < num_variables; j++) {
-		// check against existing variables
-		for (auto& current_pv : Campaign.persistent_variables) {
-			// if the active mission has a variable with the same name as a variable saved to the campaign file override its initial value with the previous mission's value
-			if ( !stricmp(Sexp_variables[j].variable_name, current_pv.variable_name) ) {
-				// if this is an eternal that shares the same name as a non-eternal warn but do nothing
+		// check against existing campaign variables first
+		for (const auto &campaign_var : Campaign.persistent_variables) {
+			// if the active mission has a variable with the same name as a variable saved to the campaign file, override its initial value with the persistent value
+			if ( !stricmp(Sexp_variables[j].variable_name, campaign_var.variable_name) ) {
+				// if the mission variable is eternal and shares the same name as a non-eternal persistent variable, warn but do nothing
 				if (Sexp_variables[j].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE) {
-					error_display(0, "Variable %s is marked eternal but has the same name as another persistent variable. One of these should be renamed to avoid confusion", Sexp_variables[j].text);
+					error_display(0, "Variable %s is marked eternal in the mission file but has the same name as another non-eternal persistent variable in the campaign.  One of these should be renamed to avoid confusion.", Sexp_variables[j].text);
 				}
-				else if (Sexp_variables[j].type  & SEXP_VARIABLE_IS_PERSISTENT) {
-					Sexp_variables[j].type = current_pv.type;
-					strcpy_s(Sexp_variables[j].text, current_pv.text);
-					break;
+				else if (Sexp_variables[j].type & SEXP_VARIABLE_IS_PERSISTENT) {
+					Sexp_variables[j].type = campaign_var.type;
+					strcpy_s(Sexp_variables[j].text, campaign_var.text);
 				} else {
-					error_display(0, "Variable %s has the same name as another persistent variable. One of these should be renamed to avoid confusion", Sexp_variables[j].text);
+					error_display(0, "Variable %s is not marked persistent in the mission file but has the same name as another persistent variable in the campaign.  One of these should be renamed to avoid confusion, or the mission variable should be marked persistent.", Sexp_variables[j].text);
 				}
+				break;
 			}
 		}
-	}
 
-	// next, see if any eternal variables are set loop through the current mission's variables
-	for (j = 0; j < num_variables; j++) {
-		// check against existing variables
-		for (i = 0; i < (int)Player->variables.size(); i++) {
-			// if the active mission has a variable with the same name as a variable saved to the player file override its initial value with the previous mission's value
-			if ( !stricmp(Sexp_variables[j].variable_name, Player->variables[i].variable_name) ) {
-				if (Sexp_variables[j].type & SEXP_VARIABLE_IS_PERSISTENT) {
-					// if the variable in the player file is marked as eternal but the version in the mission file is not, we assume that the player file one is rogue
-					// and use the one in the mission file instead.
-					if ((Player->variables[i].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE) && !(Sexp_variables[j].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE)) {
-						break;
-					}
-					// replace the default values with the ones saved to the player file
-					Sexp_variables[j].type = Player->variables[i].type;
-					strcpy_s(Sexp_variables[j].text, Player->variables[i].text);
-
-					/*
-					// check that the eternal flag has been set. Players using a player file from before the eternal flag was added may have old player-persistent variables
-					// these should be converted to non-eternals
-					if (!(Player->variables[i].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE)) {
-						Sexp_variables[j].type &= ~SEXP_VARIABLE_SAVE_TO_PLAYER_FILE;
-					}
-					*/
-
-					break;
+		// now check against existing player (aka "eternal") variables
+		for (const auto &player_var : Player->variables) {
+			// if the active mission has a variable with the same name as a variable saved to the player file, override its initial value with the persistent value
+			if ( !stricmp(Sexp_variables[j].variable_name, player_var.variable_name) ) {
+				// the variable in the mission file must be persistent and marked eternal,
+				// otherwise we assume that the player file variable is rogue and we do not use the persistent value
+				if ((Sexp_variables[j].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE) && (Sexp_variables[j].type & SEXP_VARIABLE_IS_PERSISTENT)) {
+					Sexp_variables[j].type = player_var.type;
+					strcpy_s(Sexp_variables[j].text, player_var.text);
 				} else {
-					error_display(0, "Variable %s has the same name as an eternal variable. One of these should be renamed to avoid confusion", Sexp_variables[j].variable_name);
+					error_display(0, "Variable %s is not marked persistent and/or not marked eternal in the mission file but has the same name as another eternally persistent variable.  This may be an unintentional name collision, or the mission variable may need to be marked eternally persistent.", Sexp_variables[j].text);
 				}
+				break;
 			}
 		}
 	}
@@ -6321,11 +6306,11 @@ void parse_sexp_containers()
 		if (p_container != nullptr) {
 			auto &container = *p_container;
 
-			// if this is an eternal container that shares the same name as a non-eternal, warn but do nothing
+			// if the mission container is eternal and shares the same name as a non-eternal persistent container, warn but do nothing
 			if (container.is_eternal()) {
 				error_display(0,
-					"SEXP container %s is marked eternal but has the same name as another persistent container. One of "
-					"these should be renamed to avoid confusion",
+					"SEXP container %s is marked eternal in the mission file but has the same name as another non-eternal persistent container in the campaign.  One of "
+					"these should be renamed to avoid confusion.",
 					container.container_name.c_str());
 			} else if (container.is_persistent()) {
 				if (container.type_matches(current_pc)) {
@@ -6336,53 +6321,48 @@ void parse_sexp_containers()
 					container = current_pc;
 				} else {
 					error_display(0,
-						"SEXP container %s is marked persistent but its type (%x) doesn't match a similarly named "
-						"persistent container's type (%x). One of "
-						"these should be renamed to avoid confusion",
+						"SEXP container %s is marked persistent in the mission file but its type (%x) doesn't match a similarly named "
+						"persistent container's type (%x) in the campaign.  One of these should be renamed to avoid confusion.",
 						container.container_name.c_str(),
 						(int)container.get_non_persistent_type(),
 						(int)current_pc.get_non_persistent_type());
 				}
 			} else {
 				error_display(0,
-					"SEXP container %s has the same name as another persistent container. One of these should be "
-					"renamed to avoid confusion",
+					"SEXP container %s is not marked persistent in the mission file but has the same name as another persistent container in the campaign.  One of these should be "
+					"renamed to avoid confusion, or the mission container should be marked persistent.",
 					container.container_name.c_str());
 			}
 		}
 	}
 
 	// then update this mission's containers from player-persistent containers
-	for (const auto& player_container : Player->containers) {
+	for (const auto &player_container : Player->containers) {
 		auto *p_container = get_sexp_container(player_container.container_name.c_str());
 		if (p_container != nullptr) {
 			auto &container = *p_container;
 
-			if (container.is_persistent()) {
-				if (player_container.is_eternal() && !container.is_eternal()) {
-					// use the mission's non-eternal container over the player-persistent eternal container
-					continue;
-				} else {
-					if (container.type_matches(player_container)) {
-						// TODO: when network containers are supported, review whether replacement should occur
-						// if one container is marked for network use and the other isn't
+			// the container in the mission file must be persistent and marked eternal,
+			// otherwise we assume that the player file container is rogue and we do not use the persistent values
+			if (container.is_eternal() && container.is_persistent()) {
+				if (container.type_matches(player_container)) {
+					// TODO: when network containers are supported, review whether replacement should occur
+					// if one container is marked for network use and the other isn't
 
-						// replace!
-						container = player_container;
-					} else {
-						error_display(0,
-							"SEXP container %s is marked persistent but its type (%x) doesn't match a similarly named "
-							"eternal container's type (%x). One of "
-							"these should be renamed to avoid confusion",
-							container.container_name.c_str(),
-							(int)container.get_non_persistent_type(),
-							(int)player_container.get_non_persistent_type());
-					}
+					// replace!
+					container = player_container;
+				} else {
+					error_display(0,
+						"SEXP container %s is marked persistent in the mission file but its type (%x) doesn't match a similarly named "
+						"persistent container's type (%x) in the player file.  One of these should be renamed to avoid confusion.",
+						container.container_name.c_str(),
+						(int)container.get_non_persistent_type(),
+						(int)player_container.get_non_persistent_type());
 				}
 			} else {
 				error_display(0,
-					"SEXP container %s has the same name as an eternal container. One of these should be renamed "
-					"to avoid confusion",
+					"SEXP container %s is not marked persistent and/or not marked eternal in the mission file but has the same name as another eternally persistent container.  This "
+					"may be an unintentional name collision, or the mission container may need to be marked eternally persistent.",
 					container.container_name.c_str());
 			}
 		}

code/missionui/missionbrief.cpp

@@ -345,7 +345,7 @@ void brief_skip_training_pressed()
 	// tricky part.  Need to move to the next mission in the campaign.
 	mission_goal_mark_objectives_complete();
 	mission_goal_fail_incomplete();
-	mission_campaign_store_goals_and_events_and_variables();
+	mission_campaign_store_goals_and_events_and_variables(false);
 
 	mission_campaign_eval_next_mission();
 	mission_campaign_mission_over();	

code/missionui/missiondebrief.cpp

@@ -1325,8 +1325,8 @@ void debrief_accept(int ok_to_post_start_game_event, bool API_Access)
 		// mission that isn't in a campaign.
 		if ( Game_mode & GM_CAMPAIGN_MODE ) {
 
-			mission_campaign_store_variables(SEXP_VARIABLE_SAVE_ON_MISSION_PROGRESS);
-			mission_campaign_store_containers(ContainerType::SAVE_ON_MISSION_PROGRESS);
+			mission_campaign_store_variables(SEXP_VARIABLE_SAVE_ON_MISSION_PROGRESS, false);
+			mission_campaign_store_containers(ContainerType::SAVE_ON_MISSION_PROGRESS, false);
 
 			// check for possible mission loop
 			// check for (1) mission loop available, (2) don't have to repeat last mission

code/missionui/missionloopbrief.cpp

@@ -94,7 +94,7 @@ int Loop_brief_bitmap;
 
 generic_anim Loop_anim;
 
-int Loop_sound;
+int Loop_sound = -1;
 
 // ---------------------------------------------------------------------------------------------------------------------------------------
 // MISSION LOOP BRIEF FUNCTIONS
@@ -127,6 +127,9 @@ void loop_brief_init()
 	int idx;
 	ui_button_info *b;
 
+	// init sound handle
+	Loop_sound = -1;
+
 	// load the background bitmap
 	Loop_brief_bitmap = bm_load(Loop_brief_fname[gr_screen.res]);
 	Assert(Loop_brief_bitmap != -1);

code/missionui/redalert.cpp

@@ -1232,7 +1232,7 @@ void red_alert_maybe_move_to_next_mission()
 			// this should handle close-out and moving to next mission
 			gameseq_post_event(GS_EVENT_DEBRIEF);
 		} else {
-			mission_campaign_store_goals_and_events_and_variables();
+			mission_campaign_store_goals_and_events_and_variables(true);
 			scoring_level_close();
 			mission_campaign_eval_next_mission();
 			mission_campaign_mission_over();

code/model/animation/modelanimation.cpp

@@ -420,7 +420,8 @@ namespace animation {
 	}
 
 	void ModelAnimationSubmodel::reset(polymodel_instance* pmi) {
-		if(!m_submodel)
+		auto cache_it = m_submodel.find(pmi->model_num);
+		if (cache_it == m_submodel.end())
 			findSubmodel(pmi);
 
 		auto dataIt = m_initialData.find({ pmi->id });
@@ -515,8 +516,9 @@ namespace animation {
 		polymodel* pm = model_get(pmi->model_num);
 
 		//Do we have a submodel number already cached?
-		if (m_submodel)
-			submodelNumber = *m_submodel;
+		auto cache_it = m_submodel.find(pm->id);
+		if (cache_it != m_submodel.end())
+			submodelNumber = cache_it->second;
 		//We seem to have a submodel name
 		else {
 			for (int i = 0; i < pm->n_models; i++) {
@@ -526,7 +528,7 @@ namespace animation {
 				}
 			}
 
-			m_submodel = submodelNumber;
+			m_submodel.emplace(pm->id, submodelNumber);
 		}
 
 		//If the model does not exist, return null. The system is expected to just silently tolerate this,
@@ -559,11 +561,12 @@ namespace animation {
 		polymodel* pm = model_get(pmi->model_num);
 
 		//Do we have a submodel number already cached?
-		if (m_submodel)
-			submodelNumber = *m_submodel;
+		auto cache_it = m_submodel.find(pm->id);
+		if (cache_it != m_submodel.end())
+			submodelNumber = cache_it->second;
 		//Do we know if we were told to find the barrel submodel or not? This implies we have a subsystem name, not a submodel name
 		else {
-			ship_info* sip = nullptr;
+			const ship_info* sip = nullptr;
 			if (pmi->objnum >= 0) {
 				sip = &Ship_info[Ships[Objects[pmi->objnum].instance].ship_info_index];
 			}
@@ -579,7 +582,7 @@ namespace animation {
 
 			for (int i = 0; i < sip->n_subsystems; i++) {
 				if (!subsystem_stricmp(sip->subsystems[i].subobj_name, m_name.c_str())) {
-					if ((bool)m_findBarrel) {
+					if (m_findBarrel) {
 						//Check if the barrel subobj is a dedicated existing subobj or just the base turret.
 						if (sip->subsystems[i].turret_gun_sobj != sip->subsystems[i].subobj_num)
 							submodelNumber = sip->subsystems[i].turret_gun_sobj;
@@ -592,7 +595,7 @@ namespace animation {
 				}
 			}
 
-			m_submodel = submodelNumber;
+			m_submodel.emplace(pm->id, submodelNumber);
 		}
 
 		//If the model does not exist, return null. The system is expected to just silently tolerate this,
@@ -682,7 +685,7 @@ namespace animation {
 
 		for (const auto& submodel : other.m_submodels) {
 			auto newSubmodel = std::shared_ptr<ModelAnimationSubmodel>(submodel->copy());
-			newSubmodel->m_submodel = std::nullopt;
+			newSubmodel->m_submodel = {};
 			m_submodels.push_back(newSubmodel);
 		}
 

code/model/animation/modelanimation.h

@@ -127,7 +127,8 @@ namespace animation {
 	class ModelAnimationSubmodel {
 	protected:
 		SCP_string m_name;
-		std::optional<int> m_submodel;
+		//This maps from model id to submodel ID, as if an animation is reused on different models (such as with techroom models), IDs might differ per model
+		SCP_unordered_map<int, int> m_submodel;
 		bool is_turret = false;
 
 	private:

code/model/model.h

@@ -137,7 +137,6 @@ struct submodel_instance
 	TIMESTAMP stepped_translation_started;
 
 	bool	blown_off = false;						// If set, this subobject is blown off
-	bool	collision_checked = false;
 
 	// 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.
@@ -453,7 +452,7 @@ public:
 	matrix	frame_of_reference;		// used to be called 'orientation' - this is just used for setting the rotation axis and the animation angles
 
 	int		bsp_data_size;
-	ubyte		*bsp_data;
+	std::shared_ptr<ubyte[]> bsp_data;
 
 	int collision_tree_index;
 
@@ -485,7 +484,7 @@ public:
 	vertex_buffer buffer;
 	vertex_buffer trans_buffer;
 
-	vertex *outline_buffer;
+	std::shared_ptr<vertex[]> outline_buffer;
 	uint n_verts_outline;
 
 	vec3d	render_box_min;
@@ -508,7 +507,7 @@ public:
 typedef struct mp_vert {
 	vec3d		pos;				// xyz coordinates of vertex in object's frame of reference
 	int			nturrets;		// number of turrets guarding this vertex
-	int			*turret_ids;	// array of indices into ship_subsys linked list (can't index using [] though)
+	std::shared_ptr<int[]>			turret_ids;	// array of indices into ship_subsys linked list (can't index using [] though)
 	float			radius;			// How far the closest obstruction is from this vertex
 } mp_vert;
 
@@ -517,7 +516,7 @@ typedef struct model_path {
 	char			parent_name[MAX_NAME_LEN];			// parent name of submodel that path is linked to in POF
 	int			parent_submodel;
 	int			nverts;
-	mp_vert		*verts;
+	std::shared_ptr<mp_vert[]> verts;
 	int			goal;			// Which of the verts is the one closest to the goal of this path
 	int			type;			// What this path takes you to... See MP_TYPE_??? defines above for details
 	int			value;		// This depends on the type.
@@ -567,7 +566,7 @@ struct glow_point{
 
 typedef struct thruster_bank {
 	int		num_points;
-	glow_point *points;
+	std::shared_ptr<glow_point[]> points;
 
 	// Engine wash info
 	struct engine_wash_info	*wash_info_pointer;		// index into Engine_wash_info
@@ -590,7 +589,7 @@ typedef struct glow_point_bank {  // glow bank structure -Bobboau
 	int			submodel_parent; 
 	int			LOD; 
 	int			num_points; 
-	glow_point	*points;
+	std::shared_ptr<glow_point[]> points;
 	int			glow_bitmap; 
 	int			glow_neb_bitmap; 
 } glow_point_bank;
@@ -650,7 +649,7 @@ typedef struct dock_bay {
 	int		num_slots;
 	int		type_flags;					// indicates what this docking bay can be used for (i.e. cargo/rearm, etc)
 	int		num_spline_paths;			// number of spline paths which lead to this docking bay
-	int		*splines;					// array of indices into the Spline_path array
+	std::shared_ptr<int[]> splines;					// array of indices into the Spline_path array
 	int		parent_submodel;			// if this dockpoint should be relative to a submodel instead of the main model
 	char		name[MAX_NAME_LEN];		// name of this docking location
 	vec3d	pnt[MAX_DOCK_SLOTS];
@@ -692,14 +691,14 @@ typedef struct shield_vertex {
 struct shield_info {
 	int				nverts;
 	int				ntris;
-	shield_vertex	*verts;
-	shield_tri		*tris;
+	std::shared_ptr<shield_vertex[]> verts;
+	std::shared_ptr<shield_tri[]> tris;
 
-	gr_buffer_handle buffer_id;
+	std::shared_ptr<gr_buffer_handle> buffer_id;
 	int buffer_n_verts;
 	vertex_layout layout;
 
-	shield_info() : nverts(0), ntris(0), verts(NULL), tris(NULL), buffer_id(-1), buffer_n_verts(0), layout() {	}
+	shield_info() : nverts(0), ntris(0), verts(nullptr), tris(nullptr), buffer_id(std::make_shared<gr_buffer_handle>(gr_buffer_handle::invalid())), buffer_n_verts(0) {	}
 };
 
 #define BSP_LIGHT_TYPE_WEAPON 1
@@ -852,7 +851,7 @@ public:
 	vec3d		bounding_box[8];
 
 	int			num_lights;							// how many lights there are
-	bsp_light *	lights;								// array of light info
+	std::shared_ptr<bsp_light[]> lights;								// array of light info
 
 	int			n_view_positions;					// number of viewing positions available on this ship
 	eye			view_positions[MAX_EYES];		//viewing positions.  Default to {0,0,0}. in location 0
@@ -866,7 +865,7 @@ public:
 	int n_textures;
 	texture_map	maps[MAX_MODEL_TEXTURES];
 	
-	bsp_info		*submodel;							// an array of size n_models of submodel info.
+	std::shared_ptr<bsp_info[]> submodel;							// an array of size n_models of submodel info.
 
 	// linked lists for special polygon types on this model.  Most ships I think will have most
 	// of these.  (most ships however, probably won't have approach points).
@@ -874,19 +873,19 @@ public:
 	int			n_missiles;							// number of secondary weapon banks (not counting turrets)
 	int			n_docks;								// number of docking points
 	int			n_thrusters;						// number of thrusters on this ship.
-	w_bank		*gun_banks;							// array of gun banks
-	w_bank		*missile_banks;					// array of missile banks
-	dock_bay		*docking_bays;						// array of docking point pairs
-	thruster_bank		*thrusters;							// array of thruster objects -- likely to change in the future
-	ship_bay_t		*ship_bay;							// contains path indexes for ship bay approach/depart paths
+	std::shared_ptr<w_bank[]> gun_banks;							// array of gun banks
+	std::shared_ptr<w_bank[]> missile_banks;					// array of missile banks
+	std::shared_ptr<dock_bay[]> docking_bays;						// array of docking point pairs
+	std::shared_ptr<thruster_bank[]> thrusters;							// array of thruster objects -- likely to change in the future
+	std::shared_ptr<ship_bay_t> ship_bay;							// contains path indexes for ship bay approach/depart paths
 
 	shield_info	shield;								// new shield information
-	ubyte	*shield_collision_tree;
+	std::shared_ptr<ubyte[]> shield_collision_tree;
 	int		sldc_size;
 	SCP_vector<vec3d>		shield_points;
 
 	int			n_paths;
-	model_path	*paths;
+	std::shared_ptr<model_path[]>	paths;
 
 	// physics info
 	float			mass;
@@ -894,7 +893,7 @@ public:
 	matrix		moment_of_inertia;	
 
 	int num_xc;				// number of cross sections
-	cross_section* xc;	// pointer to array of cross sections (used in big ship explosions)
+	std::shared_ptr<cross_section[]> xc;	// pointer to array of cross sections (used in big ship explosions)
 
 	int num_split_plane;	// number of split planes
 	float split_plane[MAX_SPLIT_PLANE];	// actual split plane z coords (for big ship explosions)
@@ -905,13 +904,13 @@ public:
 #ifndef NDEBUG
 	int			ram_used;		// How much RAM this model uses
 	int			debug_info_size;
-	char			*debug_info;
+	std::shared_ptr<char[]>	debug_info;
 #endif
 
 	int used_this_mission;		// used for page-in system, how many times this model has been loaded per mission - taylor
 
 	int n_glow_point_banks;						// number of glow points on this ship. -Bobboau
-	glow_point_bank *glow_point_banks;			// array of glow objects -Bobboau
+	std::shared_ptr<glow_point_bank[]> glow_point_banks;			// array of glow objects -Bobboau
 
 	indexed_vertex_source vert_source;
 	
@@ -1281,6 +1280,12 @@ typedef struct mc_info {
 	float   radius = 0;                 // If MC_CHECK_THICK is set, checks a sphere moving with the radius.
 	int     lod = 0;                    // Which detail level of the submodel to check instead
 
+	// Per-submodel collision_checked flags (indexed by submodel number).
+	// When non-empty, model_collide uses this to determine which submodels to skip.
+	// Auto-initialized from pmi in model_collide when empty; callers may pre-populate
+	// to control which submodels are checked (e.g., rotating submodel collision).
+	SCP_vector<char> collision_checked;
+
 	// Return values
 	int     num_hits = 0;               // How many collisions were found
 	float   hit_dist = 0.0f;            // The distance from p0 to hitpoint

code/model/modelcollide.cpp

@@ -859,7 +859,7 @@ bool mc_shield_check_common(shield_tri	*tri)
 		// also finds the uv's where the ray hit.
 		if ( fvi_point_face(&hitpoint, 3, points, &tri->norm, NULL,NULL,NULL ) )	{
 			Mc->hit_dist = dist;
-			Mc->shield_hit_tri = (int)(tri - Mc_pm->shield.tris);
+			Mc->shield_hit_tri = (int)(tri - Mc_pm->shield.tris.get());
 			Mc->hit_point = hitpoint;
 			Mc->hit_normal = tri->norm;
 			Mc->hit_submodel = -1;
@@ -877,7 +877,7 @@ bool mc_shield_check_common(shield_tri	*tri)
 			// same behavior whether face or edge
 			// normal, edge_hit, hit_point all updated thru sphereline_face
 			sphere_check_closest_shield_dist = Mc->hit_dist;
-			Mc->shield_hit_tri = (int)(tri - Mc_pm->shield.tris);
+			Mc->shield_hit_tri = (int)(tri - Mc_pm->shield.tris.get());
 			Mc->hit_submodel = -1;
 			Mc->num_hits++;
 			return true;		// We hit, so we're done
@@ -893,22 +893,22 @@ bool mc_check_sldc(int offset)
 	if (offset > Mc_pm->sldc_size - 5) //no way is this big enough
 		return false;
 
-	int* type_p = (int*)(Mc_pm->shield_collision_tree + offset);
+	int* type_p = (int*)(Mc_pm->shield_collision_tree.get() + offset);
 
 	// not used
 	//int *size_p = (int *)(Mc_pm->shield_collision_tree+offset+4);
 	// split and polygons
-	auto* minbox_p = (vec3d*)(Mc_pm->shield_collision_tree + offset + 8);
-	auto* maxbox_p = (vec3d*)(Mc_pm->shield_collision_tree + offset + 20);
+	auto* minbox_p = (vec3d*)(Mc_pm->shield_collision_tree.get() + offset + 8);
+	auto* maxbox_p = (vec3d*)(Mc_pm->shield_collision_tree.get() + offset + 20);
 
 	// split
-	auto* front_offset_p = (unsigned int*)(Mc_pm->shield_collision_tree + offset + 32);
-	auto* back_offset_p = (unsigned int*)(Mc_pm->shield_collision_tree + offset + 36);
+	auto* front_offset_p = (unsigned int*)(Mc_pm->shield_collision_tree.get() + offset + 32);
+	auto* back_offset_p = (unsigned int*)(Mc_pm->shield_collision_tree.get() + offset + 36);
 
 	// polygons
-	auto* num_polygons_p = (unsigned int*)(Mc_pm->shield_collision_tree + offset + 32);
+	auto* num_polygons_p = (unsigned int*)(Mc_pm->shield_collision_tree.get() + offset + 32);
 
-	auto* shld_polys = (unsigned int*)(Mc_pm->shield_collision_tree + offset + 36);
+	auto* shld_polys = (unsigned int*)(Mc_pm->shield_collision_tree.get() + offset + 36);
 
 
 
@@ -1098,7 +1098,7 @@ NoHit:
 			vm_vec_add2(&instance_offset, &csmi->canonical_offset);
 
 			blown_off = csmi->blown_off;
-			collision_checked = csmi->collision_checked;
+			collision_checked = !Mc->collision_checked.empty() && Mc->collision_checked[i];
 		}
 
 		// Don't check it or its children if it is destroyed
@@ -1150,6 +1150,11 @@ int model_collide(mc_info *mc_info_obj)
 		Mc_pmi = NULL;
 	}
 
+	if (Mc_pmi && Mc->collision_checked.size() != static_cast<size_t>(Mc_pm->n_models)) {
+		Assertion(Mc->collision_checked.empty(), "model_collide was called with a dirty mc_info state! Please report to the SCP.");
+		Mc->collision_checked.resize(Mc_pm->n_models, 0);
+	}
+
 	// DA 11/19/98 - disable this check for rotating submodels
 	// Don't do check if for very small movement
 //	if (Mc_mag < 0.01f) {

code/model/modelinterp.cpp

@@ -1345,7 +1345,7 @@ int submodel_get_num_polys(int model_num, int submodel_num )
 
 	pm = model_get(model_num);
 
-	return submodel_get_num_polys_sub( pm->submodel[submodel_num].bsp_data );
+	return submodel_get_num_polys_sub( pm->submodel[submodel_num].bsp_data.get() );
 }
 
 /**
@@ -1946,9 +1946,9 @@ void model_interp_submit_buffers(indexed_vertex_source *vert_src, size_t vertex_
 		return;
 	}
 
-	if ( vert_src->Vertex_list != NULL ) {
+	if ( vert_src->Vertex_list != nullptr ) {
 		size_t offset;
-		gr_heap_allocate(GpuHeap::ModelVertex, vert_src->Vertex_list_size, vert_src->Vertex_list, offset, vert_src->Vbuffer_handle);
+		gr_heap_allocate(GpuHeap::ModelVertex, vert_src->Vertex_list_size, vert_src->Vertex_list.get(), offset, vert_src->Vbuffer_handle);
 
 		// If this happens then someone must have allocated something from the heap with a different stride than what we
 		// are using.
@@ -1956,15 +1956,13 @@ void model_interp_submit_buffers(indexed_vertex_source *vert_src, size_t vertex_
 		vert_src->Base_vertex_offset = offset / vertex_stride;
 		vert_src->Vertex_offset = offset;
 
-		vm_free(vert_src->Vertex_list);
-		vert_src->Vertex_list = NULL;
+		vert_src->Vertex_list.reset();
 	}
 
-	if ( vert_src->Index_list != NULL ) {
-		gr_heap_allocate(GpuHeap::ModelIndex, vert_src->Index_list_size, vert_src->Index_list, vert_src->Index_offset, vert_src->Ibuffer_handle);
+	if ( vert_src->Index_list != nullptr ) {
+		gr_heap_allocate(GpuHeap::ModelIndex, vert_src->Index_list_size, vert_src->Index_list.get(), vert_src->Index_offset, vert_src->Ibuffer_handle);
 
-		vm_free(vert_src->Index_list);
-		vert_src->Index_list = NULL;
+		vert_src->Index_list.reset();
 	}
 }
 
@@ -1978,30 +1976,26 @@ bool model_interp_pack_buffer(indexed_vertex_source *vert_src, vertex_buffer *vb
 
 	int i, n_verts = 0;
 	size_t j;
-	if ( vert_src->Vertex_list == NULL ) {
-		vert_src->Vertex_list = vm_malloc(vert_src->Vertex_list_size);
+	if ( vert_src->Vertex_list == nullptr ) {
+		vert_src->Vertex_list = make_shared<uint8_t[]>(vert_src->Vertex_list_size);
 
 		// return invalid if we don't have the memory
-		if ( vert_src->Vertex_list == NULL ) {
+		if ( vert_src->Vertex_list == nullptr ) {
 			return false;
 		}
-
-		memset(vert_src->Vertex_list, 0, vert_src->Vertex_list_size);
 	}
 
-	if ( vert_src->Index_list == NULL ) {
-		vert_src->Index_list = vm_malloc(vert_src->Index_list_size);
+	if ( vert_src->Index_list == nullptr ) {
+		vert_src->Index_list = make_shared<uint8_t[]>(vert_src->Index_list_size);
 
 		// return invalid if we don't have the memory
-		if ( vert_src->Index_list == NULL ) {
+		if ( vert_src->Index_list == nullptr ) {
 			return false;
 		}
-
-		memset(vert_src->Index_list, 0, vert_src->Index_list_size);
 	}
 
 	// bump to our index in the array
-	auto array = reinterpret_cast<interp_vertex*>(static_cast<uint8_t*>(vert_src->Vertex_list) + (vb->vertex_offset));
+	auto array = reinterpret_cast<interp_vertex*>(vert_src->Vertex_list.get() + vb->vertex_offset);
 
 	// generate the vertex array
 	n_verts = vb->model_list->n_verts;
@@ -2067,7 +2061,7 @@ bool model_interp_pack_buffer(indexed_vertex_source *vert_src, vertex_buffer *vb
 		const uint *index = tex_buf->get_index();
 
 		// bump to our spot in the buffer
-		auto ibuf = static_cast<uint8_t*>(vert_src->Index_list) + offset;
+		auto ibuf = vert_src->Index_list.get() + offset;
 
 		if ( vb->tex_buf[j].flags & VB_FLAG_LARGE_INDEX ) {
 			memcpy(ibuf, index, n_verts * sizeof(uint));
@@ -2179,7 +2173,7 @@ void interp_configure_vertex_buffers(polymodel *pm, int mn, const model_read_def
 
 	int milliseconds = timer_get_milliseconds();
 
-	auto bsp_polies = new bsp_polygon_data(model->bsp_data, model->bsp_data_size);
+	auto bsp_polies = new bsp_polygon_data(model->bsp_data.get(), model->bsp_data_size);
 
 	auto textureReplace = deferredTasks.texture_replacements.find(mn);
 	if (textureReplace != deferredTasks.texture_replacements.end())
@@ -2211,9 +2205,9 @@ void interp_configure_vertex_buffers(polymodel *pm, int mn, const model_read_def
 
 	if ( outline_n_lines > 0 ) {
 		model->n_verts_outline = outline_n_lines * 2;
-		model->outline_buffer = (vertex*)vm_malloc(sizeof(vertex) * model->n_verts_outline);
+		model->outline_buffer = make_shared<vertex[]>(model->n_verts_outline);
 
-		bsp_polies->generate_lines(-1, model->outline_buffer);
+		bsp_polies->generate_lines(-1, model->outline_buffer.get());
 	}
 
 	// done with the bsp now that we have the vertex data
@@ -2586,14 +2580,16 @@ void model_interp_process_shield_mesh(polymodel * pm)
 	}
 	
 	if ( !buffer.empty() ) {
-		pm->shield.buffer_id = gr_create_buffer(BufferType::Vertex, BufferUsageHint::Static);
-		pm->shield.buffer_n_verts = n_verts;
-		gr_update_buffer_data(pm->shield.buffer_id, buffer.size() * sizeof(vec3d), &buffer[0]);
+		if (*pm->shield.buffer_id == gr_buffer_handle::invalid()) {
+			*pm->shield.buffer_id = gr_create_buffer(BufferType::Vertex, BufferUsageHint::Static);
+			pm->shield.buffer_n_verts = n_verts;
+			gr_update_buffer_data(*pm->shield.buffer_id, buffer.size() * sizeof(vec3d), buffer.data());
 
-		pm->shield.layout.add_vertex_component(vertex_format_data::POSITION3, sizeof(vec3d) * 2, 0);
-		pm->shield.layout.add_vertex_component(vertex_format_data::NORMAL, sizeof(vec3d) * 2, sizeof(vec3d));
+			pm->shield.layout.add_vertex_component(vertex_format_data::POSITION3, sizeof(vec3d) * 2, 0);
+			pm->shield.layout.add_vertex_component(vertex_format_data::NORMAL, sizeof(vec3d) * 2, sizeof(vec3d));
+		}
 	} else {
-		pm->shield.buffer_id = gr_buffer_handle::invalid();
+		*pm->shield.buffer_id = gr_buffer_handle::invalid();
 	}
 }
 
@@ -3303,5 +3299,5 @@ void bsp_polygon_data::replace_textures_used(const SCP_map<int, int>& replacemen
 }
 
 SCP_set<int> model_get_textures_used(const polymodel* pm, int submodel) {
-	return bsp_polygon_data{ pm->submodel[submodel].bsp_data, pm->submodel[submodel].bsp_data_size }.get_textures_used();
+	return bsp_polygon_data{ pm->submodel[submodel].bsp_data.get(), pm->submodel[submodel].bsp_data_size }.get_textures_used();
 }

code/model/modelread.cpp

@@ -186,112 +186,24 @@ public:
 
 SCP_unordered_map<int, intrinsic_motion> Intrinsic_motions;
 
-
 void model_free(polymodel* pm)
 {
-	int i, j;
-	safe_kill(pm->ship_bay);
-
-	if (pm->paths) {
-		for (i = 0; i < pm->n_paths; i++) {
-			for (j = 0; j < pm->paths[i].nverts; j++) {
-				if (pm->paths[i].verts[j].turret_ids) {
-					vm_free(pm->paths[i].verts[j].turret_ids);
-				}
-			}
-			if (pm->paths[i].verts) {
-				vm_free(pm->paths[i].verts);
-			}
-		}
-		vm_free(pm->paths);
-	}
-
-	if (pm->shield.verts) {
-		vm_free(pm->shield.verts);
-	}
-
-	if (pm->shield.tris) {
-		vm_free(pm->shield.tris);
-	}
-
-	if (pm->gun_banks) {	// NOLINT
-		delete[] pm->gun_banks;
-	}
-
-	if (pm->missile_banks) {	// NOLINT
-		delete[] pm->missile_banks;
-	}
-
-	if (pm->docking_bays) {
-		for (i = 0; i < pm->n_docks; i++) {
-			if (pm->docking_bays[i].splines) {
-				vm_free(pm->docking_bays[i].splines);
-			}
-		}
-		vm_free(pm->docking_bays);
-	}
-
-
-	if (pm->thrusters) {
-		for (i = 0; i < pm->n_thrusters; i++) {
-			if (pm->thrusters[i].points)
-				vm_free(pm->thrusters[i].points);
-		}
-
-		vm_free(pm->thrusters);
-	}
-
-	if (pm->glow_point_banks) { // free the glows!!! -Bobboau
-		for (i = 0; i < pm->n_glow_point_banks; i++) {
-			if (pm->glow_point_banks[i].points)
-				vm_free(pm->glow_point_banks[i].points);
-		}
-
-		vm_free(pm->glow_point_banks);
-	}
-
-#ifndef NDEBUG
-	if (pm->debug_info) {
-		vm_free(pm->debug_info);
-	}
-#endif
+	int i;
 
 	if (pm->submodel) {
 		for (i = 0; i < pm->n_models; i++) {
 			pm->submodel[i].buffer.clear();
 
-			if (pm->submodel[i].bsp_data) {
-				vm_free(pm->submodel[i].bsp_data);
-			}
-
 			if (pm->submodel[i].collision_tree_index >= 0) {
 				model_remove_bsp_collision_tree(pm->submodel[i].collision_tree_index);
-			}
-
-			if (pm->submodel[i].outline_buffer != nullptr) {
-				vm_free(pm->submodel[i].outline_buffer);
-				pm->submodel[i].outline_buffer = nullptr;
+				pm->submodel[i].collision_tree_index = 0;
 			}
 		}
-
-		delete[] pm->submodel;
 	}
 
-	if (pm->xc) {
-		vm_free(pm->xc);
-	}
-
-	if (pm->lights) {
-		vm_free(pm->lights);
-	}
-
-	if (pm->shield_collision_tree) {
-		vm_free(pm->shield_collision_tree);
-	}
-
-	if (pm->shield.buffer_id.isValid()) {
-		gr_delete_buffer(pm->shield.buffer_id);
-		pm->shield.buffer_id = gr_buffer_handle::invalid();
+	if (pm->shield.buffer_id->isValid()) {
+		gr_delete_buffer(*pm->shield.buffer_id);
+		*pm->shield.buffer_id = gr_buffer_handle::invalid();
 		pm->shield.buffer_n_verts = 0;
 	}
 
@@ -303,10 +215,7 @@ void model_free(polymodel* pm)
 		pm->vert_source.Base_vertex_offset = 0;
 	}
 
-	if (pm->vert_source.Vertex_list != NULL) {
-		vm_free(pm->vert_source.Vertex_list);
-		pm->vert_source.Vertex_list = NULL;
-	}
+	pm->vert_source.Vertex_list.reset();
 
 	if (pm->vert_source.Ibuffer_handle.isValid()) {
 		gr_heap_deallocate(GpuHeap::ModelIndex, pm->vert_source.Index_offset);
@@ -315,10 +224,7 @@ void model_free(polymodel* pm)
 		pm->vert_source.Index_offset = 0;
 	}
 
-	if (pm->vert_source.Index_list != NULL) {
-		vm_free(pm->vert_source.Index_list);
-		pm->vert_source.Index_list = NULL;
-	}
+	pm->vert_source.Index_list.reset();
 
 	pm->vert_source.Vertex_list_size = 0;
 	pm->vert_source.Index_list_size = 0;
@@ -371,6 +277,15 @@ void model_unload(int modelnum, int force)
 	for (auto& si : Ship_info) {
 		if (pm->id == si.model_num) {
 			si.model_num = -1;
+
+			// also reset any subsystem model_num references that pointed to this model,
+			// otherwise stale ids can survive across missions and cause subsystems to fail
+			// to re-link when the model is reloaded with a different id.
+			for (int k = 0; k < si.n_subsystems; k++) {
+				if (si.subsystems[k].model_num == pm->id) {
+					si.subsystems[k].model_num = -1;
+				}
+			}
 		}
 
 		if (pm->id == si.cockpit_model_num) {
@@ -686,7 +601,11 @@ void model_copy_subsystems( int n_subsystems, model_subsystem *d_sp, model_subsy
 			}
 		}
 		if ( j == n_subsystems )
-			Int3();							// get allender -- something is amiss with models
+			Error(LOCATION, "Subsystem '%s' could not be matched between two ship classes that share a model. "
+				"The destination ship will be missing this subsystem at runtime. "
+				"Check that subsystem names are spelled identically on both ship classes, "
+				"and that any modular table extensions to one ship are mirrored on the other.",
+				source->subobj_name);
 
 	}
 }
@@ -1652,8 +1571,6 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 	}
 
 	pm->version = version;
-	Assert(strlen(filename) < FILESPEC_LENGTH );
-	strcpy_s(pm->filename, filename);
 
 	memset( &pm->view_positions, 0, sizeof(pm->view_positions) );
 
@@ -1708,7 +1625,7 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 					Warning(LOCATION, "Model <%s> has a radius <= 0.00001f\n", filename);
 				}
 
-				pm->submodel = new bsp_info[MAX(1,pm->n_models)];
+				pm->submodel = make_shared<bsp_info[]>(MAX(1,pm->n_models));
 
 				//Assert(pm->n_models <= MAX_SUBMODELS);
 
@@ -1801,11 +1718,11 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 				}
 
 				// read in cross section info
-				pm->xc = NULL;
+				pm->xc = nullptr;
 				if ( pm->version >= 2014 ) {
 					pm->num_xc = cfread_int(fp);
 					if (pm->num_xc > 0) {
-						pm->xc = (cross_section*) vm_malloc(pm->num_xc*sizeof(cross_section));
+						pm->xc = make_shared<cross_section[]>(pm->num_xc);
 						for (i=0; i<pm->num_xc; i++) {
 							pm->xc[i].z = cfread_float(fp);
 							pm->xc[i].radius = cfread_float(fp);
@@ -1820,7 +1737,7 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 					//mprintf(( "Found %d lights!\n", pm->num_lights ));
 
 					if (pm->num_lights > 0) {
-						pm->lights = (bsp_light *)vm_malloc( sizeof(bsp_light)*pm->num_lights );
+						pm->lights = make_shared<bsp_light[]>(pm->num_lights );
 						for (i=0; i<pm->num_lights; i++ )	{			
 							cfread_vector(&pm->lights[i].pos,fp);
 							pm->lights[i].type = cfread_int(fp);
@@ -2112,9 +2029,9 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 				{
 					sm->bsp_data_size = cfread_int(fp);
 					if (sm->bsp_data_size > 0) {
-						sm->bsp_data = reinterpret_cast<ubyte*>(vm_malloc(sm->bsp_data_size));
-						cfread(sm->bsp_data, 1, sm->bsp_data_size, fp);
-						swap_bsp_data(pm, sm->bsp_data);
+						sm->bsp_data = make_shared<ubyte[]>(sm->bsp_data_size);
+						cfread(sm->bsp_data.get(), 1, sm->bsp_data_size, fp);
+						swap_bsp_data(pm, sm->bsp_data.get());
 					}
 					else {
 						sm->bsp_data = nullptr;
@@ -2125,28 +2042,27 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 					sm->bsp_data_size = cfread_int(fp);
 
 					if (sm->bsp_data_size > 0) {
-						auto bsp_data = reinterpret_cast<ubyte*>(vm_malloc(sm->bsp_data_size));
+						auto bsp_data = make_shared<ubyte[]>(sm->bsp_data_size);
 
-						cfread(bsp_data, 1, sm->bsp_data_size, fp);
+						cfread(bsp_data.get(), 1, sm->bsp_data_size, fp);
 
 						// byte swap first thing
-						swap_bsp_data(pm, bsp_data);
+						swap_bsp_data(pm, bsp_data.get());
 
 						extern bool Cmdline_no_bsp_align;
 						if (Cmdline_no_bsp_align) {
 							sm->bsp_data = bsp_data;
 						}
 						else {
-							auto bsp_data_size_aligned = align_bsp_data(bsp_data, nullptr, sm->bsp_data_size);
+							auto bsp_data_size_aligned = align_bsp_data(bsp_data.get(), nullptr, sm->bsp_data_size);
 
 							if (bsp_data_size_aligned != static_cast<uint>(sm->bsp_data_size)) {
-								auto bsp_data_aligned = reinterpret_cast<ubyte*>(vm_malloc(bsp_data_size_aligned));
+								auto bsp_data_aligned = make_shared<ubyte[]>(bsp_data_size_aligned);
 
-								align_bsp_data(bsp_data, bsp_data_aligned, sm->bsp_data_size);
+								align_bsp_data(bsp_data.get(), bsp_data_aligned.get(), sm->bsp_data_size);
 
 								// release unaligned data
-								vm_free(bsp_data);
-								bsp_data = nullptr;
+								bsp_data.reset();
 
 								nprintf(("Model", "BSP ALIGN => %s:%s resized by %d bytes (%d total)\n", pm->filename, sm->name, bsp_data_size_aligned - sm->bsp_data_size, bsp_data_size_aligned));
 
@@ -2190,9 +2106,9 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 					//mprintf(("SLDC Shield Collision Tree was %d bytes in size\n", pm->sldc_size));
 					pm->sldc_size = convert_sldc_to_slc2(sldc_tree.get(), slc2_tree.get(), pm->sldc_size);
 					//mprintf(("SLC2 Shield Collision Tree is %d bytes in size\n", pm->sldc_size));
-					pm->shield_collision_tree = (ubyte*)vm_malloc(pm->sldc_size); //sldc_size is slc2 size, reused variable
-					memcpy(pm->shield_collision_tree, slc2_tree.get(), pm->sldc_size);
-					swap_sldc_data(pm->shield_collision_tree);
+					pm->shield_collision_tree = make_shared<ubyte[]>(pm->sldc_size); //sldc_size is slc2 size, reused variable
+					memcpy(pm->shield_collision_tree.get(), slc2_tree.get(), pm->sldc_size);
+					swap_sldc_data(pm->shield_collision_tree.get());
 				}
 			}
 			break;
@@ -2201,9 +2117,9 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 			{
 				if (pm->version >= 2200) {
 					pm->sldc_size = cfread_int(fp);
-					pm->shield_collision_tree = (ubyte*)vm_malloc(pm->sldc_size);
-					cfread(pm->shield_collision_tree, 1, pm->sldc_size, fp);
-					swap_sldc_data(pm->shield_collision_tree);
+					pm->shield_collision_tree = make_shared<ubyte[]>(pm->sldc_size);
+					cfread(pm->shield_collision_tree.get(), 1, pm->sldc_size, fp);
+					swap_sldc_data(pm->shield_collision_tree.get());
 					//mprintf(( "SLC2 Shield Collision Tree, %d bytes in size\n", pm->sldc_size));
 				}
 			}
@@ -2214,7 +2130,7 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 					pm->shield.nverts = cfread_int( fp );		// get the number of vertices in the list
 
 					if (pm->shield.nverts > 0) {
-						pm->shield.verts = (shield_vertex *)vm_malloc(pm->shield.nverts * sizeof(shield_vertex) );
+						pm->shield.verts = make_shared<shield_vertex[]>(pm->shield.nverts);
 						Assert( pm->shield.verts );
 						for ( i = 0; i < pm->shield.nverts; i++ ) {						// read in the vertex list
 							cfread_vector( &(pm->shield.verts[i].pos), fp );
@@ -2224,7 +2140,7 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 					pm->shield.ntris = cfread_int( fp );		// get the number of triangles that compose the shield
 
 					if (pm->shield.ntris > 0) {
-						pm->shield.tris = (shield_tri *)vm_malloc(pm->shield.ntris * sizeof(shield_tri) );
+						pm->shield.tris = make_shared<shield_tri[]>(pm->shield.ntris);
 						Assert( pm->shield.tris );
 						for ( i = 0; i < pm->shield.ntris; i++ ) {
 							cfread_vector( &temp_vec, fp );
@@ -2257,11 +2173,11 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 			case ID_MPNT:
 			{
 				int n_weps = cfread_int(fp);
-				w_bank *wep_banks = nullptr;
+				std::shared_ptr<w_bank[]> wep_banks = nullptr;
 
 				if (n_weps > 0)
 				{
-					wep_banks = new w_bank[n_weps];
+					wep_banks = make_shared<w_bank[]>(n_weps);
 					for (i = 0; i < n_weps; ++i)
 					{
 						w_bank *bank = &wep_banks[i];
@@ -2310,8 +2226,7 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 				pm->n_docks = cfread_int(fp);
 
 				if (pm->n_docks > 0) {
-					pm->docking_bays = (dock_bay *)vm_malloc(sizeof(dock_bay) * pm->n_docks);
-					Assert( pm->docking_bays != NULL );
+					pm->docking_bays = make_shared<dock_bay[]>(pm->n_docks);
 
 					for (i = 0; i < pm->n_docks; i++ ) {
 						char *p;
@@ -2340,7 +2255,7 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 
 						bay->num_spline_paths = cfread_int( fp );
 						if ( bay->num_spline_paths > 0 ) {
-							bay->splines = (int *)vm_malloc(sizeof(int) * bay->num_spline_paths);
+							bay->splines = make_shared<int[]>(bay->num_spline_paths);
 							for ( j = 0; j < bay->num_spline_paths; j++ )
 								bay->splines[j] = cfread_int(fp);
 						} else {
@@ -2417,13 +2332,10 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 				int gpb_num = cfread_int(fp);
 
 				pm->n_glow_point_banks = gpb_num;
-				pm->glow_point_banks = NULL;
+				pm->glow_point_banks = nullptr;
 
 				if (gpb_num > 0)
-				{
-					pm->glow_point_banks = (glow_point_bank *) vm_malloc(sizeof(glow_point_bank) * gpb_num);
-					Assert(pm->glow_point_banks != NULL);
-				}
+					pm->glow_point_banks = make_shared<glow_point_bank[]>(gpb_num);
 
 				for (int gpb = 0; gpb < gpb_num; gpb++)
 				{
@@ -2438,12 +2350,12 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 					bank->LOD = cfread_int(fp);
 					bank->type = cfread_int(fp);
 					bank->num_points = cfread_int(fp);
-					bank->points = NULL;
+					bank->points = nullptr;
 					bank->glow_bitmap = -1;
 					bank->glow_neb_bitmap = -1;
 
 					if (bank->num_points > 0)
-						bank->points = (glow_point *) vm_malloc(sizeof(glow_point) * bank->num_points);
+						bank->points = make_shared<glow_point[]>(bank->num_points);
 
 					//if((bank->off_time > 0) && (bank->disp_time > 0))
 						//bank->is_on = false;
@@ -2517,17 +2429,16 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 				pm->n_thrusters = cfread_int(fp);
 
 				if (pm->n_thrusters > 0) {
-					pm->thrusters = (thruster_bank *)vm_malloc(sizeof(thruster_bank) * pm->n_thrusters);
-					Assert( pm->thrusters != NULL );
+					pm->thrusters = make_shared<thruster_bank[]>(pm->n_thrusters);
 
 					for (i = 0; i < pm->n_thrusters; i++ ) {
 						thruster_bank *bank = &pm->thrusters[i];
 
 						bank->num_points = cfread_int(fp);
-						bank->points = NULL;
+						bank->points = nullptr;
 
 						if (bank->num_points > 0)
-							bank->points = (glow_point *) vm_malloc(sizeof(glow_point) * bank->num_points);
+							bank->points = make_shared<glow_point[]>(bank->num_points);
 
 						bank->obj_num = -1;
 						bank->submodel_num = -1;
@@ -2703,10 +2614,8 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 
 				#ifndef NDEBUG
 					pm->debug_info_size = len;
-					pm->debug_info = (char *)vm_malloc(pm->debug_info_size+1);
-					Assert(pm->debug_info!=NULL);
-					memset(pm->debug_info,0,len+1);
-					cfread( pm->debug_info, 1, len, fp );
+					pm->debug_info = make_shared<char[]>(pm->debug_info_size+1);
+					cfread( pm->debug_info.get(), 1, len, fp );
 				#endif
 				break;
 
@@ -2720,10 +2629,7 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 					break;
 				}
 
-				pm->paths = (model_path *)vm_malloc(sizeof(model_path)*pm->n_paths);
-				Assert( pm->paths != NULL );
-
-				memset( pm->paths, 0, sizeof(model_path) * pm->n_paths );
+				pm->paths = make_shared<model_path[]>(pm->n_paths);
 					
 				for (i=0; i<pm->n_paths; i++ )	{
 					cfread_string_len(pm->paths[i].name, MAX_NAME_LEN, fp);
@@ -2757,12 +2663,10 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 					}
 
 					pm->paths[i].nverts = cfread_int( fp );
-					pm->paths[i].verts = (mp_vert *)vm_malloc( sizeof(mp_vert) * pm->paths[i].nverts );
+					pm->paths[i].verts = make_shared<mp_vert[]>(pm->paths[i].nverts);
 					pm->paths[i].goal = pm->paths[i].nverts - 1;
 					pm->paths[i].type = MP_TYPE_UNUSED;
 					pm->paths[i].value = 0;
-					Assert(pm->paths[i].verts!=NULL);
-					memset( pm->paths[i].verts, 0, sizeof(mp_vert) * pm->paths[i].nverts );
 
 					for (j=0; j<pm->paths[i].nverts; j++ )	{
 						cfread_vector(&pm->paths[i].verts[j].pos,fp );
@@ -2775,7 +2679,7 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 							pm->paths[i].verts[j].nturrets = nturrets;
 
 							if (nturrets > 0) {
-								pm->paths[i].verts[j].turret_ids = (int *)vm_malloc( sizeof(int) * nturrets );
+								pm->paths[i].verts[j].turret_ids = make_shared<int[]>(nturrets);
 								for ( k = 0; k < nturrets; k++ )
 									pm->paths[i].verts[j].turret_ids[k] = cfread_int( fp );
 							}
@@ -3051,6 +2955,9 @@ modelread_status read_model_file(polymodel* pm, const char* filename, ErrorType
 //reads a binary file containing a 3d model
 modelread_status read_and_process_model_file(polymodel* pm, const char* filename, int n_subsystems, model_subsystem* subsystems, ErrorType error_type, model_read_deferred_tasks& deferredTasks)
 {
+	Assert(strlen(filename) < FILESPEC_LENGTH );
+	strcpy_s(pm->filename, filename);
+	
 	modelread_status status = read_model_file(pm, filename, error_type, deferredTasks);
 
 	//By now, we have finished reading this model. If it was virtual, we might have accumulated cache.
@@ -3558,8 +3465,8 @@ int model_load(const  char* filename, ship_info* sip, ErrorType error_type, bool
 		pm->submodel[i].collision_tree_index = model_create_bsp_collision_tree();
 		bsp_collision_tree* tree             = model_get_bsp_collision_tree(pm->submodel[i].collision_tree_index);
 
-		Macro_ubyte_bounds = pm->submodel[i].bsp_data + pm->submodel[i].bsp_data_size;
-		model_collide_parse_bsp(tree, pm->submodel[i].bsp_data, pm->version);
+		Macro_ubyte_bounds = pm->submodel[i].bsp_data.get() + pm->submodel[i].bsp_data_size;
+		model_collide_parse_bsp(tree, pm->submodel[i].bsp_data.get(), pm->version);
 		Macro_ubyte_bounds = nullptr;
 	}
 
@@ -3742,10 +3649,9 @@ void model_set_bay_path_nums(polymodel *pm)
 {
 	int i;
 
-	if (pm->ship_bay != NULL)
+	if (pm->ship_bay != nullptr)
 	{
-		vm_free(pm->ship_bay);
-		pm->ship_bay = NULL;
+		pm->ship_bay.reset();
 	}
 
 	/*
@@ -3756,8 +3662,7 @@ void model_set_bay_path_nums(polymodel *pm)
 	*/
 
 	// malloc out storage for the path information
-	pm->ship_bay = (ship_bay *) vm_malloc(sizeof(ship_bay));
-	Assert(pm->ship_bay != NULL);
+	pm->ship_bay = make_shared<ship_bay_t>();
 
 	pm->ship_bay->num_paths = 0;
 	// TODO: determine if zeroing out here is affecting any earlier initializations

code/model/modelrender.cpp

@@ -1291,7 +1291,7 @@ void model_render_children_buffers(model_draw_list* scene, model_material *rende
 	if ( (model_flags & MR_SHOW_OUTLINE || model_flags & MR_SHOW_OUTLINE_HTL || model_flags & MR_SHOW_OUTLINE_PRESET) && 
 		sm->outline_buffer != nullptr ) {
 		color outline_color = interp->get_color();
-		scene->add_outline(sm->outline_buffer, sm->n_verts_outline, &outline_color);
+		scene->add_outline(sm->outline_buffer.get(), sm->n_verts_outline, &outline_color);
 	} else {
 		if ( trans_buffer && sm->trans_buffer.flags & VB_FLAG_TRANS ) {
 			model_render_buffers(scene, rendering_material, interp, &sm->trans_buffer, pm, mn, detail_level, tmap_flags);
@@ -2930,7 +2930,7 @@ void model_render_queue(const model_render_params* interp, model_draw_list* scen
 
 		if ( (is_outlines_only || is_outlines_only_htl) && pm->submodel[detail_model_num].outline_buffer != NULL ) {
 			color outline_color = interp->get_color();
-			scene->add_outline(pm->submodel[detail_model_num].outline_buffer, pm->submodel[detail_model_num].n_verts_outline, &outline_color);
+			scene->add_outline(pm->submodel[detail_model_num].outline_buffer.get(), pm->submodel[detail_model_num].n_verts_outline, &outline_color);
 		} else {
 			model_render_buffers(scene, &rendering_material, interp, &pm->submodel[detail_model_num].buffer, pm, detail_model_num, detail_level, tmap_flags);
 

code/model/modelreplace.cpp

@@ -202,10 +202,12 @@ void virtual_pof_init() {
 
 template<typename T, bool vmalloc, typename member_t>
 inline void object_copy_including_array_member_inner(const T& item, T& result, int size, member_t T::* ptm) {
-	if (vmalloc)
-		result.*ptm = (member_t)vm_malloc(sizeof(typename std::remove_pointer<member_t>::type) * (size));
+	if constexpr (is_smart_pointer_v<std::decay_t<decltype(result.*ptm)>>)
+		result.*ptm = make_shared<typename std::decay_t<decltype(result.*ptm)>::element_type[]>(size);
+	else if constexpr (vmalloc)
+		result.*ptm = static_cast<member_t>(vm_malloc(sizeof(std::remove_pointer_t<member_t>) * size));
 	else
-		result.*ptm = new typename std::remove_pointer<member_t>::type[size];
+		result.*ptm = new std::remove_pointer_t<member_t>[size];
 
 	for (int i = 0; i < size; i++)
 		(result.*ptm)[i] = (item.*ptm)[i];
@@ -225,40 +227,19 @@ T object_copy_including_array_member(const T& item, int T::* size, member_t T::*
 }
 
 template<typename T>
-int reallocate_and_copy_array(T*& array, int& size, size_t to_add) {
+int reallocate_and_copy_array(std::shared_ptr<T[]>& array, int& size, size_t to_add) {
 	//Make sure to keep old data
-	T* oldArray = array;
-
-	Assertion(size >= 0, "Tried to realloc an array of negative size %d!", size);
-	int size_before = size;
-
-	//Realloc new submodel array of proper size
-	size += static_cast<int>(to_add);
-	array = new T[MAX(0,size)];
-
-	//Copy over old data. Pointers in the struct can still point to old members, we will just delete the outer bsp_info array
-	for (int i = 0; i < size_before; i++)
-		array[i] = std::move(oldArray[i]);
-	delete[] oldArray;
-
-	return size_before;
-}
-
-template<typename T>
-int reallocate_and_copy_array_vmalloc(T*& array, int& size, size_t to_add) {
-	//Make sure to keep old data
-	T* oldArray = array;
+	std::shared_ptr<T[]> oldArray = array;
 
 	int size_before = size;
 
 	//Realloc new submodel array of proper size
 	size += static_cast<int>(to_add);
-	array = (T*)vm_malloc(sizeof(T) * size);
+	array = make_shared<T[]>(size);
 
 	//Copy over old data. Pointers in the struct can still point to old members, we will just delete the outer bsp_info array
 	for (int i = 0; i < size_before; i++)
 		array[i] = std::move(oldArray[i]);
-	vm_free(oldArray);
 
 	return size_before;
 }
@@ -753,10 +734,10 @@ VirtualPOFOperationAddWeapons::VirtualPOFOperationAddWeapons() {
 void VirtualPOFOperationAddWeapons::process(polymodel* pm, model_read_deferred_tasks& /*deferredTasks*/, model_parse_depth depth, const VirtualPOFDefinition& virtualPof) const {
 	auto appendingPM = virtual_pof_build_cache(appendingPOF, depth);
 
-	w_bank*& banks = primary ? pm->gun_banks : pm->missile_banks;
+	auto& banks = primary ? pm->gun_banks : pm->missile_banks;
 	int& n_banks = primary ? pm->n_guns : pm->n_missiles;
 
-	const w_bank* const& banks_src = primary ? appendingPM->pm()->gun_banks : appendingPM->pm()->missile_banks;
+	const auto& banks_src = primary ? appendingPM->pm()->gun_banks : appendingPM->pm()->missile_banks;
 	const int& n_banks_src = primary ? appendingPM->pm()->n_guns : appendingPM->pm()->n_missiles;
 
 	const int& n_banks_max = primary ? MAX_SHIP_PRIMARY_BANKS : MAX_SHIP_SECONDARY_BANKS;
@@ -860,9 +841,9 @@ void VirtualPOFOperationAddDockPoint::process(polymodel* pm, model_read_deferred
 		}
 	}
 
-	int destdock = reallocate_and_copy_array_vmalloc(pm->docking_bays, pm->n_docks, 1);
+	int destdock = reallocate_and_copy_array(pm->docking_bays, pm->n_docks, 1);
 	pm->docking_bays[destdock] = object_copy_including_array_member(appendingPM->docking_bays[dockpoint], &dock_bay::num_spline_paths, &dock_bay::splines);
-	int splinefrom = reallocate_and_copy_array_vmalloc(pm->paths, pm->n_paths, pm->docking_bays[destdock].num_spline_paths);
+	int splinefrom = reallocate_and_copy_array(pm->paths, pm->n_paths, pm->docking_bays[destdock].num_spline_paths);
 	
 	for (int i = 0; i < pm->docking_bays[destdock].num_spline_paths; i++) {
 		pm->paths[i + splinefrom] = object_copy_including_array_member(appendingPM->paths[appendingPM->docking_bays[dockpoint].splines[i]], &model_path::nverts, &model_path::verts);
@@ -957,7 +938,7 @@ void VirtualPOFOperationAddPath::process(polymodel* pm, model_read_deferred_task
 		return;
 	}
 
-	int destpath = reallocate_and_copy_array_vmalloc(pm->paths, pm->n_paths, 1);
+	int destpath = reallocate_and_copy_array(pm->paths, pm->n_paths, 1);
 	pm->paths[destpath] = object_copy_including_array_member(appendingPM->paths[sourcePathNr], &model_path::nverts, &model_path::verts);
 
 	if (targetParentSubsystem) {

code/network/multi.cpp

@@ -161,6 +161,8 @@ char Multi_tracker_id_string[255];
 ushort Multi_current_file_checksum = 0;
 int Multi_current_file_length = -1;
 
+bool Multi_sexp_end_mission_pending = false;
+
 
 // -------------------------------------------------------------------------------------------------
 //	multi_init() is called only once, at game start-up.  Get player address + port, initialize the
@@ -248,6 +250,9 @@ void multi_vars_init()
 	Multi_current_file_checksum = 0;
 	Multi_current_file_length = -1;
 
+	// sexp end-mission deferred debrief flag
+	Multi_sexp_end_mission_pending = false;
+
 	Active_games.clear();
 	Game_server_head = NULL;
 
@@ -1752,7 +1757,7 @@ void multi_standalone_postgame_init()
 	if ( Game_mode & GM_CAMPAIGN_MODE ) {
 		// MUST store goals and events first - may be used to evaluate next mission
 		// store goals and events
-		mission_campaign_store_goals_and_events_and_variables();
+		mission_campaign_store_goals_and_events_and_variables(false);
 
 		// evaluate next mission
 		mission_campaign_eval_next_mission();

code/network/multi.h

@@ -920,6 +920,12 @@ extern char Multi_tracker_id_string[255];
 extern ushort Multi_current_file_checksum;
 extern int Multi_current_file_length;
 
+// Set by multi_sexp_end_mission() so that process_endgame_packet() knows to call
+// send_debrief_event() immediately upon receiving MISSION_END, skipping warp-out.
+// Must be checked after stats have been broadcast by the server (which happens
+// in send_endgame_packet() before the MISSION_END packet is sent).
+extern bool Multi_sexp_end_mission_pending;
+
 // ip address list vars
 #define IP_STRING_LEN 60
 #define MAX_IP_ADDRS 100

code/network/multi_interpolate.cpp

@@ -14,6 +14,8 @@ void interpolation_manager::reassess_packet_index(vec3d* pos, matrix* ori, physi
 	int current_index = static_cast<int>(_packets.size()) - 2;
 	int prev_index = static_cast<int>(_packets.size()) - 1;
 
+	const bool index_only = ( !pos || !ori || !pip );
+
 	// iterate through the packets
 	for (; current_index > -1; current_index--, prev_index--) {
 
@@ -25,7 +27,7 @@ void interpolation_manager::reassess_packet_index(vec3d* pos, matrix* ori, physi
 			// probably the "hackiest" thing about this.  If we were just straight simulating, 
 			// and we now need to go back, pretend that the position we were in *was* our old packet
 			// and we are now going towards our new packet's physics.
-			if (_simulation_mode) {
+			if (_simulation_mode && !index_only) {
 				replace_packet(prev_index, pos, ori, pip); // TODO, if simulation mode was forced by the collision code, this method regresses a bug where collisions instantly kill
 				_simulation_mode = false;
 			}
@@ -34,9 +36,11 @@ void interpolation_manager::reassess_packet_index(vec3d* pos, matrix* ori, physi
 		}
 	}
 
-	// if we didn't find indexes then we are overwhelmingly likely to have passed the server somehow
-	// and we need to make sure that we just straight simulate these ships
-	_simulation_mode = true;
+	if ( !index_only ) {
+		// if we didn't find indexes then we are overwhelmingly likely to have passed the server somehow
+		// and we need to make sure that we just straight simulate these ships
+		_simulation_mode = true;
+	}
 }
 
 void interpolate_main_helper(int objnum, vec3d* pos, matrix* ori, physics_info* pip, vec3d* last_pos, matrix* last_orient, vec3d* gravity, bool player_ship)
@@ -112,7 +116,7 @@ void interpolation_manager::interpolate_main(vec3d* pos, matrix* ori, physics_in
 	}
 
 	// calc what the current timing should be.
-	float numerator = static_cast<float>(_packets[_upcoming_packet_index].remote_missiontime) - static_cast<float>(Multi_Timing_Info.get_current_time());
+	float numerator = static_cast<float>(Multi_Timing_Info.get_current_time()) - static_cast<float>(_packets[_prev_packet_index].remote_missiontime);
 	float denominator = static_cast<float>(_packets[_upcoming_packet_index].remote_missiontime) - static_cast<float>(_packets[_prev_packet_index].remote_missiontime);
 	
 	// work around for weird situations that might cause NAN (you just never know with multi)
@@ -162,7 +166,7 @@ void interpolation_manager::interpolate_main(vec3d* pos, matrix* ori, physics_in
 void interpolation_manager::reinterpolate_previous(TIMESTAMP stamp, int prev_packet_index, int next_packet_index,  vec3d& position, matrix& orientation, vec3d& velocity, vec3d& rotational_velocity)
 {
 	// calc what the timing was previously.
-	float numerator = static_cast<float>(_packets[next_packet_index].remote_missiontime) - static_cast<float>(stamp.value());
+	float numerator = static_cast<float>(stamp.value()) - static_cast<float>(_packets[prev_packet_index].remote_missiontime);
 	float denominator = static_cast<float>(_packets[next_packet_index].remote_missiontime) - static_cast<float>(_packets[prev_packet_index].remote_missiontime);
 
 	denominator = (denominator > 0.05f) ? denominator : 0.05f;
@@ -174,7 +178,7 @@ void interpolation_manager::reinterpolate_previous(TIMESTAMP stamp, int prev_pac
 
 	physics_snapshot temp_snapshot;
 
-	physics_interpolate_snapshots(temp_snapshot, _packets[_prev_packet_index].snapshot, _packets[_upcoming_packet_index].snapshot, scale);
+	physics_interpolate_snapshots(temp_snapshot, _packets[prev_packet_index].snapshot, _packets[next_packet_index].snapshot, scale);
 	physics_apply_snapshot_manual(position, orientation, velocity, rotational_velocity, temp_snapshot);
 }
 
@@ -213,21 +217,26 @@ void interpolation_manager::add_packet(int objnum, int frame, int packet_timesta
 				_prev_packet_index = 1;
 			}
 
-			if (static_cast<int>(_packets.size()) > PACKET_INFO_LIMIT) {
+			if (_packets.size() > PACKET_INFO_LIMIT) {
 				_packets.pop_back();
 			}
 
-			// whenenver the server gets a player packet, we need to update the ship record, since the old info is now stale
-			if (Objects[objnum].flags[Object::Object_Flags::Player_ship]){
+			if ((_upcoming_packet_index >= 0) && (_prev_packet_index >= 0)) {
+				// update packet indexes (ignoring simulation mode)
+				// NOTE: indexes must be valid before being reassesed!
+				reassess_packet_index();
 
-				int start_time = Multi_Timing_Info.get_mission_start_time();
+				// whenenver the server gets a player packet, we need to update the ship record, since the old info is now stale
+				if (Objects[objnum].flags[Object::Object_Flags::Player_ship]){
+					int start_time = Multi_Timing_Info.get_mission_start_time();
 
-				multi_ship_record_signal_update(objnum, TIMESTAMP(start_time + _packets[_prev_packet_index].remote_missiontime), TIMESTAMP(start_time + _packets[_upcoming_packet_index].remote_missiontime), _prev_packet_index, _upcoming_packet_index);
+					multi_ship_record_signal_update(objnum, TIMESTAMP(start_time + _packets[_prev_packet_index].remote_missiontime), TIMESTAMP(start_time + _packets[_upcoming_packet_index].remote_missiontime), _prev_packet_index, _upcoming_packet_index);
 
-				// if it's not the front packet, we need to update more info past the current packet, as well.  
-				// Should be rare though as it is a contingency for out of order packets.
-				if (_upcoming_packet_index != 0){
-					multi_ship_record_signal_update(objnum, TIMESTAMP(start_time + _packets[_upcoming_packet_index].remote_missiontime), TIMESTAMP(start_time + _packets[_upcoming_packet_index - 1].remote_missiontime), _upcoming_packet_index, _upcoming_packet_index - 1);
+					// if it's not the front packet, we need to update more info past the current packet, as well.
+					// Should be rare though as it is a contingency for out of order packets.
+					if (_upcoming_packet_index > 0) {
+						multi_ship_record_signal_update(objnum, TIMESTAMP(start_time + _packets[_upcoming_packet_index].remote_missiontime), TIMESTAMP(start_time + _packets[_upcoming_packet_index - 1].remote_missiontime), _upcoming_packet_index, _upcoming_packet_index - 1);
+					}
 				}
 			}
 
@@ -240,6 +249,11 @@ void interpolation_manager::add_packet(int objnum, int frame, int packet_timesta
 // should never replace index 0
 void interpolation_manager::replace_packet(int index, vec3d* pos, matrix* orient, physics_info* pip) 
 {
+	if (index < 1) {
+		UNREACHABLE("Invalid replace interpolation packet index! (%d)", index);
+		return;
+	}
+
 	// the hackiest part of the hack? Setting its frame. Let FSO think that it was basically brand new.
 	// it needs to handle it this way because otherwise another packet might get placed in front of it, 
 	// and we lose our intended effect of interpolating the simulation error away.

code/network/multi_interpolate.h

@@ -6,7 +6,7 @@
 
 struct physics_info;
 
-constexpr int PACKET_INFO_LIMIT = 4; // we should never need more than 4 packets to do interpolation.  Overwrite the oldest ones if we do.
+constexpr size_t PACKET_INFO_LIMIT = 4; // we should never need more than 4 packets to do interpolation.  Overwrite the oldest ones if we do.
 
 typedef struct packet_info {
 
@@ -42,7 +42,7 @@ private:
 	SCP_vector<packet_info> _packets;	// all the info from the position/orientation portion of packets that we care to keep
 	int _source_player_index;
 
-	void reassess_packet_index(vec3d* pos, matrix* ori, physics_info* pip);		// for finding which packets from within _packets we should use
+	void reassess_packet_index(vec3d* pos = nullptr, matrix* ori = nullptr, physics_info* pip = nullptr);		// for finding which packets from within _packets we should use
 	void replace_packet(int index, vec3d* pos, matrix* orient, physics_info* pip);	// a function that acts as a workaround, when coming out of simulation_mode
 
 	// Frame numbers that helps us figure out if we should ignore new information coming from the server because
@@ -52,6 +52,7 @@ private:
 	int _client_info_comparison_frame; 				// what frame was the last cleint info received?
 	SCP_vector<std::pair<int,int>> _subsystems_comparison_frame;	// what frame was the last subsystem information received? (for each subsystem) First is health, second is animation
 	int _ai_comparison_frame;						// what frame was the last ai information received?
+	int _support_comparison_frame;					// what frame was the last support information received?
 
 public:
 
@@ -85,6 +86,7 @@ public:
 
 
 	int get_ai_comparison_frame() const { return _ai_comparison_frame; }
+	int get_support_comparison_frame() const { return _support_comparison_frame; }
 
 	void set_hull_comparison_frame(int frame) { _hull_comparison_frame = frame; }
 	void set_shields_comparison_frame(int frame) { _shields_comparison_frame = frame; }
@@ -105,6 +107,7 @@ public:
 	}
 
 	void set_ai_comparison_frame(int frame) { _ai_comparison_frame = frame; }
+	void set_support_comparison_frame(int frame) { _support_comparison_frame = frame; }
 
 	void force_interpolation_mode() { _simulation_mode = true; }
 
@@ -134,6 +137,7 @@ public:
 		}
 
 		_ai_comparison_frame = -1;
+		_support_comparison_frame = -1;
 	}
 
 	void clean_up() 
@@ -155,6 +159,7 @@ public:
 		_shields_comparison_frame = -1;
 		_client_info_comparison_frame = -1;
 		_ai_comparison_frame = -1;
+		_support_comparison_frame = -1;
 		_source_player_index = -1;
 	}
 };

code/network/multi_obj.cpp

@@ -139,8 +139,12 @@ struct oo_general_info {
 
 oo_general_info Oo_info;
 
-// flags
-bool Afterburn_hack = false;			// HACK!!!
+// This is part of a fix for AB trails and related model animations not working
+// for other ships in multi. The "hack" part is so the fix applies to hosts as well.
+// This could possibly be revisited in a future multi bump with a better solution - taylor
+// See: https://github.com/scp-fs2open/fs2open.github.com/commit/186bd01
+// Or: "mantis bug 895 response" in SCP Internal on HLP forums
+static bool Afterburn_hack = false;			// HACK!!!
 
 // returns the last frame's index.
 int multi_find_prev_frame_idx();
@@ -904,10 +908,10 @@ void multi_ship_record_signal_update(int objnum, TIMESTAMP lower_time_limit, TIM
 		}
 	}
 
-	if (prev_index < 0 || post_index < 0) { 
-		mprintf(("Getting prev_index %d and post_index %d, which is not valid, while trying to update the ship record.\n", prev_index, post_index));
+	if ((prev_index < 0) || (prev_index == post_index)) {
 		return;
-	} else if (prev_index == post_index) {
+	} else if (post_index < 0) {
+		mprintf(("Getting prev_index %d and post_index %d, which is not valid, while trying to update the ship record.\n", prev_index, post_index));
 		return;
 	}
 
@@ -1490,16 +1494,18 @@ int multi_oo_pack_data(net_player *pl, object *objp, ushort oo_flags, ubyte *dat
 		auto submode = (short)(aip->submode);
 		ushort target_signature = 0;
 
-		// either send out the waypoint they are trying to get to *or* their current target
+		// either send out the waypoint they are trying to get to *or* their current target.
 		if (umode == AIM_WAYPOINTS) {
 			// if it's already started pointing to a waypoint, grab its net_signature and send that instead
 			waypoint* wp;
 			if ((wp = find_waypoint_at_indexes(aip->wp_list_index, aip->wp_index)) != nullptr) {
 				target_signature = Objects[wp->get_objnum()].net_signature;
 			}
-		} // send the target signature. 2021 Version!
-		else if ((aip->goals[0].target_name != nullptr) && strlen(aip->goals[0].target_name) != 0) {
-			
+		} else if (aip->target_objnum >= 0) {
+			// prefer live target_objnum so clients can check both ordered goal targets and spontaneous targets
+			target_signature = Objects[aip->target_objnum].net_signature;
+		}  else if ((aip->goals[0].target_name != nullptr) && strlen(aip->goals[0].target_name) != 0) {
+			// send the target signature. 2021 Version!
 			int instance = ship_name_lookup(aip->goals[0].target_name);
 			if (instance > -1) {
 				target_signature = Objects[Ships[instance].objnum].net_signature;
@@ -1838,10 +1844,18 @@ int multi_oo_unpack_data(net_player* pl, ubyte* data, int seq_num, int time_delt
 	// SPECIAL CLIENT INFO
 	// ---------------------------------------------------------------------------------------------------------------
 
+	// make sure the ab hack is reset before we read in new info
+	Afterburn_hack = false;
+
 	// if this is from a player, read his button info
 	if(MULTIPLAYER_MASTER){
 		int r0 = multi_oo_unpack_client_data(pl, data + offset, seq_num > Interp_info[objnum].get_client_info_comparison_frame());
 		offset += r0;
+
+		// update comparison frame
+		if (seq_num > Interp_info[objnum].get_client_info_comparison_frame()) {
+			Interp_info[objnum].set_client_info_comparison_frame(seq_num);
+		}
 	}
 
 	// ---------------------------------------------------------------------------------------------------------------
@@ -2127,15 +2141,6 @@ int multi_oo_unpack_data(net_player* pl, ubyte* data, int seq_num, int time_delt
 
 		if( seq_num > Interp_info[objnum].get_ai_comparison_frame() ){
 			if ( shipp->ai_index >= 0 ){
-				// make sure to undo the wrap if it occurred during compression for unset ai mode.
-				if (umode == 255) {
-					Ai_info[shipp->ai_index].mode = -1; 
-				}
-				else {
-					Ai_info[shipp->ai_index].mode = umode;
-				}
-				Ai_info[shipp->ai_index].submode = submode;		
-
 				// set this guy's target objnum, and other info
 				target_objp = multi_get_network_object( target_signature );
 
@@ -2182,29 +2187,30 @@ int multi_oo_unpack_data(net_player* pl, ubyte* data, int seq_num, int time_delt
 		GET_INT(ai_submode);
 		GET_USHORT(dock_sig);		
 
-		// verify that it's a valid ship							
-		if((shipp != nullptr) && (shipp->ai_index >= 0) && (shipp->ai_index < MAX_AI_INFO)){
-			// bash ai info, this info does not get rebashed, because it is not as vital.
-			Ai_info[shipp->ai_index].ai_flags.from_u64(ai_flags);
-			Ai_info[shipp->ai_index].mode = ai_mode;
-			Ai_info[shipp->ai_index].submode = ai_submode;
+		if (seq_num > Interp_info[objnum].get_support_comparison_frame()) {
+			// verify that it's a valid ship
+			if((shipp != nullptr) && (shipp->ai_index >= 0) && (shipp->ai_index < MAX_AI_INFO)){
+				// bash ai info, this info does not get rebashed, because it is not as vital.
+				Ai_info[shipp->ai_index].ai_flags.from_u64(ai_flags);
+				Ai_info[shipp->ai_index].mode = ai_mode;
+				Ai_info[shipp->ai_index].submode = ai_submode;
 
-			object *objp = multi_get_network_object( dock_sig );
-			if(objp != nullptr){
-				Ai_info[shipp->ai_index].support_ship_objnum = OBJ_INDEX(objp);
-				if ((objp->instance > -1) && (objp->type == OBJ_SHIP)) {
-					Ai_info[shipp->ai_index].goals[0].target_name = Ships[objp->instance].ship_name;
-					Ai_info[shipp->ai_index].goals[0].target_signature = objp->signature;
-				} else {
-					Ai_info[shipp->ai_index].goals[0].target_name = nullptr;
-					Ai_info[shipp->ai_index].goals[0].target_signature = 0;
+				object *objp = multi_get_network_object( dock_sig );
+				if(objp != nullptr){
+					Ai_info[shipp->ai_index].support_ship_objnum = OBJ_INDEX(objp);
+					if ((objp->instance > -1) && (objp->type == OBJ_SHIP)) {
+						Ai_info[shipp->ai_index].goals[0].target_name = Ships[objp->instance].ship_name;
+						Ai_info[shipp->ai_index].goals[0].target_signature = objp->signature;
+					} else {
+						Ai_info[shipp->ai_index].goals[0].target_name = nullptr;
+						Ai_info[shipp->ai_index].goals[0].target_signature = 0;
+					}
 				}
 			}
-		}			
-	} 
 
-	// make sure the ab hack is reset before we read in new info
-	Afterburn_hack = false;
+			Interp_info[objnum].set_support_comparison_frame(seq_num);
+		}
+	}
 
 	// afterburner info
 	if ( (oo_flags & OO_AFTERBURNER_NEW) || Afterburn_hack ) {

code/network/multi_options.cpp

@@ -314,6 +314,19 @@ void multi_options_read_config()
 							Multi_options_g.webapiPort = (ushort) result;
 						}
 					}
+				} else
+				// set framerate for standalone
+				if ( SETTING("+fps_cap") ) {
+					NEXT_TOKEN();
+					if (tok != nullptr) {
+						if ((atoi(tok) >= 15) && (atoi(tok) <= 120)) {
+							Multi_options_g.std_framecap = atoi(tok);
+						}
+					}
+				} else
+				// disable rollback for primary/dumbfire weapons
+				if ( SETTING("+rollback_disabled") ) {
+					Multi_options_g.std_rollback = false;
 				}
 			}
 

code/network/multi_options.h

@@ -50,6 +50,7 @@ typedef struct multi_global_options {
 	char		std_passwd[STD_PASSWD_LEN+1];							// standalone host password
 	char		std_pname[STD_NAME_LEN+1];								// permanent name for the standalone - if any
 	int		std_framecap;												// standalone frame cap
+	bool	std_rollback;												// use rollback for primary/dumbfire shots
 
 	ushort		webapiPort;
 	SCP_string	webapiUsername;
@@ -79,6 +80,7 @@ typedef struct multi_global_options {
 		memset(std_passwd, 0, STD_PASSWD_LEN+1);
 		memset(std_pname, 0, STD_NAME_LEN+1);
 		std_framecap = 30;
+		std_rollback = true;
 
 		webapiPort = 8080;
 		webapiUsername = "admin";

code/network/multi_portfwd.cpp

@@ -98,6 +98,7 @@ void multi_port_forward_init()
 
 void multi_port_forward_do()
 {
+	static bool failed_before = false;
 	pcp_fstate_e state;
 	size_t info_count = 0;
 	int wait_ms;
@@ -117,7 +118,10 @@ void multi_port_forward_do()
 	// check progress and log if needed
 	if ( pcp_eval_flow_state(PF_pcp_flow, &state) ) {
 		if (state == pcp_state_failed) {
-			ml_string("Port forward => Mapping failed!");
+			if ( !failed_before ) {
+				ml_string("Port forward => Mapping failed!");
+				failed_before = true;	// reduce log spam
+			}
 		} else if (state == pcp_state_succeeded) {
 			pcp_flow_info_t *flow_info = pcp_flow_get_info(PF_pcp_flow, &info_count);
 
@@ -143,6 +147,8 @@ void multi_port_forward_do()
 				ml_printf("Port forward => Mapping valid until %s", time_str);
 			}
 
+			failed_before = false;
+
 			if (flow_info) {
 				free(flow_info);	// system call, allocated in lib  ** don't change! **
 			}
@@ -178,7 +184,7 @@ void multi_port_forward_close()
 #ifndef NDEBUG
 static void PF_logger_fn(pcp_loglvl_e /* lvl */, const char *msg)
 {
-	nprintf(("portfwd", "Port forward => %s", msg));
+	nprintf(("portfwd", "Port forward => %s\n", msg));
 }
 #endif
 

code/network/multimsgs.cpp

@@ -4046,9 +4046,17 @@ void process_endgame_packet(ubyte * /*data*/, header *hinfo)
 		// do any special processing for being in a state other than the gameplay state
 		multi_handle_state_special();
 
+		// If end-mission fired via SEXP, enter debrief immediately (skipping warp-out).
+		// Stats have already been received via multi_broadcast_stats() which send_endgame_packet()
+		// calls before sending this packet, so scoring_level_close() will see correct kill data.
+		if (Multi_sexp_end_mission_pending) {
+			Multi_sexp_end_mission_pending = false;
+			send_debrief_event();
+		}
+
 		// make sure we're not already in the debrief state
 		if((gameseq_get_state() != GS_STATE_DEBRIEF) && (gameseq_get_state() != GS_STATE_MULTI_DOGFIGHT_DEBRIEF)){
-			multi_warpout_all_players();			
+			multi_warpout_all_players();
 		}
 	}	
 }
@@ -7837,7 +7845,7 @@ void send_non_homing_fired_packet(ship* shipp, int banks_or_number_of_missiles_f
 
 	// We need the time elpased, so send the last frame we got from the server and how much time has happened since then.
 	int last_received_frame = multi_client_lookup_frame_idx();
-	auto time_elapsed = static_cast<ushort>(timestamp_since(multi_client_lookup_frame_timestamp()));
+	auto time_elapsed = static_cast<ushort>(Multi_Timing_Info.get_current_time() - multi_client_lookup_frame_timestamp());
 
 	ADD_INT(last_received_frame);
 	ADD_USHORT(time_elapsed);
@@ -7931,7 +7939,7 @@ void process_non_homing_fired_packet(ubyte* data, header* hinfo)
 
 	object* objp_ref = multi_get_network_object(target_ref);
 
-	if (objp_ref == nullptr || objp_ref->type != OBJ_SHIP) {
+	if ((Is_standalone && !Multi_options_g.std_rollback) || !objp_ref || (objp_ref->type != OBJ_SHIP)) {
 		// new way failed, use the old new way.
 
 		if (objp_ref != nullptr){

code/network/multiui.cpp

@@ -1803,7 +1803,7 @@ void multi_join_cull_timeouts()
 
 	// traverse through the entire list if any items exist
 	int i = 0;
-	for (auto game = Active_games.begin(); game != Active_games.end(); ++game) {
+	for (auto game = Active_games.begin(); game != Active_games.end(); ) {
 		if (game->heard_from_timer.isValid() && (ui_timestamp_elapsed(game->heard_from_timer))) {
 
 			// handle any gui details related to deleting this item
@@ -1815,9 +1815,10 @@ void multi_join_cull_timeouts()
 			}
 
 			// delete the item
-			Active_games.erase(game);
+			game = Active_games.erase(game);
 		} else {
 			++i;
+			++game;
 		}
 	}
 }
@@ -7820,7 +7821,7 @@ void multi_sync_blit_screen_all()
 			// display his name and status
 			multi_sync_display_name(Net_players[idx].m_player->callsign,count,idx);
 
-			multi_sync_display_status(multi_sync_get_state_string(&Net_player[idx]).c_str(), count);
+			multi_sync_display_status(multi_sync_get_state_string(&Net_players[idx]).c_str(), count);
 			count++;
 		}
 	}	
@@ -8836,6 +8837,7 @@ void multi_debrief_esc_hit()
 						multi_sw_report(stats_saved);
 					}
 				}
+
 			}
 
 			multi_quit_game(PROMPT_HOST);

code/network/stand_gui-unix.cpp

@@ -457,7 +457,11 @@ json_t* playerGet(ResourceContext * /*context*/) {
         json_object_set(obj, "host", (MULTI_HOST(p)) ? json_true() : json_false());
         json_object_set(obj, "observer", (MULTI_OBSERVER(p)) ? json_true() : json_false());
         json_object_set(obj, "callsign", json_string(p.m_player->callsign));
-        json_object_set(obj, "ship", json_string(Ship_info[p.p_info.ship_class].name));
+        if (p.p_info.ship_class >= 0 && p.p_info.ship_class < ship_info_size()) {
+            json_object_set(obj, "ship", json_string(Ship_info[p.p_info.ship_class].name));
+        } else {
+            json_object_set(obj, "ship", json_string(""));
+        }
 
         json_array_append(playerList, obj);
     }

code/network/stand_gui.cpp

@@ -1067,7 +1067,7 @@ void std_pinfo_display_player_info(net_player *p)
 	txt[sizeof(txt)-1] = '\0';
 
 	// set his ship type -- Cyborg17, if it's valid!
-	if (p->p_info.ship_class >= 0 && p->p_info.ship_class < static_cast<int>(Ship_info.size())) {
+	if (p->p_info.ship_class >= 0 && p->p_info.ship_class < ship_info_size()) {
 		SetWindowText(Player_ship_type, Ship_info[p->p_info.ship_class].name);
 	}
 

code/object/collideshipship.cpp

@@ -259,9 +259,9 @@ int ship_ship_check_collision(collision_info_struct *ship_ship_hit_info)
 			model_get_moving_submodel_list(submodel_vector, heavy_obj);
 
 			// turn off all moving submodels, collide against only 1 at a time.
-			// turn off collision detection for all moving submodels
+			mc.collision_checked.assign(pm->n_models, 0);
 			for (auto submodel : submodel_vector) {
-				pmi->submodel[submodel].collision_checked = true;
+				mc.collision_checked[submodel] = true;
 			}
 
 			// Only check single submodel now, since children of moving submodels are handled as moving as well
@@ -273,14 +273,12 @@ int ship_ship_check_collision(collision_info_struct *ship_ship_hit_info)
 
 			// check each submodel in turn
 			for (auto submodel : submodel_vector) {
-				auto smi = &pmi->submodel[submodel];
-
 				// turn on just one submodel for collision test
-				smi->collision_checked = false;
+				mc.collision_checked[submodel] = false;
 
-				if (smi->blown_off)
+				if (pmi->submodel[submodel].blown_off)
 				{
-					smi->collision_checked = true;
+					mc.collision_checked[submodel] = true;
 					continue;
 				}
 
@@ -314,8 +312,13 @@ int ship_ship_check_collision(collision_info_struct *ship_ship_hit_info)
 						model_instance_local_to_global_point(&ship_ship_hit_info->light_collision_cm_pos, &int_light_pos, pm, pmi, mc.hit_submodel, &heavy_obj->orient, &zero);
 					}
 				}
+
+				// re-disable this submodel before enabling the next one
+				mc.collision_checked[submodel] = true;
 			}
 
+			// Clear collision_checked before subsequent model_collide calls so it auto-inits fresh
+			mc.collision_checked.clear();
 		}
 
 		// Now complete base model collision checks that do not take into account rotating submodels.

code/object/collideshipweapon.cpp

@@ -28,8 +28,16 @@
 #include "ship/shiphit.h"
 #include "weapon/weapon.h"
 
-//mc, notify_ai_shield_down, shield_collision, quadrant_num, shield_tri_hit, shield_hitpoint
-using ship_weapon_collision_data = std::tuple<std::optional<mc_info>, int, bool, int, int, vec3d>;
+struct ship_weapon_collision_data {
+	std::optional<mc_info> mc;
+	int notify_ai_shield_down;
+	bool shield_collision;
+	int quadrant_num;
+	int shield_tri_hit;
+	vec3d shield_hitpos;
+	bool should_update_danger_weapon;	// deferred to main thread for thread safety
+	bool should_detonate;				// deferred to main thread for thread safety
+};
 
 extern int Game_skill_level;
 extern float ai_endangered_time(const object *ship_objp, const object *weapon_objp);
@@ -205,14 +213,14 @@ static std::tuple<bool, bool, ship_weapon_collision_data> ship_weapon_check_coll
 	Assert( shipp->objnum == OBJ_INDEX(ship_objp));
 
 	// Make ships that are warping in not get collision detection done
-	if ( shipp->is_arriving() ) return {false, true, {std::nullopt, -1, false, -1, -1, ZERO_VECTOR}};
+	if ( shipp->is_arriving() ) return {false, true, {std::nullopt, -1, false, -1, -1, ZERO_VECTOR, false, false}};
 	
 	//	Return information for AI to detect incoming fire.
 	//	Could perhaps be done elsewhere at lower cost --MK, 11/7/97
 	float	dist = vm_vec_dist_quick(&ship_objp->pos, &weapon_objp->pos);
-	if (dist < weapon_objp->phys_info.speed) {
-		update_danger_weapon(ship_objp, weapon_objp);
-	}
+	bool should_update_danger_weapon = (dist < weapon_objp->phys_info.speed);
+	bool should_detonate = false;
+	bool postproc = should_update_danger_weapon;
 
 	int	valid_hit_occurred = 0;				// If this is set, then hitpos is set
 	int	quadrant_num = -1;
@@ -502,12 +510,13 @@ static std::tuple<bool, bool, ship_weapon_collision_data> ship_weapon_check_coll
 		*next_hit = (int) (1000.0f * (mc->hit_dist*(flFrametime + time_limit) - flFrametime) );
 		if (*next_hit > 0)
 			// if hit occurs outside of this frame, do not do damage
-			return { false, false, {std::nullopt, -1, false, -1, -1, ZERO_VECTOR} }; //No hit, but continue checking
+			return { postproc, false, {std::nullopt, -1, false, -1, -1, ZERO_VECTOR, should_update_danger_weapon, false} }; //No hit, but continue checking
 	}
 
-	bool postproc = valid_hit_occurred || notify_ai_shield_down >= 0;
+	postproc = postproc || valid_hit_occurred || notify_ai_shield_down >= 0;
 	ship_weapon_collision_data collision_data {
-		valid_hit_occurred ? std::optional(*mc) : std::nullopt, notify_ai_shield_down, postproc, quadrant_num, shield_tri_hit, shield_hitpos
+		valid_hit_occurred ? std::optional(*mc) : std::nullopt, notify_ai_shield_down, postproc, quadrant_num, shield_tri_hit, shield_hitpos,
+		should_update_danger_weapon, false
 	};
 
 	// when the $Fixed Missile Detonation: flag is active, skip this whole block, as it's redundant to a similar check in weapon_home()
@@ -520,17 +529,16 @@ static std::tuple<bool, bool, ship_weapon_collision_data> ship_weapon_check_coll
 			if (vm_vec_dot(&vec_to_ship, &weapon_objp->orient.vec.fvec) < 0.0f) {
 				// check if we're colliding against "invisible" ship
 				if (!(shipp->flags[Ship::Ship_Flags::Dont_collide_invis])) {
-					wp->lifeleft = 0.001f;
-					wp->weapon_flags.set(Weapon::Weapon_Flags::Begun_detonation);
-
-					if (ship_objp == Player_obj)
-						nprintf(("Jim", "Frame %i: Weapon %d set to detonate, dist = %7.3f.\n", Framecount, OBJ_INDEX(weapon_objp), dist));
+					// Detonation writes are deferred to the main thread for thread safety
+					should_detonate = true;
+					postproc = true;
 					valid_hit_occurred = 1; //No hit, continue checking
 				}
 			}
 		}
 	}
 
+	collision_data.should_detonate = should_detonate;
 	return { postproc, !static_cast<bool>(valid_hit_occurred), collision_data} ;
 }
 
@@ -542,15 +550,26 @@ static void ship_weapon_process_collision(obj_pair* pair, const ship_weapon_coll
 	const weapon_info* wip = &Weapon_info[wp->weapon_info_index];
 	const ship_info* sip = &Ship_info[shipp->ship_info_index];
 
-	const auto& [mc_opt, notify_ai_shield_down, shield_collision, quadrant_num, shield_tri_hit, shield_hitpos] = collision_data;
-	bool valid_hit_occurred = mc_opt.has_value();
-	auto mc = valid_hit_occurred ? &(*mc_opt) : nullptr;
+	bool valid_hit_occurred = collision_data.mc.has_value();
+	auto mc = valid_hit_occurred ? &(*collision_data.mc) : nullptr;
 
-	if (notify_ai_shield_down >= 0)
-		Ai_info[Ships[ship_objp->instance].ai_index].danger_shield_quadrant = notify_ai_shield_down;
+	// Perform deferred writes that are unsafe to do from worker threads
+	if (collision_data.should_update_danger_weapon)
+		update_danger_weapon(ship_objp, weapon_objp);
 
-	if (shield_tri_hit >= 0)
-		add_shield_point(OBJ_INDEX(ship_objp), shield_tri_hit, &shield_hitpos, wip->shield_impact_effect_radius);
+	if (collision_data.should_detonate) {
+		wp->lifeleft = 0.001f;
+		wp->weapon_flags.set(Weapon::Weapon_Flags::Begun_detonation);
+
+		if (ship_objp == Player_obj)
+			nprintf(("Jim", "Frame %i: Weapon %d set to detonate, dist = %7.3f.\n", Framecount, OBJ_INDEX(weapon_objp), vm_vec_dist_quick(&ship_objp->pos, &weapon_objp->pos)));
+	}
+
+	if (collision_data.notify_ai_shield_down >= 0)
+		Ai_info[Ships[ship_objp->instance].ai_index].danger_shield_quadrant = collision_data.notify_ai_shield_down;
+
+	if (collision_data.shield_tri_hit >= 0)
+		add_shield_point(OBJ_INDEX(ship_objp), collision_data.shield_tri_hit, &collision_data.shield_hitpos, wip->shield_impact_effect_radius);
 
 	if ( valid_hit_occurred )
 	{
@@ -582,12 +601,12 @@ static void ship_weapon_process_collision(obj_pair* pair, const ship_weapon_coll
 		}
 
 		if(!ship_override && !weapon_override) {
-			if (shield_collision && quadrant_num >= 0) {
+			if (collision_data.shield_collision && collision_data.quadrant_num >= 0) {
 				if ((sip->shield_impact_explosion_anim.isValid()) && (wip->shield_impact_explosion_radius > 0)) {
 					shield_impact_explosion(mc->hit_point, mc->hit_normal, ship_objp, weapon_objp, wip->shield_impact_explosion_radius, sip->shield_impact_explosion_anim);
 				}
 			}
-			ship_weapon_do_hit_stuff(ship_objp, weapon_objp, &mc->hit_point_world, &mc->hit_point, quadrant_num, mc->hit_submodel, &mc->hit_normal);
+			ship_weapon_do_hit_stuff(ship_objp, weapon_objp, &mc->hit_point_world, &mc->hit_point, collision_data.quadrant_num, mc->hit_submodel, &mc->hit_normal);
 		}
 
 		if (scripting::hooks::OnWeaponCollision->isActive() && !(weapon_override && !ship_override)) {

code/object/objcollide.cpp

@@ -762,6 +762,7 @@ void spin_up_mp_collision() {
 void spin_down_mp_collision() {
 	threading::spin_down_threaded_task();
 	collision_processing_done.store(true);
+	threading::spin_down_wait_complete();
 }
 
 void queue_mp_collision(uint ctype, const obj_pair& colliding) {
@@ -1169,11 +1170,11 @@ void collide_mp_worker_thread(size_t threadIdx) {
 						UNREACHABLE("Got non MP-compatible collision type!");
 				}
 
-				auto&& [check_again, collision_data_maybe, collision_fnc] = check_collision(&collision_check.objs);
+				auto&& [never_check_again, collision_data_maybe, collision_fnc] = check_collision(&collision_check.objs);
 
 				{
 					std::scoped_lock lock{thread.result_mutex};
-					thread.queue_results->emplace_back(collision_thread_data::collision_queue_result{collision_check.objs, check_again, collision_data_maybe, collision_fnc});
+					thread.queue_results->emplace_back(collision_thread_data::collision_queue_result{collision_check.objs, never_check_again, collision_data_maybe, collision_fnc});
 				}
 				thread.result_length.fetch_add(1, std::memory_order_release);
 				thread.queue_length.fetch_sub(1, std::memory_order_release);

code/parse/sexp.cpp

@@ -17152,10 +17152,18 @@ void sexp_end_mission(int n)
 
 void multi_sexp_end_mission()
 {
-	// This is a bit of hack, but when in a debrief state clients will skip the
-	// warp out sequence when the endgame packet is processed.
-	send_debrief_event();
-	// Standard way to end mission (equivalent to Alt-J)
+	// Signal process_endgame_packet() to skip the warp-out sequence by entering
+	// debrief immediately when the MISSION_END packet arrives.  We cannot call
+	// send_debrief_event() here directly: on standalone servers the client has not
+	// yet received mission stats (m_okKills etc.) at this point, so
+	// scoring_level_close() would run with zeroed kill counts and
+	// Pilot.update_stats() would create no kill entries.  debrief_close() would
+	// then try to back out entries that were never created and hit an UNREACHABLE.
+	//
+	// send_endgame_packet() broadcasts stats to clients *before* sending MISSION_END,
+	// so by the time process_endgame_packet() fires, the stats are already present
+	// and it is safe to enter debrief there.
+	Multi_sexp_end_mission_pending = true;
 	multi_handle_end_mission_request();
 }
 
@@ -36138,22 +36146,6 @@ int sexp_variable_count()
 	return count;
 }
 
-/**
- * Count number of persistent sexp_variables that are set
- */
-int sexp_campaign_file_variable_count()
-{
-	int count = 0;
-
-	for (int i=0; i<MAX_SEXP_VARIABLES; i++) {
-		if ( (Sexp_variables[i].type & SEXP_VARIABLE_SET) && (Sexp_variables[i].type & SEXP_VARIABLE_IS_PERSISTENT) && !(Sexp_variables[i].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE) ) {
-			count++;
-		}
-	}
-
-	return count;
-}
-
 /**
  * Given an index in Sexp_variables, returns the number variables of a type in the array until this point
  */

code/parse/sexp.h

@@ -1102,7 +1102,7 @@ enum class sexp_mode
 #define SEXP_ATOM				2
 
 // flags for sexpressions -- masked onto the end of the type field
-#define SEXP_FLAG_PERSISTENT				(1<<31)		// should this sexp node be persistant across missions
+#define SEXP_FLAG_PERSISTENT				(1<<31)		// should this sexp node be kept across missions, i.e. not freed -- note, NOT the same as variable/container persistence
 #define SEXP_FLAG_VARIABLE					(1<<30)
 
 // sexp variable definitions
@@ -1126,6 +1126,18 @@ enum class sexp_mode
 #define SEXP_VARIABLE_NETWORK				(1<<28)
 #define SEXP_VARIABLE_SAVE_TO_PLAYER_FILE	(1<<27)
 
+// There are three types of persistence for variables and containers:
+// 1. No persistence: the value is only kept for the duration of a mission's gameplay
+// 2. Campaign-persistence: the value is scoped to a campaign, and has no value outside the campaign
+// 3. Player-persistence: the value is scoped to the player/pilot file, and can be referenced in any campaign
+// And there are two ways that persistent variables/containers are saved:
+// 1. When the mission progresses with an outcome that is "accepted" by the player
+// 2. When the mission closes in any way (progress, quit, restart)
+// So, there can be four combinations of persistence (campaign/player times progress/close).  When persistent variables
+// were first implemented, there was an assumption that player-persistence implied save-on-close, and that
+// campaign-persistence implied save-on-progress, and the original mission parsing code reflects that.  But after the
+// 2018 rework, either type can be used with either save.  The 2018 rework also introduced new terminology:
+// "eternal" means player-persistent, and "non-eternal" means campaign-persistent.
 #define SEXP_VARIABLE_IS_PERSISTENT (SEXP_VARIABLE_SAVE_ON_MISSION_PROGRESS|SEXP_VARIABLE_SAVE_ON_MISSION_CLOSE)
 
 #define BLOCK_EXP_SIZE					6
@@ -1508,7 +1520,6 @@ bool sexp_replace_variable_names_with_values(char *text, int max_len);	// Goober
 bool sexp_replace_variable_names_with_values(SCP_string &text);	// Goober5000
 int get_nth_variable_index(int nth, int variable_type);	// Karajorma
 int sexp_variable_count();
-int sexp_campaign_file_variable_count();	// Goober5000
 int sexp_variable_typed_count(int sexp_variables_index, int variable_type); // Karajorma
 void sexp_variable_delete(int index);
 void sexp_variable_sort();

code/parse/sexp_container.cpp

@@ -150,7 +150,7 @@ ContainerType sexp_container::get_data_type() const
 
 bool sexp_container::name_matches(const sexp_container &container) const
 {
-	return !stricmp(container.container_name.c_str(), container_name.c_str());
+	return lcase_equal(container.container_name, container_name);
 }
 
 bool sexp_container::empty() const
@@ -812,18 +812,6 @@ bool sexp_container_CTEXT_helper(int &node, sexp_container &container, SCP_strin
 	}
 }
 
-// inspired by sexp_campaign_file_variable_count()
-bool sexp_container_has_persistent_non_eternal_containers()
-{
-	for (const auto &container : Sexp_containers) {
-		if (container.is_persistent() && !container.is_eternal()) {
-			return true;
-		}
-	}
-
-	return false;
-}
-
 const char *sexp_container_CTEXT(int node)
 {
 	auto *p_container = get_sexp_container(Sexp_nodes[node].text);

code/parse/sexp_container.h

@@ -209,9 +209,6 @@ bool sexp_container_replace_refs_with_values(char *text, size_t max_len);
 
 const char *sexp_container_CTEXT(int node);
 
-// persistence
-bool sexp_container_has_persistent_non_eternal_containers();
-
 // SEXPs
 int sexp_container_eval_status_sexp(int op_num, int node);
 int sexp_container_eval_change_sexp(int op_num, int node);

code/particle/ParticleSource.cpp

@@ -32,6 +32,10 @@ void ParticleSource::finishCreation() {
 	if (Is_standalone)
 		return;
 
+	Assertion(isValid(), "Cannot create a ParticleSource that is not valid!");
+	if (!isValid())
+		return;
+
 	m_host->setupProcessing();
 
 	for (const auto& effect : ParticleManager::get()->getEffect(m_effect)) {

code/particle/hosts/EffectHostTurret.cpp

@@ -4,8 +4,8 @@
 #include "model/model.h"
 #include "ship/ship.h"
 
-EffectHostTurret::EffectHostTurret(object* objp, int submodel, int fire_pnt, matrix orientationOverride, bool orientationOverrideRelative) :
-	EffectHost(orientationOverride, orientationOverrideRelative), m_objnum(OBJ_INDEX(objp)), m_objsig(objp->signature), m_submodel(submodel), m_fire_pnt(fire_pnt) {}
+EffectHostTurret::EffectHostTurret(const object* objp, int submodel, int fire_pnt, bool is_fighterbeam, matrix orientationOverride, bool orientationOverrideRelative) :
+	EffectHost(orientationOverride, orientationOverrideRelative), m_objnum(OBJ_INDEX(objp)), m_objsig(objp ? objp->signature : -1), m_submodel(submodel), m_fire_pnt(fire_pnt), m_is_fighterbeam(is_fighterbeam) {}
 
 std::pair<vec3d, matrix> EffectHostTurret::getPositionAndOrientation(bool relativeToParent, float interp, const std::optional<vec3d>& tabled_offset) const {
 	ship& shipp = Ships[Objects[m_objnum].instance];
@@ -13,7 +13,14 @@ std::pair<vec3d, matrix> EffectHostTurret::getPositionAndOrientation(bool relati
 	const polymodel_instance* pmi = model_get_instance(shipp.model_instance_num);
 
 	vec3d gvec, pos;
-	model_subsystem *tp = ship_get_indexed_subsys(&shipp, pm->submodel[m_submodel].subsys_num)->system_info;
+	model_subsystem* tp;
+	if (m_is_fighterbeam) {
+		tp = &shipp.beam_sys_info;
+	} else {
+		auto subsys = ship_get_subsys_for_submodel(&shipp, m_submodel);
+		Assertion(subsys != nullptr, "EffectHostTurret constructed with missing subsystem!");
+		tp = subsys->system_info;
+	}
 	vec3d* gun_pos = &tp->turret_firing_point[m_fire_pnt % tp->turret_num_firing_points];
 	const matrix& gun_frame_of_reference = pm->submodel[m_submodel].frame_of_reference;
 
@@ -72,6 +79,5 @@ float EffectHostTurret::getHostRadius() const {
 }
 
 bool EffectHostTurret::isValid() const {
-	return m_objnum >= 0 && m_submodel >= 0 && Objects[m_objnum].signature == m_objsig && Objects[m_objnum].type == OBJ_SHIP &&
-			model_get(Ship_info[Ships[Objects[m_objnum].instance].ship_info_index].model_num)->submodel[m_submodel].subsys_num >= 0;
-}
+	return m_objnum >= 0 && m_submodel >= 0 && Objects[m_objnum].signature == m_objsig && Objects[m_objnum].type == OBJ_SHIP;
+}

code/particle/hosts/EffectHostTurret.h

@@ -5,9 +5,10 @@
 
 class EffectHostTurret : public EffectHost {
 	int m_objnum, m_objsig, m_submodel, m_fire_pnt;
+	bool m_is_fighterbeam;
 
 public:
-	EffectHostTurret(object* objp, int submodel, int fire_pnt, matrix orientationOverride = vmd_identity_matrix, bool orientationOverrideRelative = true);
+	EffectHostTurret(const object* objp, int submodel, int fire_pnt, bool is_fighterbeam, matrix orientationOverride = vmd_identity_matrix, bool orientationOverrideRelative = true);
 
 	std::pair<vec3d, matrix> getPositionAndOrientation(bool relativeToParent, float interp, const std::optional<vec3d>& tabled_offset) const override;
 
@@ -18,4 +19,4 @@ public:
 	float getHostRadius() const override;
 
 	bool isValid() const override;
-};
+};

code/pilotfile/csg.cpp

@@ -1183,7 +1183,13 @@ void pilotfile::csg_read_variables()
 			temp_var.type = cfread_int(cfp);
 			cfread_string_len(temp_var.text, TOKEN_LENGTH, cfp);
 			cfread_string_len(temp_var.variable_name, TOKEN_LENGTH, cfp);
-			Campaign.persistent_variables.push_back(temp_var);
+			Campaign.persistent_variables.push_back(std::move(temp_var));
+
+			// eternal variables belong in the player file
+			Assert(!(temp_var.type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE));
+			if (temp_var.type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE) {
+				Campaign.persistent_variables.pop_back();
+			}
 		}
 	}
 
@@ -1202,7 +1208,7 @@ void pilotfile::csg_read_variables()
 				temp_var.type = cfread_int(cfp);
 				cfread_string_len(temp_var.text, TOKEN_LENGTH, cfp);
 				cfread_string_len(temp_var.variable_name, TOKEN_LENGTH, cfp);
-				Campaign.red_alert_variables.push_back(temp_var);
+				Campaign.red_alert_variables.push_back(std::move(temp_var));
 			}
 		}
 	}
@@ -1217,11 +1223,9 @@ void pilotfile::csg_write_variables()
 	cfwrite_int((int)Campaign.persistent_variables.size(), cfp);
 
 	for (idx = 0; idx < (int)Campaign.persistent_variables.size(); idx++) {
-		if (!(Campaign.persistent_variables[idx].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE)) {
-			cfwrite_int(Campaign.persistent_variables[idx].type, cfp);
-			cfwrite_string_len(Campaign.persistent_variables[idx].text, cfp);
-			cfwrite_string_len(Campaign.persistent_variables[idx].variable_name, cfp);
-		}
+		cfwrite_int(Campaign.persistent_variables[idx].type, cfp);
+		cfwrite_string_len(Campaign.persistent_variables[idx].text, cfp);
+		cfwrite_string_len(Campaign.persistent_variables[idx].variable_name, cfp);
 	}
 
 	cfwrite_int((int)Campaign.red_alert_variables.size(), cfp);
@@ -1499,6 +1503,12 @@ void pilotfile::csg_read_containers()
 		Campaign.persistent_containers.emplace_back();
 		auto& container = Campaign.persistent_containers.back();
 		csg_read_container(container);
+
+		// eternal containers belong in the player file
+		Assert(!container.is_eternal());
+		if (container.is_eternal()) {
+			Campaign.persistent_containers.pop_back();
+		}
 	}
 
 	Campaign.red_alert_containers.clear();
@@ -1551,7 +1561,6 @@ void pilotfile::csg_write_containers()
 	cfwrite_int((int)Campaign.persistent_containers.size(), cfp);
 
 	for (const auto& container : Campaign.persistent_containers) {
-		Assert(!container.is_eternal()); // eternal containers should be written to player file
 		csg_write_container(container);
 	}
 

code/pilotfile/plr.cpp

@@ -356,7 +356,10 @@ void pilotfile::plr_read_variables()
 		handler->readString("text", n_var.text, TOKEN_LENGTH);
 		handler->readString("variable_name", n_var.variable_name, TOKEN_LENGTH);
 
-		p->variables.push_back( n_var );
+		Assert(n_var.type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE);
+		if (n_var.type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE) {
+			p->variables.push_back(std::move(n_var));
+		}
 	}
 	handler->endArrayRead();
 }
@@ -413,6 +416,11 @@ void pilotfile::plr_read_containers()
 		} else {
 			UNREACHABLE("Unknown container type %d", (int)container.type);
 		}
+
+		Assert(container.is_eternal());
+		if (!container.is_eternal()) {
+			p->containers.pop_back();
+		}
 	}
 	handler->endArrayRead();
 }

code/scripting/api/libs/base.cpp

@@ -596,6 +596,14 @@ ADE_FUNC(inDebug, l_Base, nullptr, "Determines if FSO is running in Release or D
 	#endif
 }
 
+ADE_FUNC(inStandalone, l_Base, nullptr, "Determines if FSO is running in Standalone mode or not", "boolean", "true if standalone, false otherwise") {
+	if (Is_standalone) {
+		return ADE_RETURN_TRUE;
+	} else {
+		return ADE_RETURN_FALSE;
+	}
+}
+
 ADE_FUNC(isEngineVersionAtLeast,
 		 l_Base,
 		 "number major, number minor, number build, [number revision = 0]",

code/scripting/api/libs/graphics.cpp

@@ -1319,6 +1319,7 @@ ADE_FUNC(drawOffscreenIndicator, l_Graphics, "object Object, [boolean draw=true,
 			int dir;
 			float tri_separation;
 
+			offscreengauge->resetClip();
 			offscreengauge->calculatePosition(&target_point, &targetp->pos, &outpoint, &dir, &tri_separation);
 
 			if (draw) {
@@ -1662,7 +1663,7 @@ ADE_FUNC(createTexture, l_Graphics, "[number Width=512, number Height=512, enume
 			t = BMP_FLAG_RENDER_TARGET_DYNAMIC;
 	}
 
-	int idx = bm_make_render_target(w, h, t);
+	int idx = bm_make_render_target(w, h, t | BMP_FLAG_RENDER_TARGET_DEPTH_ATTACHMENT);
 
 	if(idx < 0)
 		return ade_set_error(L, "o", l_Texture.Set(texture_h()));

code/scripting/api/libs/ui.cpp

@@ -804,7 +804,7 @@ ADE_FUNC(skipTraining,
 	// tricky part.  Need to move to the next mission in the campaign.
 	mission_goal_mark_objectives_complete();
 	mission_goal_fail_incomplete();
-	mission_campaign_store_goals_and_events_and_variables();
+	mission_campaign_store_goals_and_events_and_variables(false);
 
 	mission_campaign_eval_next_mission();
 	mission_campaign_mission_over();

code/scripting/api/objs/particle.cpp

@@ -7,6 +7,7 @@
 #include "model.h"
 #include "particle/ParticleManager.h"
 #include "particle/ParticleEffect.h"
+#include "ship/ship.h"
 
 namespace scripting {
 namespace api {
@@ -397,11 +398,20 @@ ADE_FUNC(createOnTurret, l_ParticleSource, "object object, submodel submodel, nu
 	if (!(subobjh.isValid() && objh.isValid()))
 		return ade_set_args(L, "b", false);
 
+	if (objh.objp()->type != OBJ_SHIP)
+		return ade_set_args(L, "b", false);
+
+	// this submodel must be a turret
+	auto shipp = &Ships[objh.objp()->instance];
+	auto subsys = ship_get_subsys_for_submodel(shipp, subobjh.GetSubmodelIndex());
+	if (subsys == nullptr || subsys->system_info->type != SUBSYSTEM_TURRET)
+		return ade_set_args(L, "b", false);
+
 	particle::ParticleSource* psp = ps.Get();
 	if (psp == nullptr)
 		return ade_set_args(L, "b", false);
 
-	psp->setHost(std::make_unique<EffectHostTurret>(objh.objp(), subobjh.GetSubmodelIndex(), firepoint, *orientationOverride.GetMatrix(), orientationOverrideRelative));
+	psp->setHost(std::make_unique<EffectHostTurret>(objh.objp(), subobjh.GetSubmodelIndex(), firepoint, false, *orientationOverride.GetMatrix(), orientationOverrideRelative));
 	psp->finishCreation();
 
 	return ade_set_args(L, "b", true);

code/scripting/api/objs/ship.cpp

@@ -34,7 +34,6 @@
 #include "object/objectdock.h"
 #include "parse/parselo.h"
 #include "playerman/player.h"
-#include "scripting/api/objs/message.h"
 #include "ship/ship.h"
 #include "ship/shipfx.h"
 #include "ship/shiphit.h"

code/scripting/api/objs/shipclass.cpp

@@ -1158,7 +1158,7 @@ ADE_FUNC(renderTechModel,
 		}
 	}
 
-	return ade_set_args(L, "b", render_tech_model(TECH_SHIP, x1, y1, x2, y2, zoom, lighting, idx, &orient, tcolor));
+	return ade_set_args(L, "b", render_tech_model(TECH_SHIP, x1, y1, x2, y2, zoom, lighting, idx, &orient, "" , 0.0f, &vmd_zero_vector, tcolor));
 }
 
 // Nuke's alternate tech model rendering function
@@ -1191,7 +1191,7 @@ ADE_FUNC(renderTechModel2, l_Shipclass, "number X1, number Y1, number X2, number
 		}
 	}
 
-	return ade_set_args(L, "b", render_tech_model(TECH_SHIP, x1, y1, x2, y2, zoom, true, idx, orient, tcolor));
+	return ade_set_args(L, "b", render_tech_model(TECH_SHIP, x1, y1, x2, y2, zoom, true, idx, orient, "" , 0.0f, &vmd_zero_vector, tcolor));
 }
 
 ADE_FUNC(renderSelectModel,

code/scripting/lua.cpp

@@ -131,6 +131,10 @@ static int io_open_limited (lua_State *L) {
 		auto file_type = GetFileType(handle);
 		CloseHandle(handle);
 		file_allowed = (file_type == FILE_TYPE_PIPE) || (file_type == FILE_TYPE_CHAR);
+		if (file_type == FILE_TYPE_PIPE) {
+			//For pipes only, we should make sure that the handler has actually closed before continuing.
+			WaitNamedPipeA(filename, 30000);
+		}
 	}
 #else
 	struct stat file_stat_buffer;

code/ship/shield.cpp

@@ -394,7 +394,7 @@ void shield_render_triangle(int texture, float alpha, gshield_tri *trip, matrix
 
 void shield_render_decal(polymodel *pm, matrix *orient, vec3d *pos, matrix* hit_orient, vec3d *hit_pos, float hit_radius, int bitmap_id, color *clr)
 {
-	if (!pm->shield.buffer_id.isValid() || pm->shield.buffer_n_verts < 3) {
+	if (!pm->shield.buffer_id->isValid() || pm->shield.buffer_n_verts < 3) {
 		return;
 	}
 
@@ -410,7 +410,7 @@ void shield_render_decal(polymodel *pm, matrix *orient, vec3d *pos, matrix* hit_
 	material_info.set_impact_transform(*hit_orient, *hit_pos);
 	material_info.set_cull_mode(false);
 
-	gr_render_shield_impact(&material_info, PRIM_TYPE_TRIS, &pm->shield.layout, pm->shield.buffer_id, pm->shield.buffer_n_verts);
+	gr_render_shield_impact(&material_info, PRIM_TYPE_TRIS, &pm->shield.layout, *pm->shield.buffer_id, pm->shield.buffer_n_verts);
 
 	g3_done_instance(true);
 }
@@ -562,9 +562,8 @@ void render_shields()
 void create_tris_containing(vec3d *vp, matrix *orient, shield_info *shieldp, vec3d *tcp, vec3d *centerp, float radius, vec3d *rvec, vec3d *uvec)
 {
 	int	i, j;
-	shield_vertex *verts;
 
-	verts = shieldp->verts;
+	const auto& verts = shieldp->verts;
 
 	for (i=0; i<Num_tris; i++) {
 		if ( !shieldp->tris[i].used ) {
@@ -624,7 +623,7 @@ int get_global_shield_tri()
 
 void create_shield_from_triangle(int trinum, matrix *orient, shield_info *shieldp, vec3d *tcp, vec3d *centerp, float radius, vec3d *rvec, vec3d *uvec)
 {
-	rs_compute_uvs( &shieldp->tris[trinum], shieldp->verts, tcp, radius, rvec, uvec);
+	rs_compute_uvs( &shieldp->tris[trinum], shieldp->verts.get(), tcp, radius, rvec, uvec);
 
 	shieldp->tris[trinum].used = 1;
 

code/ship/ship.cpp

@@ -7884,6 +7884,30 @@ static void ship_copy_subsystem_fixup(ship_info *sip)
 
 }
 
+// Verify that every model_subsystem on this ship_info is linked into the same
+// model the ship is using.  If any subsystem still points at a different
+// model_num, fail with a diagnostic naming both polymodels.  This is the
+// canonical post-condition check for both ship_copy_subsystem_fixup() and
+// model_load() (which is supposed to link subsystems via do_new_subsystem()).
+// context_label disambiguates the call site in the resulting Error text.
+static void verify_ship_subsystems_linked(const ship_info *sip, const char *context_label)
+{
+	for (int i = 0; i < sip->n_subsystems; i++) {
+		if (sip->subsystems[i].model_num == sip->model_num)
+			continue;
+
+		polymodel *sip_pm = (sip->model_num >= 0) ? model_get(sip->model_num) : nullptr;
+		polymodel *subsys_pm = (sip->subsystems[i].model_num >= 0) ? model_get(sip->subsystems[i].model_num) : nullptr;
+		Error(LOCATION, "%s, ship '%s' does not have subsystem '%s' linked into the model file, '%s'.\n\n(Ship_info model is '%s' and subsystem model is '%s'.)",
+			context_label,
+			sip->name,
+			sip->subsystems[i].subobj_name,
+			sip->pof_file,
+			(sip_pm != nullptr) ? sip_pm->filename : "NULL",
+			(subsys_pm != nullptr) ? subsys_pm->filename : "NULL");
+	}
+}
+
 // as with object, don't set next and prev to NULL because they keep the object on the free and used lists
 void ship_subsys::clear()
 {
@@ -8669,7 +8693,7 @@ static void ship_add_cockpit_display(cockpit_display_info *display, int cockpit_
 	auto& glow_texture = (*Player_cockpit_textures)[glow_target];
 	if ( glow_texture == -1) {
 		bm_get_info(diffuse_handle, &w, &h);
-		glow_texture = bm_make_render_target(w, h, BMP_FLAG_RENDER_TARGET_DYNAMIC);
+		glow_texture = bm_make_render_target(w, h, BMP_FLAG_RENDER_TARGET_DYNAMIC | BMP_FLAG_RENDER_TARGET_DEPTH_ATTACHMENT);
 
 		// if no render target was made, bail
 		if ( glow_texture < 0 ) {
@@ -10790,31 +10814,31 @@ void update_firing_sounds(object* objp, ship* shipp)
 		// equality comparisons to -1 are correct here, -2 is valid and means a loop is active but the modder didnt specify an actual loop sound
 
 		if (swp->firing_loop_sounds[i] == -1 && trigger_down && !primaries_locked && selected && has_resources && burst_only_allowed && !dying) {
+			auto* pm = model_get(Ship_info[shipp->ship_info_index].model_num);
+			vec3d snd_pos;
+			vm_vec_avg_n(&snd_pos, pm->gun_banks[i].num_slots, pm->gun_banks[i].pnt);
+
 			if (wip->start_firing_snd.isValid() && start_snd_played != wip->start_firing_snd) {
-				if (objp == Player_obj)
-					snd_play(gamesnd_get_game_sound(wip->start_firing_snd));
-				else
-					snd_play_3d(gamesnd_get_game_sound(wip->start_firing_snd), &objp->pos, &View_position);
+				obj_snd_assign(shipp->objnum, wip->start_firing_snd, &snd_pos, OS_PLAY_ON_PLAYER | OS_LOOPING_DISABLED);
 
 				start_snd_played = wip->start_firing_snd;
 			}
 
-			vec3d pos = model_get(Ship_info[shipp->ship_info_index].model_num)->view_positions[0].pnt;
-
 			if (wip->linked_loop_firing_snd.isValid() && shipp->flags[Ship::Ship_Flags::Primary_linked])
-				swp->firing_loop_sounds[i] = obj_snd_assign(shipp->objnum, wip->linked_loop_firing_snd, &pos, OS_PLAY_ON_PLAYER);
+				swp->firing_loop_sounds[i] = obj_snd_assign(shipp->objnum, wip->linked_loop_firing_snd, &snd_pos, OS_PLAY_ON_PLAYER);
 			else if (wip->loop_firing_snd.isValid())
-				swp->firing_loop_sounds[i] = obj_snd_assign(shipp->objnum, wip->loop_firing_snd, &pos, OS_PLAY_ON_PLAYER);
+				swp->firing_loop_sounds[i] = obj_snd_assign(shipp->objnum, wip->loop_firing_snd, &snd_pos, OS_PLAY_ON_PLAYER);
 			else
 				swp->firing_loop_sounds[i] = -2;
 		} 
 
 		if (swp->firing_loop_sounds[i] != -1 && (!trigger_down || primaries_locked || !selected || !has_resources || !burst_only_allowed || dying)) {
+			auto* pm = model_get(Ship_info[shipp->ship_info_index].model_num);
+			vec3d snd_pos;
+			vm_vec_avg_n(&snd_pos, pm->gun_banks[i].num_slots, pm->gun_banks[i].pnt);
+
 			if (wip->end_firing_snd.isValid() && end_snd_played != wip->end_firing_snd) {
-				if (objp == Player_obj)
-					snd_play(gamesnd_get_game_sound(wip->end_firing_snd));
-				else
-					snd_play_3d(gamesnd_get_game_sound(wip->end_firing_snd), &objp->pos, &View_position);
+				obj_snd_assign(shipp->objnum, wip->end_firing_snd, &snd_pos, OS_PLAY_ON_PLAYER | OS_LOOPING_DISABLED);
 
 				end_snd_played = wip->end_firing_snd;
 			}
@@ -11467,6 +11491,8 @@ int ship_create(matrix* orient, vec3d* pos, int ship_type, const char* ship_name
 	polymodel *pm = model_get(sip->model_num);
 
 	ship_copy_subsystem_fixup(sip);
+	verify_ship_subsystems_linked(sip, "After ship_copy_subsystem_fixup in ship_create");
+
 	show_ship_subsys_count();
 
 	if ( sip->num_detail_levels != pm->n_detail_levels )
@@ -11766,10 +11792,18 @@ static void ship_model_change(int n, int ship_type)
 	}
 
 	model_delete_instance(sp->model_instance_num);
+	if (sp->cockpit_model_instance >= 0) {
+		model_delete_instance(sp->cockpit_model_instance);
+	}
 
 	// create new model instance data
 	// note: this is needed for both subsystem stuff and submodel animation stuff
 	sp->model_instance_num = model_create_instance(OBJ_INDEX(objp), sip->model_num);
+	if (sip->cockpit_model_num >= 0)
+		sp->cockpit_model_instance = model_create_instance(model_objnum_special::OBJNUM_COCKPIT, sip->cockpit_model_num);
+	else
+		sp->cockpit_model_instance = -1;
+	
 	pmi = model_get_instance(sp->model_instance_num);
 
 	// Goober5000 - deal with texture replacement by re-applying the same code we used during parsing
@@ -12069,6 +12103,8 @@ void change_ship_type(int n, int ship_type, int by_sexp)
 	Assertion(weapon_turret_matches.empty(), "Failed to find matches for every turret a projectile was fired from on ship %s in change_ship_type(); get a coder!\n", sp->ship_name);
 	Assertion(last_targeted_matches.empty(), "Somehow failed to find every subsystem a player was previously targeting on ship %s in change_ship_type(); get a coder!\n", sp->ship_name);
 
+	decals::invalidateForShip(sp);
+
 	// point to new ship data
 	ship_model_change(n, ship_type);
 	sp->ship_info_index = ship_type;
@@ -12305,7 +12341,7 @@ void change_ship_type(int n, int ship_type, int by_sexp)
 		}
 
 		// see if this dockpoint is found on the new model under a different name
-		int new_dockpoint_index = find_item_with_string(pm->docking_bays, pm->n_docks, &dock_bay::name, dockpoint_name);
+		int new_dockpoint_index = find_item_with_string(pm->docking_bays.get(), pm->n_docks, &dock_bay::name, dockpoint_name);
 		if (new_dockpoint_index >= 0)
 		{
 			dock_ptr->dockpoint_used = new_dockpoint_index;
@@ -14021,7 +14057,6 @@ void ship_process_targeting_lasers()
 
 			// hmm, why didn't it fire?
 			if(shipp->targeting_laser_objnum < 0){
-				Int3();
 				ship_stop_targeting_laser(shipp);
 			}
 		}
@@ -15403,7 +15438,7 @@ int ship_type_name_lookup(const char *name)
 // subsysp is a pointer to the subsystem.
 int get_subsystem_pos(vec3d* pos, const object* objp, const ship_subsys* subsysp)
 {
-	if (subsysp == NULL) {
+	if ( !subsysp || !subsysp->system_info ) {
 		*pos = objp->pos;
 		return 0;
 	}
@@ -19093,30 +19128,13 @@ void ship_page_in()
 					sip->subsystems[j].model_num = -1;
 
 				ship_copy_subsystem_fixup(&(*sip));
-
-#ifndef NDEBUG
-				for (j = 0; j < sip->n_subsystems; j++) {
-					if (sip->subsystems[j].model_num != sip->model_num) {
-						polymodel *sip_pm = (sip->model_num >= 0) ? model_get(sip->model_num) : NULL;
-						polymodel *subsys_pm = (sip->subsystems[j].model_num >= 0) ? model_get(sip->subsystems[j].model_num) : NULL;
-						Warning(LOCATION, "After ship_copy_subsystem_fixup, ship '%s' does not have subsystem '%s' linked into the model file, '%s'.\n\n(Ship_info model is '%s' and subsystem model is '%s'.)", sip->name, sip->subsystems[j].subobj_name, sip->pof_file, (sip_pm != NULL) ? sip_pm->filename : "NULL", (subsys_pm != NULL) ? subsys_pm->filename : "NULL");
-					}
-				}
-#endif
+				verify_ship_subsystems_linked(&(*sip), "After ship_copy_subsystem_fixup in ship_page_in");
 			} else {
 				// Just to be safe (I mean to check that my code works...)
 				Assert( sip->model_num >= 0 );
 				Assert( sip->model_num == model_previously_loaded );
 
-#ifndef NDEBUG
-				for (j = 0; j < sip->n_subsystems; j++) {
-					if (sip->subsystems[j].model_num != sip->model_num) {
-						polymodel *sip_pm = (sip->model_num >= 0) ? model_get(sip->model_num) : NULL;
-						polymodel *subsys_pm = (sip->subsystems[j].model_num >= 0) ? model_get(sip->subsystems[j].model_num) : NULL;
-						Warning(LOCATION, "Without ship_copy_subsystem_fixup, ship '%s' does not have subsystem '%s' linked into the model file, '%s'.\n\n(Ship_info model is '%s' and subsystem model is '%s'.)", sip->name, sip->subsystems[j].subobj_name, sip->pof_file, (sip_pm != NULL) ? sip_pm->filename : "NULL", (subsys_pm != NULL) ? subsys_pm->filename : "NULL");
-					}
-				}
-#endif
+				verify_ship_subsystems_linked(&(*sip), "Without ship_copy_subsystem_fixup in ship_page_in");
 			}
 		} else {
 			// Model not loaded, so load it
@@ -19124,11 +19142,8 @@ void ship_page_in()
 
 			Assert( sip->model_num >= 0 );
 
-#ifndef NDEBUG
-			// Verify that all the subsystem model numbers are updated
-			for (j = 0; j < sip->n_subsystems; j++)
-				Assertion( sip->subsystems[j].model_num == sip->model_num, "Model reference for subsystem %s (model num: %d) on model %s (model num: %d) is invalid.\n", sip->subsystems[j].name, sip->subsystems[j].model_num, sip->pof_file, sip->model_num );	// JAS
-#endif
+			// Verify that all the subsystem model numbers were updated by model_load
+			verify_ship_subsystems_linked(&(*sip), "After model_load in ship_page_in");
 		}
 
 		// more weapon marking, the weapon info in Ship_info[] is the default
@@ -21053,6 +21068,7 @@ void parse_armor_type()
 	stuff_string(name_buf, F_NAME, NAME_LENGTH);
 	
 	tat = ArmorType(name_buf);
+	tat.flags = 0;
 	
 	//now parse the actual table (damage type/armor type pairs)
 	tat.ParseData();
@@ -21915,20 +21931,18 @@ void toggle_ignore_list_flag(Ship::Ship_Flags flag) {
 
 ship_subsys* ship_get_subsys_for_submodel(ship* shipp, int submodel)
 {
-	ship_subsys* subsys;
-
-	if (submodel == -1) {
+	// no submodel
+	if (submodel == -1)
 		return nullptr;
-	}
 
-	for (subsys = GET_FIRST(&shipp->subsys_list); subsys != END_OF_LIST(&shipp->subsys_list);
-	     subsys = GET_NEXT(subsys)) {
-		if (subsys->system_info->subobj_num == submodel) {
-			return subsys;
-		}
-	}
+	auto pm = model_get(Ship_info[shipp->ship_info_index].model_num);
+	Assertion(submodel >= 0 && submodel < pm->n_models, "Ship %s submodel index %d is out of range!", shipp->ship_name, submodel);
 
-	return nullptr;
+	// no associated subsystem
+	if (pm->submodel[submodel].subsys_num == -1)
+		return nullptr;
+
+	return ship_get_indexed_subsys(shipp, pm->submodel[submodel].subsys_num);
 }
 
 /**

code/ship/shipfx.cpp

@@ -790,13 +790,7 @@ bool shipfx_eye_in_shadow( vec3d *eye_pos, object * src_obj, int light_n )
 	vec3d rp0, rp1;
 	vec3d light_dir;
 
-	// The mc_info struct only needs to be initialized once for this entire function.  This is because
-	// every time the mc variable is reused, every parameter that model_collide reads from is reassigned.
-	// Therefore the stale fields in the rest of the struct do not matter because either a) they are never
-	// read from, or b) they are overwritten by the new collision calculation.
-	mc_info mc;
-
-	rp0 = *eye_pos;	
+	rp0 = *eye_pos;
 	
 	// get the light dir
 	if(!light_get_global_dir(&light_dir, light_n)){
@@ -810,6 +804,8 @@ bool shipfx_eye_in_shadow( vec3d *eye_pos, object * src_obj, int light_n )
 			continue;
 
 		if ( src_obj != objp )	{
+			mc_info mc;
+
 			vm_vec_scale_add( &rp1, &rp0, &light_dir, objp->radius*10.0f );
 
 			mc.model_instance_num = Ships[objp->instance].model_instance_num;
@@ -836,6 +832,8 @@ bool shipfx_eye_in_shadow( vec3d *eye_pos, object * src_obj, int light_n )
 
 		vm_vec_scale_add( &rp1, &rp0, &light_dir, objp->radius*10.0f );
 
+		mc_info mc;
+
 		mc.model_instance_num = -1;
 		mc.model_num = db.model_num;	// Fill in the model to check
 		mc.submodel_num = db.submodel_num;
@@ -864,6 +862,8 @@ bool shipfx_eye_in_shadow( vec3d *eye_pos, object * src_obj, int light_n )
 				vm_vec_unrotate(&pos, &sip->cockpit_offset, &eye_ori);
 				vm_vec_add2(&pos, &eye_posi);
 
+				mc_info mc;
+
 				mc.model_instance_num = -1;
 				mc.model_num = sip->cockpit_model_num;
 				mc.submodel_num = -1;
@@ -913,6 +913,8 @@ bool shipfx_eye_in_shadow( vec3d *eye_pos, object * src_obj, int light_n )
 				&& (!Show_ship_only_if_cockpits_enabled || Cockpit_active) ) {
 				vm_vec_scale_add( &rp1, &rp0, &light_dir, Viewer_obj->radius*10.0f );
 
+				mc_info mc;
+
 				mc.model_instance_num = -1;
 				mc.model_num = sip->model_num;
 				mc.submodel_num = -1;
@@ -976,6 +978,8 @@ bool shipfx_eye_in_shadow( vec3d *eye_pos, object * src_obj, int light_n )
 
         vm_vec_scale_add( &rp1, &rp0, &light_dir, objp->radius*10.0f );
 
+    	mc_info mc;
+
 		mc.model_instance_num = -1;
 		mc.model_num = Asteroid_info[ast->asteroid_type].subtypes[ast->asteroid_subtype].model_number;	// Fill in the model to check
 		mc.submodel_num = -1;
@@ -2854,7 +2858,7 @@ public:
 	int getInt(int *output);
 	//Returns handle, or < 0 on failure/wrong type
 	gamesnd_id getSound();
-	//Returns 1 if buffer was successfully written to
+	//Returns 1 if buffer was successfully written to (output_max includes the null terminator)
 	int getString(char *output, size_t output_max);
 
 	//Returns true if TYPE_NONE
@@ -3049,7 +3053,7 @@ void parse_combined_variable_list(CombinedVariable *dest, flag_def_list *src, si
 		sp = &src[i];
 		dp = &dest[i];
 
-		snprintf(buf, sizeof(buf)-1, "+%s:", sp->name);
+		snprintf(buf, sizeof(buf), "+%s:", sp->name);
 		if(optional_string(buf))
 		{
 			switch(sp->var)

code/ship/shiphit.cpp

@@ -31,6 +31,7 @@
 #include "mission/missionlog.h"
 #include "mod_table/mod_table.h"
 #include "network/multi.h"
+#include "network/multi_interpolate.h"
 #include "network/multi_pmsg.h"
 #include "network/multi_respawn.h"
 #include "network/multimsgs.h"
@@ -2144,6 +2145,11 @@ void ship_apply_whack(const vec3d *force, const vec3d *hit_pos, object *objp)
 		game_whack_apply( -test.xyz.x, -test.xyz.y );
 	}
 
+	if ((Game_mode & GM_MULTIPLAYER) && (objp->type == OBJ_SHIP)) {
+		// temporarily set this as an uninterpolated ship, to make the collision
+		// look more natural until the next update comes in.
+		Interp_info[OBJ_INDEX(objp)].force_interpolation_mode();
+	}
 
 	if (object_is_docked(objp))
 	{

code/utils/threading.cpp

@@ -17,9 +17,19 @@
 
 namespace threading {
 	static size_t num_threads = 1;
+
 	static std::condition_variable wait_for_task;
 	static std::mutex wait_for_task_mutex;
 	static bool wait_for_task_condition;
+
+	static std::condition_variable wait_for_spindown_tasks;
+	static std::mutex wait_for_spindown_task_mutex;
+	static size_t wait_for_spindown_tasks_counter;
+
+	static std::condition_variable wait_for_spinup_tasks;
+	static std::mutex wait_for_spinup_task_mutex;
+	static size_t wait_for_spinup_tasks_counter;
+
 	static std::atomic<WorkerThreadTask> worker_task;
 
 	static SCP_vector<std::thread> worker_threads;
@@ -27,13 +37,26 @@ namespace threading {
 	//Internal Functions
 	static void mp_worker_thread_main(size_t threadIdx) {
 		while(true) {
+			{
+				std::scoped_lock lock {wait_for_spindown_task_mutex};
+				++wait_for_spindown_tasks_counter;
+				wait_for_spindown_tasks.notify_all();
+			}
+			//We're waiting for a new task, so spindown was successful
 			{
 				std::unique_lock<std::mutex> lk(wait_for_task_mutex);
 				wait_for_task.wait(lk, []() { return wait_for_task_condition; });
 			}
+			//Notify that we passed the wait and can now start processing. This is necessary, as slow thread wakeups in very low workloads could cause the wait_for_task_condition to be false, never waking up the thread, locking on spindown wait
+			{
+				std::scoped_lock lock {wait_for_spinup_task_mutex};
+				++wait_for_spinup_tasks_counter;
+				wait_for_spinup_tasks.notify_all();
+			}
 
 			switch (worker_task.load(std::memory_order_acquire)) {
 				case WorkerThreadTask::EXIT:
+					//We're done and will quit, so ensure we report this.
 					return;
 				case WorkerThreadTask::COLLISION:
 					collide_mp_worker_thread(threadIdx);
@@ -144,6 +167,11 @@ namespace threading {
 	//External Functions
 
 	void spin_up_threaded_task(WorkerThreadTask task) {
+		{
+			std::scoped_lock lock {wait_for_spindown_task_mutex};
+			wait_for_spindown_tasks_counter = 0;
+			//No notify here cause we only ever lock, never unlock the wait here.
+		}
 		worker_task.store(task);
 		{
 			std::scoped_lock lock {wait_for_task_mutex};
@@ -153,8 +181,24 @@ namespace threading {
 	}
 
 	void spin_down_threaded_task() {
-		std::scoped_lock lock {wait_for_task_mutex};
-		wait_for_task_condition = false;
+		//Make sure all threads are actually running before stopping the pool again
+		{
+			std::unique_lock<std::mutex> lk(wait_for_spinup_task_mutex);
+			wait_for_spinup_tasks.wait(lk, []() { return wait_for_spinup_tasks_counter >= num_threads; });
+			wait_for_spinup_tasks_counter = 0;
+		}
+		{
+			std::scoped_lock lock {wait_for_task_mutex};
+			wait_for_task_condition = false;
+		}
+	}
+
+	void spin_down_wait_complete() {
+		//Technically, spindowns should only occur when the actual code is confirmed to be complete. So we shouldn't be waiting for long here
+		{
+			std::unique_lock<std::mutex> lk(wait_for_spindown_task_mutex);
+			wait_for_spindown_tasks.wait(lk, []() { return wait_for_spindown_tasks_counter >= num_threads; });
+		};
 	}
 
 	void init_task_pool() {
@@ -181,6 +225,8 @@ namespace threading {
 	void shut_down_task_pool() {
 		spin_up_threaded_task(WorkerThreadTask::EXIT);
 
+		//Technically we could await spin_down_wait_complete here, but since we're returning and joining the threads here, there is no need
+
 		for(auto& thread : worker_threads) {
 			thread.join();
 		}

code/utils/threading.h

@@ -11,6 +11,9 @@ namespace threading {
 	//This _must_ be called on the main thread BEFORE a task completes on a thread of the task pool.
 	void spin_down_threaded_task();
 
+	//This should be called AFTER the command to finish a given task is given. This will block until all threads have returned into a state where they are able to listen to new commands.
+	void spin_down_wait_complete();
+
 	void init_task_pool();
 	void shut_down_task_pool();
 

code/weapon/beam.cpp

@@ -582,7 +582,14 @@ int beam_fire(beam_fire_info *fire_info)
 			host = std::make_unique<EffectHostObject>(new_item->objp, local_pos, orient);
 		}
 		else {
-			host = std::make_unique<EffectHostTurret>(new_item->objp, new_item->subsys->system_info->turret_gun_sobj, new_item->firingpoint);
+			bool is_fighterbeam = false;
+			if (new_item->objp->type == OBJ_SHIP) {
+				auto shipp = &Ships[new_item->objp->instance];
+				if (new_item->subsys == &shipp->fighter_beam_turret_data) {
+					is_fighterbeam = true;
+				}
+			}
+			host = std::make_unique<EffectHostTurret>(new_item->objp, new_item->subsys->system_info->turret_gun_sobj, new_item->firingpoint, is_fighterbeam);
 		}
 
 		source->setHost(std::move(host));
@@ -698,7 +705,6 @@ int beam_fire_targeting(fighter_beam_fire_info *fire_info)
 	if(objnum < 0){
 		beam_delete(new_item);
 		nprintf(("General", "obj_create() failed for beam weapon! bah!\n"));
-		Int3();
 		return -1;
 	}
 	new_item->objnum = objnum;	

code/weapon/shockwave.cpp

@@ -181,7 +181,8 @@ int shockwave_create(int parent_objnum, const vec3d* pos, const shockwave_create
 	objnum = obj_create( OBJ_SHOCKWAVE, real_parent, i, &orient, &sw->pos, sw->outer_radius, tmp_flags + Object::Object_Flags::Renders, false );
 
 	if ( objnum == -1 ){
-		Int3();
+		mprintf(("Couldn't create shockwave object -- out of object slots\n"));
+		return -1;
 	}
 
 	sw->objnum = objnum;

fred2/fredrender.cpp

@@ -1809,6 +1809,10 @@ void render_one_model_htl(object *objp) {
 
 	Assert(objp->type != OBJ_NONE);
 
+	// if this object isn't fully created yet, don't render it
+	if (objp->type == OBJ_SHIP && Ships[objp->instance].create_time == 0)
+		return;
+
 	if (objp->type == OBJ_JUMP_NODE) {
 		return;
 	}

fred2/jumpnodedlg.cpp

@@ -166,7 +166,7 @@ void jumpnode_dlg::initialize_data(int full_update)
 	GetDlgItem(IDC_NAME)->EnableWindow(enable);
 }
 
-int jumpnode_dlg::update_data()
+int jumpnode_dlg::update_data(int redraw)
 {
 	const char *str;
 	char old_name[255];
@@ -345,7 +345,8 @@ int jumpnode_dlg::update_data()
 		
 	}
 
-	update_map_window();
+	if (redraw)
+		update_map_window();
 
 	return 0;
 }

fred2/jumpnodedlg.h

@@ -20,7 +20,7 @@ class jumpnode_dlg : public CDialog
 // Construction
 public:
 	int bypass_errors;
-	int update_data();
+	int update_data(int redraw = 1);
 	void initialize_data(int full_update);
 	void OnOK();
 	BOOL Create();

fred2/management.cpp

@@ -1099,7 +1099,7 @@ int update_dialog_boxes()
 		return z;
 	}
 
-	z = Jumpnode_editor_dialog.update_data();
+	z = Jumpnode_editor_dialog.update_data(0);
 	if (z) {
 		nprintf(("Fred routing", "jumpnode dialog save failed\n"));
 		Jumpnode_editor_dialog

fred2/sexp_tree.cpp

@@ -2965,11 +2965,10 @@ int sexp_tree::add_default_operator(int op_index, int argnum)
 		item_handle = h;
 
 	} else {
+		int sexp_var_index;
 		// special case for sexps that take variables
 		const int op_type = query_operator_argument_type(op_index, argnum);
-		if (op_type == OPF_VARIABLE_NAME) {
-			int sexp_var_index = get_index_sexp_variable_name(item.text);
-			Assert(sexp_var_index != -1);
+		if ((op_type == OPF_VARIABLE_NAME) && ((sexp_var_index = get_index_sexp_variable_name(item.text)) >= 0)) {
 			int type = SEXPT_VALID | SEXPT_VARIABLE;
 			if (Sexp_variables[sexp_var_index].type & SEXP_VARIABLE_STRING) {
 				type |= SEXPT_STRING;
@@ -2983,6 +2982,7 @@ int sexp_tree::add_default_operator(int op_index, int argnum)
 			sprintf(node_text, "%s(%s)", item.text.c_str(), Sexp_variables[sexp_var_index].text);
 			add_variable_data(node_text, type);
 		}
+		// special case for sexps that take containers (this covers multiple container OPF_ types)
 		else if (item.type & SEXPT_CONTAINER_NAME) {
 			Assertion(is_container_name_opf_type(op_type) || op_type == OPF_DATA_OR_STR_CONTAINER,
 				"Attempt to add default container name for a node of non-container type (%d). Please report!",
@@ -2997,7 +2997,7 @@ int sexp_tree::add_default_operator(int op_index, int argnum)
 			Assert(argnum == 1);
 			sexp_list_item temp_item;
 			get_default_value(&temp_item, buf2, op_index, 0);
-			int sexp_var_index = get_index_sexp_variable_name(temp_item.text);
+			sexp_var_index = get_index_sexp_variable_name(temp_item.text);
 			Assert(sexp_var_index != -1);
 
 			// from name get type
@@ -3435,7 +3435,7 @@ int sexp_tree::get_default_value(sexp_list_item *item, char *text_buf, int op, i
 
 		case OPF_ANIMATION_NAME:
 			str = "<Animation trigger name>";
-			break;			
+			break;
 
 		case OPF_CONTAINER_VALUE:
 			str = "<container value>";
@@ -3445,6 +3445,22 @@ int sexp_tree::get_default_value(sexp_list_item *item, char *text_buf, int op, i
 			str = Builtin_messages[0].name;
 			break;
 
+		case OPF_VARIABLE_NAME:
+			str = "<variable name>";
+			break;
+
+		case OPF_CONTAINER_NAME:
+			str = "<container name>";
+			break;
+
+		case OPF_LIST_CONTAINER_NAME:
+			str = "<list container name>";
+			break;
+
+		case OPF_MAP_CONTAINER_NAME:
+			str = "<map container name>";
+			break;
+
 		default:
 			str = "<new default required!>";
 			break;
@@ -7318,7 +7334,15 @@ sexp_list_item *sexp_tree::get_listing_opf_variable_names()
 
 	for (i=0; i<MAX_SEXP_VARIABLES; i++) {
 		if (Sexp_variables[i].type & SEXP_VARIABLE_SET) {
-			head.add_data( Sexp_variables[i].variable_name );
+			int t = 0;
+			if (Sexp_variables[i].type & SEXP_VARIABLE_NUMBER) {
+				t = SEXPT_NUMBER;
+			} else if (Sexp_variables[i].type & SEXP_VARIABLE_STRING) {
+				t = SEXPT_STRING;
+			} else {
+				Assertion(false, "SEXP variable must be a string or a number!");
+			}
+			head.add_data(Sexp_variables[i].variable_name, (t | SEXPT_VALID | SEXPT_VARIABLE));
 		}
 	}
 

freespace2/SDLGraphicsOperations.cpp

@@ -154,13 +154,18 @@ SDLGraphicsOperations::SDLGraphicsOperations() {
 	}
 }
 SDLGraphicsOperations::~SDLGraphicsOperations() {
-	SDL_QuitSubSystem(SDL_INIT_VIDEO);
-	
-	ImGui_ImplSDL2_Shutdown();
+	// make sure imgui stuff is initialized before trying to shut it down
+	if (ImGui::GetCurrentContext()) {
+		if (ImGui::GetIO().BackendPlatformUserData) {
+			ImGui_ImplSDL2_Shutdown();
+		}
 
-	if (!Cmdline_vulkan) {
-		ImGui_ImplOpenGL3_Shutdown();
+		if ( !Cmdline_vulkan && ImGui::GetIO().BackendRendererUserData ) {
+			ImGui_ImplOpenGL3_Shutdown();
+		}
 	}
+
+	SDL_QuitSubSystem(SDL_INIT_VIDEO);
 }
 std::unique_ptr<os::Viewport> SDLGraphicsOperations::createViewport(const os::ViewPortProperties& props)
 {
@@ -267,7 +272,8 @@ std::unique_ptr<os::OpenGLContext> SDLGraphicsOperations::createOpenGLContext(os
 	mprintf(("  Actual SDL Video values    = R: %d, G: %d, B: %d, depth: %d, stencil: %d, double-buffer: %d, FSAA: %d\n",
 		r, g, b, depth, stencil, db, fsaa_samples));
 
-	
+	Assertion(ImGui::GetCurrentContext() != nullptr, "Can't use ImGui without a valid context!");
+
 	ImGui_ImplSDL2_InitForOpenGL(viewport->toSDLWindow(), ctx);
 	ImGui_ImplOpenGL3_Init();
 

freespace2/freespace.cpp

@@ -915,9 +915,9 @@ void game_level_close()
 	//to accidentally use an override here without realizing it.
 	if (!scripting::hooks::OnMissionEndHook->isActive() || !scripting::hooks::OnMissionEndHook->isOverride())
 	{
-		// save player-persistent variables and containers
-		mission_campaign_save_on_close_variables();	// Goober5000
-		mission_campaign_save_on_close_containers(); // jg18
+		// save on-close variables and containers
+		mission_campaign_store_variables(SEXP_VARIABLE_SAVE_ON_MISSION_CLOSE, false);	// Goober5000
+		mission_campaign_store_containers(ContainerType::SAVE_ON_MISSION_CLOSE, false);	// jg18
 
 		// De-Initialize the game subsystems
 		obj_delete_all();
@@ -6286,6 +6286,10 @@ void mouse_force_pos(int x, int y);
 			break;		
 
 		case GS_STATE_LOOP_BRIEF:
+			if (old_state == GS_STATE_MAIN_MENU) {
+				main_hall_stop_music(true);
+				main_hall_stop_ambient();
+			}
 			loop_brief_init();
 			break;
 

lib/libpcpnatpmp/src/pcp_event_handler.c

@@ -756,6 +756,7 @@ static int get_first_flow_iter(pcp_flow_t *f, void *data) {
     case pfs_idle:
     case pfs_wait_for_server_init:
     case pfs_send:
+    case pfs_failed:
         d->msg = f;
         return 1;
     default:
@@ -833,10 +834,15 @@ static pcp_server_state_e handle_server_ping(pcp_server_t *s) {
 
             PCP_LOG_END(PCP_LOGLVL_DEBUG);
             return pss_wait_ping_resp;
+        } else {
+            gettimeofday(&s->next_timeout, NULL);
+            PCP_LOG_END(PCP_LOGLVL_DEBUG);
+            return pss_set_not_working;
         }
     }
     s->next_timeout.tv_sec = 0;
     s->next_timeout.tv_usec = 0;
+    PCP_LOG_END(PCP_LOGLVL_DEBUG);
     return pss_ping;
 }
 

lib/prebuilt.cmake

@@ -6,12 +6,12 @@ set(PREBUILT_LIB_DIR "${CMAKE_CURRENT_BINARY_DIR}/prebuilt")
 set(CURRENT_ROOT "${CMAKE_CURRENT_BINARY_DIR}")
 
 function(get_prebuilt_path OUT_VAR)
-    if (IS_DIRECTORY "${PREBUILT_LIB_DIR}")
-        if (NOT "${FSO_PREBUILT_OVERRIDE}" STREQUAL "")
-            set(${OUT_VAR} "${FSO_PREBUILT_OVERRIDE}" PARENT_SCOPE)
-            return()
-        endif()
+    if (NOT "${FSO_PREBUILT_OVERRIDE}" STREQUAL "" AND IS_DIRECTORY "${FSO_PREBUILT_OVERRIDE}")
+        set(${OUT_VAR} "${FSO_PREBUILT_OVERRIDE}" PARENT_SCOPE)
+        return()
+    endif()
 
+    if (IS_DIRECTORY "${PREBUILT_LIB_DIR}")
         if ("${DOWNLOADED_PREBUILT_VERSION}" STREQUAL "${PREBUILT_VERSION_NAME}")
             # Libraries already downloaded and up-to-date
             set(${OUT_VAR} "${PREBUILT_LIB_DIR}" PARENT_SCOPE)

qtfred/src/mission/FredRenderer.cpp

@@ -794,6 +794,10 @@ void FredRenderer::render_one_model_htl(object* objp,
 
 	Assert(objp->type != OBJ_NONE);
 
+	// if this object isn't fully created yet, don't render it
+	if (objp->type == OBJ_SHIP && Ships[objp->instance].create_time == 0)
+		return;
+
 	if (objp->type == OBJ_JUMP_NODE) {
 		return;
 	}

qtfred/src/ui/widgets/sexp_tree.cpp

@@ -1037,11 +1037,10 @@ int sexp_tree::add_default_operator(int op_index, int argnum) {
 		add_or_replace_operator(item.op);
 		item_index = index;
 	} else {
+		int sexp_var_index;
 		// special case for sexps that take variables
 		const int op_type = query_operator_argument_type(op_index, argnum);
-		if (op_type == OPF_VARIABLE_NAME) {
-			int sexp_var_index = get_index_sexp_variable_name(item.text);
-			Assert(sexp_var_index != -1);
+		if ((op_type == OPF_VARIABLE_NAME) && ((sexp_var_index = get_index_sexp_variable_name(item.text)) >= 0)) {
 			int type = SEXPT_VALID | SEXPT_VARIABLE;
 			if (Sexp_variables[sexp_var_index].type & SEXP_VARIABLE_STRING) {
 				type |= SEXPT_STRING;
@@ -1055,21 +1054,22 @@ int sexp_tree::add_default_operator(int op_index, int argnum) {
 			sprintf(node_text, "%s(%s)", item.text.c_str(), Sexp_variables[sexp_var_index].text);
 			add_variable_data(node_text, type);
 		}
+		// special case for sexps that take containers (this covers multiple container OPF_ types)
 		else if (item.type & SEXPT_CONTAINER_NAME) {
 			Assertion(is_container_name_opf_type(op_type) || op_type == OPF_DATA_OR_STR_CONTAINER,
 				"Attempt to add default container name for a node of non-container type (%d). Please report!",
 				op_type);
 			add_container_name(item.text.c_str());
 		}
-			// modify-variable data type depends on type of variable being modified
-			// (we know this block is handling the second argument since it's not OPF_VARIABLE_NAME)
+		// modify-variable data type depends on type of variable being modified
+		// (we know this block is handling the second argument since it's not OPF_VARIABLE_NAME)
 		else if (Operators[op_index].value == OP_MODIFY_VARIABLE) {
 			// the the variable name
 			char buf2[256];
 			Assert(argnum == 1);
 			sexp_list_item temp_item;
 			get_default_value(&temp_item, buf2, op_index, 0);
-			int sexp_var_index = get_index_sexp_variable_name(temp_item.text);
+			sexp_var_index = get_index_sexp_variable_name(temp_item.text);
 			Assert(sexp_var_index != -1);
 
 			// from name get type
@@ -1084,7 +1084,7 @@ int sexp_tree::add_default_operator(int op_index, int argnum) {
 			}
 			add_data(item.text.c_str(), type);
 		}
-			// all other sexps and parameters
+		// all other sexps and parameters
 		else {
 			add_data(item.text.c_str(), item.type);
 		}
@@ -1481,6 +1481,22 @@ int sexp_tree::get_default_value(sexp_list_item* item, char* text_buf, int op, i
 		str = Builtin_messages[0].name;
 		break;
 
+	case OPF_VARIABLE_NAME:
+		str = "<variable name>";
+		break;
+
+	case OPF_CONTAINER_NAME:
+		str = "<container name>";
+		break;
+
+	case OPF_LIST_CONTAINER_NAME:
+		str = "<list container name>";
+		break;
+
+	case OPF_MAP_CONTAINER_NAME:
+		str = "<map container name>";
+		break;
+
 	default:
 		str = "<new default required!>";
 		break;
@@ -5187,7 +5203,15 @@ sexp_list_item* sexp_tree::get_listing_opf_variable_names() {
 
 	for (i = 0; i < MAX_SEXP_VARIABLES; i++) {
 		if (Sexp_variables[i].type & SEXP_VARIABLE_SET) {
-			head.add_data(Sexp_variables[i].variable_name);
+			int t = 0;
+			if (Sexp_variables[i].type & SEXP_VARIABLE_NUMBER) {
+				t = SEXPT_NUMBER;
+			} else if (Sexp_variables[i].type & SEXP_VARIABLE_STRING) {
+				t = SEXPT_STRING;
+			} else {
+				Assertion(false, "SEXP variable must be a string or a number!");
+			}
+			head.add_data(Sexp_variables[i].variable_name, (t | SEXPT_VALID | SEXPT_VARIABLE));
 		}
 	}
 

test/src/model/test_modelread.cpp

@@ -26,7 +26,7 @@ protected:
 		pm = new polymodel();
 		pmi = new polymodel_instance();
 
-		pm->submodel = new bsp_info[3];
+		pm->submodel = make_shared<bsp_info[]>(3);
 		pmi->submodel = new submodel_instance[3];
 
 		pm->submodel[0].parent = -1;
@@ -51,7 +51,7 @@ protected:
 	void TearDown() override {
 		test::FSTestFixture::TearDown();
 
-		delete[] pm->submodel;
+		pm->submodel.reset();
 		delete[] pmi->submodel;
 		delete pm;
 		delete pmi;

version_override.cmake

@@ -0,0 +1,5 @@
+set(FSO_VERSION_MAJOR 25)
+set(FSO_VERSION_MINOR 0)
+set(FSO_VERSION_BUILD 0)
+set(FSO_VERSION_REVISION 0)
+set(FSO_VERSION_REVISION_STR )