Add warning for initial properties directly inside definition (#9650)

src/script/common/c_content.cpp

@@ -195,6 +195,43 @@ void push_item_definition_full(lua_State *L, const ItemDefinition &i)
 	lua_setfield(L, -2, "node_placement_prediction");
 }
 
+/******************************************************************************/
+const std::array<const char *, 33> object_property_keys = {
+	"hp_max",
+	"breath_max",
+	"physical",
+	"collide_with_objects",
+	"collisionbox",
+	"selectionbox",
+	"pointable",
+	"visual",
+	"mesh",
+	"visual_size",
+	"textures",
+	"colors",
+	"spritediv",
+	"initial_sprite_basepos",
+	"is_visible",
+	"makes_footstep_sound",
+	"stepheight",
+	"eye_height",
+	"automatic_rotate",
+	"automatic_face_movement_dir",
+	"backface_culling",
+	"glow",
+	"nametag",
+	"nametag_color",
+	"automatic_face_movement_max_rotation_per_sec",
+	"infotext",
+	"static_save",
+	"wield_item",
+	"zoom_fov",
+	"use_texture_alpha",
+	"shaded",
+	"damage_texture_modifier",
+	"show_on_minimap"
+};
+
 /******************************************************************************/
 void read_object_properties(lua_State *L, int index,
 		ServerActiveObject *sao, ObjectProperties *prop, IItemDefManager *idef)
@@ -362,6 +399,9 @@ void read_object_properties(lua_State *L, int index,
 	getboolfield(L, -1, "show_on_minimap", prop->show_on_minimap);
 
 	getstringfield(L, -1, "damage_texture_modifier", prop->damage_texture_modifier);
+
+	// Remember to update object_property_keys above
+	// when adding a new property
 }
 
 /******************************************************************************/
@@ -459,6 +499,9 @@ void push_object_properties(lua_State *L, ObjectProperties *prop)
 	lua_setfield(L, -2, "damage_texture_modifier");
 	lua_pushboolean(L, prop->show_on_minimap);
 	lua_setfield(L, -2, "show_on_minimap");
+
+	// Remember to update object_property_keys above
+	// when adding a new property
 }
 
 /******************************************************************************/

src/script/common/c_content.h

@@ -33,6 +33,7 @@ extern "C" {
 
 #include <iostream>
 #include <vector>
+#include <array>
 
 #include "irrlichttypes_bloated.h"
 #include "util/string.h"
@@ -71,6 +72,9 @@ struct collisionMoveResult;
 
 extern struct EnumString es_TileAnimationType[];
 
+
+extern const std::array<const char *, 33> object_property_keys;
+
 void               read_content_features     (lua_State *L, ContentFeatures &f,
                                               int index);
 void               push_content_features     (lua_State *L,
@@ -118,6 +122,7 @@ void               read_object_properties    (lua_State *L, int index,
                                               ServerActiveObject *sao,
                                               ObjectProperties *prop,
                                               IItemDefManager *idef);
+
 void               push_object_properties    (lua_State *L,
                                               ObjectProperties *prop);
 

src/script/common/c_internal.cpp

@@ -177,7 +177,8 @@ void log_deprecated(lua_State *L, std::string message, int stack_depth)
 	if (mode == DeprecatedHandlingMode::Ignore)
 		return;
 
-	script_log_add_source(L, message, stack_depth);
+	if (stack_depth >= 0)
+		script_log_add_source(L, message, stack_depth);
 	warningstream << message << std::endl;
 
 	if (mode == DeprecatedHandlingMode::Error)

src/script/common/c_internal.h

@@ -147,7 +147,8 @@ DeprecatedHandlingMode get_deprecated_handling_mode();
  *
  * @param L Lua State
  * @param message The deprecation method
- * @param stack_depth How far on the stack to the first user function (ie: not builtin or core)
+ * @param stack_depth How far on the stack to the first user function
+ *        (ie: not builtin or core). -1 to disabled.
  */
 void log_deprecated(lua_State *L, std::string message, int stack_depth = 1);
 

src/script/cpp_api/s_entity.cpp

@@ -181,8 +181,39 @@ std::string ScriptApiEntity::luaentity_GetStaticdata(u16 id)
 	return std::string(s, len);
 }
 
+void ScriptApiEntity::logDeprecationForExistingProperties(lua_State *L, int index, const std::string &name)
+{
+	if (deprecation_warned_init_properties.find(name) != deprecation_warned_init_properties.end())
+		return;
+
+	if (index < 0)
+		index = lua_gettop(L) + 1 + index;
+
+	if (!lua_istable(L, index))
+		return;
+
+	for (const char *key : object_property_keys) {
+		lua_getfield(L, index, key);
+		bool exists = !lua_isnil(L, -1);
+		lua_pop(L, 1);
+
+		if (exists) {
+			std::ostringstream os;
+
+			os << "Reading initial object properties directly from an entity definition is deprecated, "
+				<< "move it to the 'initial_properties' table instead. "
+				<< "(Property '" << key << "' in entity '" << name << "')" << std::endl;
+
+			log_deprecated(L, os.str(), -1);
+
+			deprecation_warned_init_properties.insert(name);
+			break;
+		}
+	}
+}
+
 void ScriptApiEntity::luaentity_GetProperties(u16 id,
-		ServerActiveObject *self, ObjectProperties *prop)
+		ServerActiveObject *self, ObjectProperties *prop, const std::string &entity_name)
 {
 	SCRIPTAPI_PRECHECKHEADER
 
@@ -195,6 +226,7 @@ void ScriptApiEntity::luaentity_GetProperties(u16 id,
 	prop->hp_max = 10;
 
 	// Deprecated: read object properties directly
+	logDeprecationForExistingProperties(L, -1, entity_name);
 	read_object_properties(L, -1, self, prop, getServer()->idef());
 
 	// Read initial_properties

src/script/cpp_api/s_entity.h

@@ -21,6 +21,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #include "cpp_api/s_base.h"
 #include "irr_v3d.h"
+#include <unordered_set>
 
 struct ObjectProperties;
 struct ToolCapabilities;
@@ -37,7 +38,7 @@ public:
 	void luaentity_Remove(u16 id);
 	std::string luaentity_GetStaticdata(u16 id);
 	void luaentity_GetProperties(u16 id,
-			ServerActiveObject *self, ObjectProperties *prop);
+			ServerActiveObject *self, ObjectProperties *prop, const std::string &entity_name);
 	void luaentity_Step(u16 id, float dtime,
 		const collisionMoveResult *moveresult);
 	bool luaentity_Punch(u16 id,
@@ -51,4 +52,11 @@ public:
 private:
 	bool luaentity_run_simple_callback(u16 id, ServerActiveObject *sao,
 		const char *field);
+
+	void logDeprecationForExistingProperties(lua_State *L, int index, const std::string &name);
+
+	/** Stores names of entities that already caused a deprecation warning due to
+	 * properties being outside of initial_properties. If an entity's name is in here,
+	 * it won't cause any more of those deprecation warnings. */
+	std::unordered_set<std::string> deprecation_warned_init_properties;
 };

src/server/luaentity_sao.cpp

@@ -103,7 +103,7 @@ void LuaEntitySAO::addedToEnvironment(u32 dtime_s)
 	if(m_registered){
 		// Get properties
 		m_env->getScriptIface()->
-			luaentity_GetProperties(m_id, this, &m_prop);
+			luaentity_GetProperties(m_id, this, &m_prop, m_init_name);
 		// Initialize HP from properties
 		m_hp = m_prop.hp_max;
 		// Activate entity, supplying serialized state