Item Entity: Prevent moveresult assert when attached

[mazes-80]

Moveresult is assigned only when no parents

minetest/src/server/luaentity_sao.cpp

Lines 150 to 181 in baf99f8

	collisionMoveResult moveresult, *moveresult_p = nullptr;
	// Each frame, parent position is copied if the object is attached, otherwise it's calculated normally
	// If the object gets detached this comes into effect automatically from the last known origin
	if (auto *parent = getParent()) {
	m_base_position = parent->getBasePosition();
	m_velocity = v3f(0,0,0);
	m_acceleration = v3f(0,0,0);
	} else {
	if(m_prop.physical){
	aabb3f box = m_prop.collisionbox;
	box.MinEdge *= BS;
	box.MaxEdge *= BS;
	f32 pos_max_d = BS*0.25; // Distance per iteration
	v3f p_pos = m_base_position;
	v3f p_velocity = m_velocity;
	v3f p_acceleration = m_acceleration;
	moveresult = collisionMoveSimple(m_env, m_env->getGameDef(),
	pos_max_d, box, m_prop.stepheight, dtime,
	&p_pos, &p_velocity, p_acceleration,
	this, m_prop.collideWithObjects);
	moveresult_p = &moveresult;
	// Apply results
	m_base_position = p_pos;
	m_velocity = p_velocity;
	m_acceleration = p_acceleration;
	} else {
	m_base_position += (m_velocity + m_acceleration * 0.5f * dtime) * dtime;
	m_velocity += dtime * m_acceleration;
	}

An assertion is launched if no moveresult for __builtin:item

minetest/builtin/game/item_entity.lua

Lines 204 to 205 in baf99f8

	assert(moveresult,
	"Collision info missing, this is caused by an out-of-date/buggy mod or game")

This PR proposes to slip off the assert. It originated in fixing one mod that worked flawlessly until minetest-5.3.0. Each time weapon grabbed an an item entity the server restarted with the message from the culprit assert, which was sad as grabbing items is one of the purposes of this mod. I ended up seeing why the mod was not doing anything bad today, when the message states it is because of the mod.

Via #minetest Desour states it may not be good to skip action for item entity. I hope it was just an immediate reaction and nothing is really bad de-charging some position computation on parent for some steps.
In case skipping steps is really bad, silently disable attaching when __builtin and/or document, seems a valid path.

[Desour]

Via #minetest Desour states it may not be good to skip action for item entity. I hope it was just an immediate reaction and nothing is really bad de-charging some position computation on parent for some steps.
In case skipping steps is really bad, silently disable attaching when __builtin and/or document, seems a valid path.

What I meant is that attaching an item object to something (what the mod does) is a bad idea. It's generally safer to register a new entity that looks like the dropped item, but does not do the other things, like being pick-up-able or moving out of nodes. For example, pipeworks and factory do this.

---

moveresult being nil if attached should be documented.

Idk if the assert should be skipped.

[SmallJoker]

MTG carts solves it by calling disable_physics to make sure the item is handled as if it were resting: https://github.com/minetest/minetest_game/blob/8dee348d975e01490627d0cee482b8f5ae684ae8/mods/carts/cart_entity.lua#L329-L334

I think this fix makes sense. Calling this special function should not be necessary to e.g. carry items with another object.

[mazes-80]

MTG carts solves it by calling disable_physics

Thank for the hint. That was already the workaround I ended up with for the chakram mod.

[SmallJoker]

Makes sense to me.