Merge pull request #4513 from EatThePath/cleanup-eval-stack

Clean up the lua stack after EvalStringWithReturn's function creation

code/ai/aicode.cpp

@@ -10022,11 +10022,11 @@ void guard_object_was_hit(object *guard_objp, object *hitter_objp)
 
 			// This just checks to see that the target was a valid type. If it is not one of these, something really
 			// could be drastically wrong in the engine.
-			Assertion(aip->target_objnum == OBJ_SHIP || aip->target_objnum == OBJ_WEAPON || aip->target_objnum == OBJ_DEBRIS || aip->target_objnum == OBJ_ASTEROID || aip->target_objnum == OBJ_WAYPOINT,
+			Assertion(Objects[aip->target_objnum].type == OBJ_SHIP || Objects[aip->target_objnum].type == OBJ_WEAPON || Objects[aip->target_objnum].type == OBJ_DEBRIS || Objects[aip->target_objnum].type == OBJ_ASTEROID || Objects[aip->target_objnum].type == OBJ_WAYPOINT,
 				"This function just discovered that %s has an invalid target object type of %d. This is bad. Please report!", 
 				Ships[aip->shipnum].ship_name, Objects[aip->target_objnum].type);
 
-			if (!(aip->target_objnum == OBJ_SHIP || aip->target_objnum == OBJ_WEAPON || aip->target_objnum == OBJ_ASTEROID || aip->target_objnum == OBJ_DEBRIS)){
+			if (!(Objects[aip->target_objnum].type == OBJ_SHIP || Objects[aip->target_objnum].type == OBJ_WEAPON || Objects[aip->target_objnum].type == OBJ_ASTEROID || Objects[aip->target_objnum].type == OBJ_DEBRIS)){
 				// it's probably a valid target, but retail would not count those cases, so just return.
 				return;
 			} 

code/parse/sexp.cpp

@@ -32318,6 +32318,21 @@ int get_category(int sexp_id)
 	return category;
 }
 
+// Goober5000
+const char *get_category_name(int category_id)
+{
+	if (category_id == OP_CATEGORY_CHANGE2)
+		category_id = OP_CATEGORY_CHANGE;
+
+	for (auto &category : op_menu)
+	{
+		if (category.id == category_id)
+			return category.name.c_str();
+	}
+
+	return "<unknown>";
+}
+
 // Goober5000
 int category_of_subcategory(int subcategory_id)
 {

code/parse/sexp.h

@@ -1359,6 +1359,7 @@ void set_turret_secondary_ammo(ship_subsys *turret, int requested_bank, int requ
 extern int get_category(int sexp_id);
 extern int category_of_subcategory(int subcategory_id);
 extern int get_subcategory(int sexp_id);
+extern const char *get_category_name(int category_id);
 
 // Goober5000
 extern void sexp_music_close();

code/parse/sexp/sexp_lookup.cpp

@@ -125,8 +125,10 @@ void parse_sexp_table(const char* filename) {
 				luaSexp->parseTable();
 
 				int op = add_dynamic_sexp(std::move(instance), SEXP_GOAL_OPERATOR);
-				luaSexp->registerAIMode(op);
-				luaSexp->maybeRegisterPlayerOrder(op);
+				if (op >= 0) {
+					luaSexp->registerAIMode(op);
+					luaSexp->maybeRegisterPlayerOrder(op);
+				}
 			}
 			required_string("#End");
 		}
@@ -177,8 +179,19 @@ int add_dynamic_sexp(std::unique_ptr<DynamicSEXP>&& sexp, int type)
 	new_op.min = sexp->getMinimumArguments();
 	new_op.max = sexp->getMaximumArguments();
 
+	int free_op_index = get_next_free_operator(sexp->getCategory());
+	if (free_op_index >= 256) {
+		Warning(LOCATION, "There are too many SEXPs in the %s category.  The SEXP %s will not be added.", get_category_name(sexp->getCategory()), sexp->getName().c_str());
+		return -1;
+	}
+
+	if (Operators.size() >= FIRST_OP) {
+		Warning(LOCATION, "There are too many total SEXPs.  The SEXP %s will not be added.", sexp->getName().c_str());
+		return -1;
+	}
+
 	// For now, all dynamic SEXPS are only valid in missions
-	new_op.value = get_next_free_operator(sexp->getCategory()) | sexp->getCategory() | OP_NONCAMPAIGN_FLAG;
+	new_op.value = free_op_index | sexp->getCategory() | OP_NONCAMPAIGN_FLAG;
 	new_op.type = type;
 
 	global.operator_const_mapping.insert(std::make_pair(new_op.value, std::move(sexp)));

code/scripting/scripting.h

@@ -337,14 +337,20 @@ bool script_state::EvalStringWithReturn(const char* string, const char* format,
 				scripting::ade_get_args(LuaState, format, rtn);
 			}
 		} catch (const LuaException&) {
+			lua_pop(LuaState, 1);
+
 			return false;
 		}
 	} catch (const LuaException& e) {
 		LuaError(GetLuaSession(), "%s", e.what());
 
+		lua_pop(LuaState, 1);
+
 		return false;
 	}
 
+	lua_pop(LuaState, 1);
+
 	return true;
 }
 

freespace2/freespace.cpp

@@ -6684,19 +6684,6 @@ void game_shutdown(void)
 		gr_flip();
 	}
 	
-	// Free SEXP resources
-	sexp_shutdown();
-	
-	// Free the scripting resources of the new UI first
-	scpui::shutdown_scripting();
-
-	// Everything after this should be done without scripting so we can free those resources here. By this point, all things that hold Lua References must be destroyed (such as Lua SEXPs)
-	Script_system.Clear();
-
-	// Deinitialize the new UI system, needs to be done after scripting shutdown to make sure the resources were
-	// released properly
-	scpui::shutdown();
-
 	// if the player has left the "player select" screen and quit the game without actually choosing
 	// a player, Player will be nullptr, in which case we shouldn't write the player file out!
 	if (!(Game_mode & GM_STANDALONE_SERVER) && (Player!=nullptr) && !Is_standalone){
@@ -6732,7 +6719,6 @@ void game_shutdown(void)
 
 	scoring_close();
 
-
 	stars_close();			// clean out anything used by stars code
 
 	// the menu close functions will unload the bitmaps if they were displayed during the game
@@ -6742,6 +6728,19 @@ void game_shutdown(void)
 	// free left over memory from table parsing
 	player_tips_close();
 
+
+	// more fundamental shutdowns begin here ----------
+
+	sexp_shutdown();				// Free SEXP resources
+
+	scpui::shutdown_scripting();	// Free the scripting resources of the new UI first
+
+	Script_system.Clear();			// Everything after this should be done without scripting so we can free those resources here.
+									// By this point, all things that hold Lua References must be destroyed (such as Lua SEXPs)
+
+	scpui::shutdown();				// Deinitialize the new UI system, needs to be done after scripting shutdown to make sure the resources were released properly
+
+
 	control_config_common_close();
 	io::joystick::shutdown();
 
@@ -6780,7 +6779,6 @@ void game_shutdown(void)
 	}
 
 	lcl_xstr_close();
-
 }
 
 // game_stop_looped_sounds()