use std::optional so the object->instance id is always valid

Author: MjnMixael

code/lab/renderer/lab_renderer.cpp

@@ -114,17 +114,18 @@ void LabRenderer::renderModel(float frametime) {
 	}
 
 	if (obj->type == OBJ_PROP) {
-		Props[obj->instance].flags.set(Prop::Prop_Flags::Draw_as_wireframe, renderFlags[LabRenderFlag::ShowWireframe]);
-		Props[obj->instance].flags.set(Prop::Prop_Flags::Render_full_detail, renderFlags[LabRenderFlag::ShowFullDetail]);
-		Props[obj->instance].flags.set(Prop::Prop_Flags::Render_without_light,
+		prop* propp = prop_id_lookup(obj->instance);
+		propp->flags.set(Prop::Prop_Flags::Draw_as_wireframe, renderFlags[LabRenderFlag::ShowWireframe]);
+		propp->flags.set(Prop::Prop_Flags::Render_full_detail, renderFlags[LabRenderFlag::ShowFullDetail]);
+		propp->flags.set(Prop::Prop_Flags::Render_without_light,
 			renderFlags[LabRenderFlag::NoLighting] || currentMissionBackground == LAB_MISSION_NONE_STRING);
-		Props[obj->instance].flags.set(Prop::Prop_Flags::Render_without_diffuse, renderFlags[LabRenderFlag::NoDiffuseMap]);
-		Props[obj->instance].flags.set(Prop::Prop_Flags::Render_without_glowmap, renderFlags[LabRenderFlag::NoGlowMap]);
-		Props[obj->instance].flags.set(Prop::Prop_Flags::Render_without_normalmap, renderFlags[LabRenderFlag::NoNormalMap]);
-		Props[obj->instance].flags.set(Prop::Prop_Flags::Render_without_specmap, renderFlags[LabRenderFlag::NoSpecularMap]);
-		Props[obj->instance].flags.set(Prop::Prop_Flags::Render_without_reflectmap, renderFlags[LabRenderFlag::NoReflectMap]);
-		Props[obj->instance].flags.set(Prop::Prop_Flags::Render_without_heightmap, renderFlags[LabRenderFlag::NoHeightMap]);
-		Props[obj->instance].flags.set(Prop::Prop_Flags::Render_without_ambientmap, renderFlags[LabRenderFlag::NoAOMap]);
+		propp->flags.set(Prop::Prop_Flags::Render_without_diffuse, renderFlags[LabRenderFlag::NoDiffuseMap]);
+		propp->flags.set(Prop::Prop_Flags::Render_without_glowmap, renderFlags[LabRenderFlag::NoGlowMap]);
+		propp->flags.set(Prop::Prop_Flags::Render_without_normalmap, renderFlags[LabRenderFlag::NoNormalMap]);
+		propp->flags.set(Prop::Prop_Flags::Render_without_specmap, renderFlags[LabRenderFlag::NoSpecularMap]);
+		propp->flags.set(Prop::Prop_Flags::Render_without_reflectmap, renderFlags[LabRenderFlag::NoReflectMap]);
+		propp->flags.set(Prop::Prop_Flags::Render_without_heightmap, renderFlags[LabRenderFlag::NoHeightMap]);
+		propp->flags.set(Prop::Prop_Flags::Render_without_ambientmap, renderFlags[LabRenderFlag::NoAOMap]);
 	}
 
 	if (obj->type == OBJ_WEAPON) {

code/object/collideshipship.cpp

@@ -587,8 +587,9 @@ void calculate_ship_ship_collision_physics(collision_info_struct *ship_ship_hit_
 		} else if (heavy->type == OBJ_DEBRIS) {
 			pm = model_get(Debris[heavy->instance].model_num);
 		} else if (heavy->type == OBJ_PROP) {
-			pm = model_get(Prop_info[Props[heavy->instance].prop_info_index].model_num);
-			model_instance_num = Props[heavy->instance].model_instance_num;
+			prop* propp = prop_id_lookup(heavy->instance);
+			pm = model_get(Prop_info[propp->prop_info_index].model_num);
+			model_instance_num = propp->model_instance_num;
 			pmi = model_get_instance(model_instance_num);
 		} else {
 			// we should have caught this already

code/object/collideshipweapon.cpp

@@ -629,7 +629,7 @@ static int prop_weapon_check_collision(object* prop_objp, object* weapon_objp, f
 	Assert(prop_objp->type == OBJ_PROP);
 	Assert(prop_objp->instance >= 0);
 
-	prop* propp = &Props[prop_objp->instance];
+	prop* propp = prop_id_lookup(prop_objp->instance);
 	prop_info* prinfo = &Prop_info[propp->prop_info_index];
 
 	Assert(weapon_objp != nullptr);

code/object/object.cpp

@@ -2277,7 +2277,7 @@ int object_get_model_instance_num(const object *objp)
 		case OBJ_RAW_POF:
 			return Pof_objects[objp->instance].model_instance;
 		case OBJ_PROP:
-			return Props[objp->instance].model_instance_num;
+			return prop_id_lookup(objp->instance)->model_instance_num;
 		default:
 			break;
 	}

code/object/objectsort.cpp

@@ -77,7 +77,7 @@ inline bool sorted_obj::operator < (const sorted_obj &other) const
 	} else if (obj->type == OBJ_RAW_POF) {
 		model_num_a = Pof_objects[obj->instance].model_num;
 	} else if (obj->type == OBJ_PROP) {
-		model_num_a = Props[obj->instance].model_instance_num;
+		model_num_a = prop_id_lookup(obj->instance)->model_instance_num;
 	}
 
 	if ( other.obj->type == OBJ_SHIP ) {
@@ -105,7 +105,7 @@ inline bool sorted_obj::operator < (const sorted_obj &other) const
 	} else if (other.obj->type == OBJ_RAW_POF) {
 		model_num_b = Pof_objects[other.obj->instance].model_num;
 	} else if (other.obj->type == OBJ_PROP) {
-		model_num_b = Props[other.obj->instance].model_instance_num;
+		model_num_b = prop_id_lookup(other.obj->instance)->model_instance_num;
 	}
 
 	if ( model_num_a == model_num_b ) {

code/parse/sexp.cpp

@@ -5783,7 +5783,7 @@ const prop *eval_prop(int node)
 		if (Sexp_nodes[node].cache->sexp_node_data_type != OPF_PROP)
 			return nullptr;
 
-		return &Props[Sexp_nodes[node].cache->ship_registry_index];
+		return prop_id_lookup(Sexp_nodes[node].cache->ship_registry_index);
 	}
 
 	// maybe forward to a special-arg node
@@ -5803,7 +5803,7 @@ const prop *eval_prop(int node)
 		if (!is_node_value_dynamic(node))
 			Sexp_nodes[node].cache = new sexp_cached_data(OPF_PROP, -1, prop_idx);
 
-		return &Props[prop_idx];
+		return prop_id_lookup(prop_idx);
 	}
 
 	// we know nothing about this prop, apparently

code/prop/prop.cpp

@@ -17,7 +17,7 @@ bool Props_inited = false;
 
 SCP_vector<prop_info> Prop_info;
 
-SCP_vector<prop> Props;
+SCP_vector<std::optional<prop>> Props;
 
 static SCP_vector<SCP_string> Removed_props;
 
@@ -40,9 +40,13 @@ int prop_name_lookup(const char *name)
 	Assertion(name != nullptr, "NULL name passed to prop_name_lookup");
 
 	for (int i=0; i<static_cast<int>(Props.size()); i++){
-		if (Props[i].objnum >= 0){
-			if (Objects[Props[i].objnum].type == OBJ_PROP){
-				if (!stricmp(name, Props[i].prop_name)){
+		auto prop = Props[i] ? &Props[i].value() : nullptr;
+		if (prop == nullptr) {
+			continue;
+		}
+		if (prop->objnum >= 0){
+			if (Objects[prop->objnum].type == OBJ_PROP) {
+				if (!stricmp(name, prop->prop_name)) {
 					return i;
 				}
 			}
@@ -53,6 +57,15 @@ int prop_name_lookup(const char *name)
 	return -1;
 }
 
+prop* prop_id_lookup(int id)
+{
+	if (id < 0 || id >= static_cast<int>(Props.size()) || !Props[id].has_value()) {
+		Assertion(false, "Could not find prop for id %d", id);
+		return nullptr;
+	}
+	return &Props[id].value();
+}
+
 void parse_prop_table(const char* filename)
 {
 	read_file_text(filename, CF_TYPE_TABLES);
@@ -257,7 +270,10 @@ int prop_create(matrix* orient, vec3d* pos, int prop_type, const char* name)
 	pip = &(Prop_info[prop_type]);
 
 	Props.emplace_back(prop());
-	propp = &Props.back();
+	int new_id = static_cast<int>(Props.size()) - 1;
+	propp = prop_id_lookup(new_id);
+	Assertion(propp != nullptr, "Could not create prop!");
+
 	propp->prop_info_index = prop_type;
 
 	if ((name == nullptr) || (prop_name_lookup(name) >= 0)) {
@@ -321,7 +337,7 @@ int prop_create(matrix* orient, vec3d* pos, int prop_type, const char* name)
 
 	int objnum = obj_create(OBJ_PROP,
 		-1,
-		static_cast<int>(Props.size() - 1),
+		new_id,
 		orient,
 		pos,
 		model_get_radius(pip->model_num),
@@ -389,23 +405,25 @@ int prop_create(matrix* orient, vec3d* pos, int prop_type, const char* name)
 void prop_delete(object* obj)
 {
 	int num = obj->instance;
-	Assert(num >= 0 && num <= static_cast<int>(Props.size()));
 
 	int objnum = OBJ_INDEX(obj);
-	Assert(Props[num].objnum == objnum);
+	Assertion(Props[num].has_value(), "Props[%d] is already nullopt!", num);
 
-	prop* propp = &Props[num];
+	prop& propp = *Props[num];
+	Assertion(propp.objnum == objnum, "Prop object id does not match passed object!");
 
-	propp->objnum = -1;
+	propp.objnum = -1;
 
 	//animation::ModelAnimationSet::stopAnimations(model_get_instance(propp->model_instance_num));
 
 	// glow point banks
-	propp->glow_point_bank_active.clear();
+	propp.glow_point_bank_active.clear();
 
-	model_delete_instance(propp->model_instance_num);
+	model_delete_instance(propp.model_instance_num);
 
-	Props.erase(Props.begin() + num);
+	// Leave the slot empty for the duration of the scene
+	// The Props array will be compacted at the end of the level
+	Props[num] = std::nullopt;
 }
 
 /**
@@ -416,8 +434,7 @@ void prop_delete(object* obj)
  */
 static void prop_model_change(int n, int prop_type)
 {
-	Assert( n >= 0 && n < MAX_PROPS );
-	prop* sp = &Props[n];
+	prop* sp = prop_id_lookup(n);
 	prop_info* sip = &(Prop_info[prop_type]);
 	object* objp = &Objects[sp->objnum];
 	polymodel_instance* pmi = model_get_instance(sp->model_instance_num);
@@ -497,8 +514,7 @@ static void prop_model_change(int n, int prop_type)
  */
 void change_prop_type(int n, int prop_type)
 {
-	Assert( n >= 0 && n < MAX_PROPS );
-	prop* sp = &Props[n];
+	prop* sp = prop_id_lookup(n);
 
 	// do a quick out if we're already using the new ship class
 	if (sp->prop_info_index == prop_type)
@@ -515,10 +531,6 @@ void change_prop_type(int n, int prop_type)
 	prop_model_change(n, prop_type);
 	sp->prop_info_index = prop_type;
 
-	// get the before and after models (the new model may have only been loaded in ship_model_change)
-	auto pm = model_get(sip->model_num);
-	auto pm_orig = model_get(sip_orig->model_num);
-
 	// check class-specific flags
 
 	if (sip->flags[Prop::Info_Flags::No_collide])								// changing TO a no-collision ship class
@@ -530,10 +542,10 @@ void change_prop_type(int n, int prop_type)
 void prop_render(object* obj, model_draw_list* scene)
 {
 	int num = obj->instance;
-	Assert(num >= 0 && num <= static_cast<int>(Props.size()));
 
-	prop* propp = &Props[num];
-	prop_info* pip = &Prop_info[Props[num].prop_info_index];
+	prop* propp = prop_id_lookup(num);
+
+	prop_info* pip = &Prop_info[propp->prop_info_index];
 
 	MONITOR_INC(NumPropsRend, 1);
 
@@ -637,9 +649,14 @@ void spawn_test_prop()
 
 void props_level_close()
 {
-	while (!Props.empty()) {
-		prop_delete(&Objects[Props.back().objnum]);
+	for (auto& opt_prop : Props) {
+		if (opt_prop.has_value()) {
+			prop_delete(&Objects[opt_prop->objnum]);
+		}
 	}
+
+	// Clear all props and empty prop slots
+	Props.clear();
 }
 
 
@@ -648,14 +665,13 @@ int prop_check_collision(object* prop_obj, object* other_obj, vec3d* hitpos, col
 {
 	mc_info	mc;
 
-	Assert(prop_obj->type == OBJ_PROP);
+	Assertion(prop_obj->type == OBJ_PROP, "Object is not a prop!");
 
 	int num = prop_obj->instance;
 
-	Assert(num >= 0 && num < (int)Props.size());
-	Assert(Props[num].objnum == OBJ_INDEX(prop_obj));
-
-	prop* propp = &Props[num];
+	prop* propp = prop_id_lookup(num);
+	Assertion(propp->objnum == OBJ_INDEX(prop_obj), "Prop object id does not match passed object!");
+	
 	prop_info* prinfo = &Prop_info[propp->prop_info_index];
 
 	// debris_hit_info NULL - so debris-weapon collision
@@ -839,7 +855,7 @@ int prop_check_collision(object* prop_obj, object* other_obj, vec3d* hitpos, col
 				asteroid* astp = &Asteroids[instance];
 				mc.model_instance_num = astp->model_instance_num;
 				mc.model_num = Asteroid_info[astp->asteroid_type].subtypes[astp->asteroid_subtype].model_number;	
-				mc.submodel_num - 1;
+				mc.submodel_num - 1; // Typo from asteroth?
 			}
 
 			mc.orient = &heavy_obj->orient;				// The object's orient
@@ -862,7 +878,7 @@ int prop_check_collision(object* prop_obj, object* other_obj, vec3d* hitpos, col
 			}
 		} else { // prop is the heavy object
 			mc.flags = (MC_CHECK_MODEL | MC_CHECK_SPHERELINE);
-			int instance = heavy_obj->instance;
+			//int instance = heavy_obj->instance;
 
 			// fill the appropriate model data
 			mc.model_instance_num = propp->model_instance_num;

code/prop/prop.h

@@ -50,7 +50,7 @@ extern bool Props_inited;
 // Global prop info array
 extern SCP_vector<prop_info> Prop_info;
 
-extern SCP_vector<prop> Props;
+extern SCP_vector<std::optional<prop>> Props;
 
 inline int prop_info_size()
 {
@@ -69,6 +69,7 @@ void props_level_close();
 
 int prop_info_lookup(const char* token);
 int prop_name_lookup(const char* name);
+prop* prop_id_lookup(int id);
 
 void change_prop_type(int n, int prop_type);
 

code/scripting/api/libs/mission.cpp

@@ -129,6 +129,45 @@ int object_subclass_count(A& object_subclass_array, size_t array_size)
 	return object_subclass_at_index(object_subclass_array, array_size, COUNT_OBJECTS);
 }
 
+// Overload for a vector of std::optional objects
+template <typename T>
+int object_subclass_at_index(const SCP_vector<std::optional<T>>& vec, int index)
+{
+	int count = 0;
+
+	for (const auto& opt_obj : vec) {
+
+		if (!opt_obj.has_value()) {
+			continue;
+		}
+
+
+		const T& obj = *opt_obj;
+
+		int objnum = obj.objnum;
+		if (objnum < 0 || objnum >= MAX_OBJECTS)
+			continue;
+		if (Objects[objnum].flags[Object::Object_Flags::Should_be_dead])
+			continue;
+
+		++count;
+
+		if (count == index) {
+			return obj.objnum;
+		}
+	}
+
+	if (index == COUNT_OBJECTS)
+		return count;
+	else
+		return -1;
+}
+
+template <typename A>
+int object_subclass_count(A& object_subclass_array)
+{
+	return object_subclass_at_index(object_subclass_array, COUNT_OBJECTS);
+}
 
 namespace scripting {
 namespace api {
@@ -565,15 +604,15 @@ ADE_INDEXER(l_Mission_Props, "number/string IndexOrName", "Gets prop", "prop", "
 
 	if (idx >= 0)
 	{
-		return ade_set_args(L, "o", l_Prop.Set(object_h(&Objects[Props[idx].objnum])));
+		return ade_set_args(L, "o", l_Prop.Set(object_h(&Objects[prop_id_lookup(idx)->objnum])));
 	}
 	else
 	{
 		idx = atoi(name);
 
 		int objnum = -1;
 		if (idx > 0)
-			objnum = object_subclass_at_index(Props, MAX_PROPS, idx);
+			objnum = object_subclass_at_index(Props, idx);
 
 		return ade_set_args(L, "o", l_Prop.Set(object_h(objnum)));
 	}
@@ -586,7 +625,7 @@ ADE_FUNC(__len, l_Mission_Props, NULL,
 		 "number",
 		 "Number of props in the mission, or 0 if props haven't been initialized yet")
 {
-	return ade_set_args(L, "i", object_subclass_count(Props, MAX_PROPS));
+	return ade_set_args(L, "i", object_subclass_count(Props));
 }
 
 //****SUBLIBRARY: Mission/Waypoints
@@ -1410,8 +1449,6 @@ ADE_FUNC(createProp,
 	int obj_idx = prop_create(real_orient, &pos, pclass, name);
 
 	if(obj_idx >= 0) {
-		auto propp = &Props[Objects[obj_idx].instance];
-
 		prop_info* pip = &Prop_info[pclass];
 
 		model_page_in_textures(pip->model_num, pclass);