Help modders deal with object invalidation (#14769)

* Skip invalid objects in raycasts
* Add `ObjectRef:is_valid` method
* Add object inside radius / area iterators which skip invalid objects
* Update docs to clarify object invalidation and how to deal with it

---------

Co-authored-by: sfan5 <sfan5@live.de>

builtin/game/misc.lua

@@ -272,3 +272,29 @@ function core.get_globals_to_transfer()
 	}
 	return all
 end
+
+do
+	local function valid_object_iterator(objects)
+		local i = 0
+		local function next_valid_object()
+			i = i + 1
+			local obj = objects[i]
+			if obj == nil then
+				return
+			end
+			if obj:is_valid() then
+				return obj
+			end
+			return next_valid_object()
+		end
+		return next_valid_object
+	end
+
+	function core.objects_inside_radius(center, radius)
+		return valid_object_iterator(core.get_objects_inside_radius(center, radius))
+	end
+
+	function core.objects_in_area(min_pos, max_pos)
+		return valid_object_iterator(core.get_objects_in_area(min_pos, max_pos))
+	end
+end

doc/lua_api.md

@@ -6133,12 +6133,24 @@ Environment access
     * Items can be added also to unloaded and non-generated blocks.
 * `minetest.get_player_by_name(name)`: Get an `ObjectRef` to a player
     * Returns nothing in case of error (player offline, doesn't exist, ...).
-* `minetest.get_objects_inside_radius(pos, radius)`
-    * returns a list of ObjectRefs.
+* `minetest.get_objects_inside_radius(center, radius)`
+    * returns a list of ObjectRefs
     * `radius`: using a Euclidean metric
-* `minetest.get_objects_in_area(pos1, pos2)`
-    * returns a list of ObjectRefs.
-    * `pos1` and `pos2` are the min and max positions of the area to search.
+    * **Warning**: Any kind of interaction with the environment or other APIs
+      can cause later objects in the list to become invalid while you're iterating it.
+      (e.g. punching an entity removes its children)
+      It is recommended to use `minetest.objects_inside_radius` instead, which
+      transparently takes care of this possibility.
+* `minetest.objects_inside_radius(center, radius)`
+    * returns an iterator of valid objects
+    * example: `for obj in minetest.objects_inside_radius(center, radius) do obj:punch(...) end`
+* `minetest.get_objects_in_area(min_pos, max_pos)`
+    * returns a list of ObjectRefs
+    * `min_pos` and `max_pos` are the min and max positions of the area to search
+    * **Warning**: The same warning as for `minetest.get_objects_inside_radius` applies.
+      Use `minetest.objects_in_area` instead to iterate only valid objects.
+* `minetest.objects_in_area(min_pos, max_pos)`
+	* returns an iterator of valid objects
 * `minetest.set_timeofday(val)`: set time of day
     * `val` is between `0` and `1`; `0` for midnight, `0.5` for midday
 * `minetest.get_timeofday()`: get time of day
@@ -7807,13 +7819,18 @@ When you receive an `ObjectRef` as a callback argument or from another API
 function, it is possible to store the reference somewhere and keep it around.
 It will keep functioning until the object is unloaded or removed.
 
-However, doing this is **NOT** recommended as there is (intentionally) no method
-to test if a previously acquired `ObjectRef` is still valid.
-Instead, `ObjectRefs` should be "let go" of as soon as control is returned from
-Lua back to the engine.
+However, doing this is **NOT** recommended - `ObjectRefs` should be "let go"
+of as soon as control is returned from Lua back to the engine.
+
 Doing so is much less error-prone and you will never need to wonder if the
 object you are working with still exists.
 
+If this is not feasible, you can test whether an `ObjectRef` is still valid
+via `object:is_valid()`.
+
+Getters may be called for invalid objects and will return nothing then.
+All other methods should not be called on invalid objects.
+
 ### Attachments
 
 It is possible to attach objects to other objects (`set_attach` method).
@@ -7832,6 +7849,8 @@ child will follow movement and rotation of that bone.
 
 ### Methods
 
+* `is_valid()`: returns whether the object is valid.
+   * See "Advice on handling `ObjectRefs`" above.
 * `get_pos()`: returns position as vector `{x=num, y=num, z=num}`
 * `set_pos(pos)`:
     * Sets the position of the object.

games/devtest/mods/unittests/entity.lua

@@ -71,13 +71,13 @@ local function test_entity_lifecycle(_, pos)
 
 	-- with binary in staticdata
 	local obj = core.add_entity(pos, "unittests:callbacks", "abc\000def")
+	assert(obj and obj:is_valid())
 	check_log({"on_activate(7)"})
 
 	obj:set_hp(0)
 	check_log({"on_death(nil)", "on_deactivate(true)"})
 
-	-- objectref must be invalid now
-	assert(obj:get_velocity() == nil)
+	assert(not obj:is_valid())
 end
 unittests.register("test_entity_lifecycle", test_entity_lifecycle, {map=true})
 
@@ -130,3 +130,57 @@ local function test_entity_attach(player, pos)
 	obj:remove()
 end
 unittests.register("test_entity_attach", test_entity_attach, {player=true, map=true})
+
+core.register_entity("unittests:dummy", {
+	initial_properties = {
+		hp_max = 1,
+		visual = "upright_sprite",
+		textures = { "no_texture.png" },
+		static_save = false,
+	},
+})
+
+local function test_entity_raycast(_, pos)
+	local obj1 = core.add_entity(pos, "unittests:dummy")
+	local obj2 = core.add_entity(pos:offset(1, 0, 0), "unittests:dummy")
+	local raycast = core.raycast(pos:offset(-1, 0, 0), pos:offset(2, 0, 0), true, false)
+	for pt in raycast do
+		if pt.type == "object" then
+			assert(pt.ref == obj1)
+			obj1:remove()
+			obj2:remove()
+			obj1 = nil -- object should be hit exactly one
+		end
+	end
+	assert(obj1 == nil)
+end
+unittests.register("test_entity_raycast", test_entity_raycast, {map=true})
+
+local function test_object_iterator(pos, make_iterator)
+	local obj1 = core.add_entity(pos, "unittests:dummy")
+	local obj2 = core.add_entity(pos, "unittests:dummy")
+	assert(obj1 and obj2)
+	local found = false
+	-- As soon as we find one of the objects, we remove both, invalidating the other.
+	for obj in make_iterator() do
+		assert(obj:is_valid())
+		if obj == obj1 or obj == obj2 then
+			obj1:remove()
+			obj2:remove()
+			found = true
+		end
+	end
+	assert(found)
+end
+
+unittests.register("test_objects_inside_radius", function(_, pos)
+	test_object_iterator(pos, function()
+		return core.objects_inside_radius(pos, 1)
+	end)
+end, {map=true})
+
+unittests.register("test_objects_in_area", function(_, pos)
+	test_object_iterator(pos, function()
+		return core.objects_in_area(pos:offset(-1, -1, -1), pos:offset(1, 1, 1))
+	end)
+end, {map=true})

src/script/lua_api/l_env.cpp

@@ -155,6 +155,7 @@ void LuaLBM::trigger(ServerEnvironment *env, v3s16 p,
 int LuaRaycast::l_next(lua_State *L)
 {
 	GET_PLAIN_ENV_PTR;
+	ServerEnvironment *senv = dynamic_cast<ServerEnvironment*>(env);
 
 	bool csm = false;
 #ifndef SERVER
@@ -163,7 +164,17 @@ int LuaRaycast::l_next(lua_State *L)
 
 	LuaRaycast *o = checkObject<LuaRaycast>(L, 1);
 	PointedThing pointed;
-	env->continueRaycast(&o->state, &pointed);
+	for (;;) {
+		env->continueRaycast(&o->state, &pointed);
+		if (pointed.type != POINTEDTHING_OBJECT)
+			break;
+		if (!senv)
+			break;
+		const auto *obj = senv->getActiveObject(pointed.object_id);
+		if (obj && !obj->isGone())
+			break;
+		// skip gone object
+	}
 	if (pointed.type == POINTEDTHING_NOTHING)
 		lua_pushnil(L);
 	else

src/script/lua_api/l_object.cpp

@@ -19,6 +19,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #include "lua_api/l_object.h"
 #include <cmath>
+#include <lua.h>
 #include "lua_api/l_internal.h"
 #include "lua_api/l_inventory.h"
 #include "lua_api/l_item.h"
@@ -106,6 +107,13 @@ int ObjectRef::l_remove(lua_State *L)
 	return 0;
 }
 
+// is_valid(self)
+int ObjectRef::l_is_valid(lua_State *L)
+{
+	lua_pushboolean(L, getobject(checkObject<ObjectRef>(L, 1)) != nullptr);
+	return 1;
+}
+
 // get_pos(self)
 int ObjectRef::l_get_pos(lua_State *L)
 {
@@ -2646,6 +2654,7 @@ const char ObjectRef::className[] = "ObjectRef";
 luaL_Reg ObjectRef::methods[] = {
 	// ServerActiveObject
 	luamethod(ObjectRef, remove),
+	luamethod(ObjectRef, is_valid),
 	luamethod_aliased(ObjectRef, get_pos, getpos),
 	luamethod_aliased(ObjectRef, set_pos, setpos),
 	luamethod(ObjectRef, add_pos),

src/script/lua_api/l_object.h

@@ -67,6 +67,9 @@ private:
 	// remove(self)
 	static int l_remove(lua_State *L);
 
+	// is_valid(self)
+	static int l_is_valid(lua_State *L);
+
 	// get_pos(self)
 	static int l_get_pos(lua_State *L);