Ghost entities

[immagiov4]

Minetest version

5.6.1

OS / Hardware

Operating system: Ubuntu 22.04.2 LTS
CPU: Intel(R) Xeon(R) CPU E5-2630 v4 4core

Summary

Sometimes calling get_children() on a player returns entities with almost no methods except for get_hp() and is_player(). This is a problem because if you try to call any other method (such as remove()), the game crashes.

Steps to reproduce

I haven't found a way to reproduce this. What I know is that the "ghost" entities are not attached to the player anymore, because they disappear after I remove them (from within their on_step callback). May this bug be somewhat lag-related?

[sfan5]

This is a problem because if you try to call any other method, the game crashes.

In what way? Error message?

What I know is that the "ghost" entities are not attached to the player anymore, because they disappear after I remove them (from within their on_step callback).

What do you mean? I thought removing them was not possible.
It would be good if you could describe what you are doing in details or post example code.

[immagiov4]

This is a problem because if you try to call any other method, the game crashes.

In what way? Error message?

Yes, an "attempt to index a nil value" error (referring to a line like entity:remove())

What do you mean? I thought removing them was not possible.
It would be good if you could describe what you are doing in details or post example code.

Sorry, I wasn't clear enough: I remove the entity but then, for some reason, the get_children() seems to think it's still there. I have no example code for this, 'cause I can't reproduce it in my local server. It happens on the AES server in the Fantasy Brawl minigame, but I have now way of knowing which entity caused it (because the "ghost" entities have no get_luaentity()). All I know is that I create an entity, attach it to the player and then it gets destroyed after x seconds in the on_step callback.

[sfan5]

Yes, an "attempt to index a nil value" error (referring to a line like entity:remove())

All entity references returned by the engine have the metatable set, so it's not possible that a method is nil. I would investigate other mods you have and check if they do some advanced things like operate with fake players.

From your description the following should reproduce it but I can indeed find nothing wrong:

-- use with /spawnentity
minetest.register_entity("test2:thing", {
	initial_properties = {
		visual = "cube",
		textures = {"smoke_puff.png", "smoke_puff.png", "smoke_puff.png",
			"smoke_puff.png", "smoke_puff.png", "smoke_puff.png"},
		static_save = false,
	},

	on_activate = function(self, staticdata, dtime_s)
		self.timer = 0
		self.object:set_attach(core.get_player_by_name("singleplayer"))
	end,

	on_step = function(self, dtime, moveresult)
		self.timer = self.timer + dtime
		if self.timer > 2 then
			local parent = self.object:get_attach()
			self.object:remove()

			print("we were attached to " .. tostring(parent))
			local children = parent:get_children()
			print("which has " .. #children .. " children now:")
			for _, obj in ipairs(children) do
				local line = "  " .. tostring(obj)
				if obj:get_hp() == nil then
					line = line .. " (broken ref)"
				end
				if obj == self.object then
					line = line .. " <- this is us"
				end
				print(line)
			end
		end
	end,
})

[immagiov4]

These are our mods, but I don't think any of them operate with fake players (our sure don't). What kind of advanced things could cause get_children to return entities with no methods? Moreover, when trying to dump the ghost entity (dump(player:get_children()[entity_index])), it prints the userdata metatable, don't know if that's useful (I don't know much about metatables). I'd also like to highlight that is not something that happens everytime, but maybe in a match out of three.

[fluxionary]

this may not be related, but on the your-land server, since 5.6.0 or 5.6.1, we've noticed that occasionally the results of get_objects_inside_radius and get_objects_in_area will be entities which are neither players, nor "live" entities (get_luaentity() returns nil). i was never able to reproduce the issue in a controlled environment. it caused several crashes, which were solved by adding appropriate checks in the relevant code.

[sfan5]

FWIW I think this should still be tagged "unconfirmed bug" because nobody has posted the actual code they used in connection with the bug, nor the actual error message.
What fluxionary posted is also not necessarily related and should be investigated in isolation.

[Zughy]

Happened to me as well and it doesn't seem to be related to children entities in particular https://gitlab.com/zughy-friends-minetest/block_league/-/issues/133. Server and client 5.8.0

Basically someone hit an entity that wasn't really there anymore. get_hp() still existed (a few lines before the 1st line of code linked in the issue, L196) but get_luaentity() didn't (the 2nd line of code). Like fluxionary, I don't know how to reproduce it in a controlled environment

[appgurueu]

"Attempt to index a nil value" on a line like entity:remove() means entity is nil (if entity.remove was nil, the error would be "attempt to call a nil value"); either the object has been invalidated, or it is a player.

I don't think this makes much sense, but we return a HP of 1 (!) for invalid entities:

minetest/src/script/lua_api/l_object.cpp

Lines 248 to 252 in fe6da3a

	if (sao == nullptr) {
	// Default hp is 1
	lua_pushnumber(L, 1);
	return 1;
	}

We should probably deprecate this behavior.

A check like obj:get_hp() > 0 is probably not very helpful. It only excludes dead players. It may give you invalid entities.

I believe is_player will simply return false for invalid entities:

minetest/src/script/lua_api/l_object.cpp

Line 848 in fe6da3a

lua_pushboolean(L, (player != nullptr));

I think this once again demonstrates how much of a footgun invalid object refs are, hence is possibly related to #14687

this may not be related, but on the your-land server, since 5.6.0 or 5.6.1, we've noticed that occasionally the results of get_objects_inside_radius and get_objects_in_area will be entities which are neither players, nor "live" entities (get_luaentity() returns nil).

This sounds like invalid object refs. See the linked issue and also this MTG issue. To sum it up: While you are iterating your objects and doing something to them (say, punching them), other objects may be invalidated. A classic example would be blowing up an entity with attached entities. The parent may kill its children, leaving you with invalid object refs.

This issue is not fixable without changing the API or adding a new API: These functions return lists. They can't remove objects from these lists after the fact (okay, technically they could, but that would be a potentially inefficient mess). A proper design requires an iterator which skips invalidated objects.

---

FWIW, so far I can not find a bug in get_children. It seems that when a child is detached, it is removed immediately from the set of children. When a child dies, it also takes care to remove itself from its parent's attachment list.

[Zughy]

A proper design requires an iterator which skips invalidated objects

Yes, please. That would have avoided my crash (since entities are obtained through a for loop of a raycast)

[appgurueu]

Tagging this possible close as outlined in #14769 (comment)