Fix object:punch(puncher, ... should allow nil for puncher

builtin/game/knockback.lua

@@ -22,14 +22,19 @@ local function vector_absmax(v)
 	return max(max(abs(v.x), abs(v.y)), abs(v.z))
 end
 
-core.register_on_punchplayer(function(player, hitter, time_from_last_punch, tool_capabilities, unused_dir, damage)
+core.register_on_punchplayer(function(player, hitter, time_from_last_punch, tool_capabilities, dir, damage)
+	if not hitter and not dir then
+		return
+	end
 	if player:get_hp() == 0 then
 		return -- RIP
 	end
 
-	-- Server::handleCommand_Interact() adds eye offset to one but not the other
-	-- so the direction is slightly off, calculate it ourselves
-	local dir = vector.subtract(player:get_pos(), hitter:get_pos())
+	if hitter then
+		-- Server::handleCommand_Interact() adds eye offset to one but not the other
+		-- so the direction is slightly off, calculate it ourselves
+		dir = vector.subtract(player:get_pos(), hitter:get_pos())
+	end
 	local d = vector.length(dir)
 	if d ~= 0.0 then
 		dir = vector.divide(dir, d)

doc/lua_api.md

@@ -5723,7 +5723,7 @@ Call these functions only at load time!
     * Called when a player is punched
     * Note: This callback is invoked even if the punched player is dead.
     * `player`: ObjectRef - Player that was punched
-    * `hitter`: ObjectRef - Player that hit
+    * `hitter`: ObjectRef - Player that hit. Can be nil.
     * `time_from_last_punch`: Meant for disallowing spamming of clicks
       (can be nil).
     * `tool_capabilities`: Capability table of used item (can be nil)
@@ -7778,11 +7778,11 @@ child will follow movement and rotation of that bone.
     * no-op if object is attached
 * `punch(puncher, time_from_last_punch, tool_capabilities, dir)`
     * punches the object, triggering all consequences a normal punch would have
-    * `puncher`: another `ObjectRef` which punched the object
+    * `puncher`: another `ObjectRef` which punched the object or `nil`
     * `dir`: direction vector of punch
     * Other arguments: See `on_punch` for entities
-    * All arguments except `puncher` can be `nil`, in which case a default
-      value will be used
+    * Arguments `time_from_last_punch`, `tool_capabilities`, and `dir`
+      will be replaced with a default value when the caller sets them to `nil`.
 * `right_click(clicker)`:
     * simulates using the 'place/use' key on the object
     * triggers all consequences as if a real player had done this

games/devtest/mods/callbacks/entities.lua

@@ -76,3 +76,26 @@ minetest.register_entity("callbacks:callback_step", {
 		message("on_step callback entity: on_step! pos="..spos(self).."; dtime="..dtime)
 	end,
 })
+
+-- Callback punch with nil puncher
+minetest.register_entity("callbacks:callback_puncher", {
+	initial_properties = {
+		visual = "upright_sprite",
+		textures = { "callbacks_callback_entity.png" },
+		infotext = "Callback entity for nil puncher test.",
+	},
+
+	on_punch = function(self, puncher, time_from_last_punch, tool_capabilities, dir, damage)
+		if puncher then
+			puncher:punch(nil, time_from_last_punch, tool_capabilities, dir)
+			self.object:punch(nil, time_from_last_punch, tool_capabilities, dir)
+		else
+			message(
+				"Callback entity: on_punch with nil puncher "..
+				"pos="..spos(self).."; "..
+				"time_from_last_punch="..time_from_last_punch.."; "..
+				"tool_capabilities="..dump(tool_capabilities).."; "..
+				"dir="..dump(dir).."; damage="..damage)
+		end
+	end,
+})

games/devtest/mods/callbacks/init.lua

@@ -1,3 +1,4 @@
 dofile(minetest.get_modpath("callbacks").."/items.lua")
 dofile(minetest.get_modpath("callbacks").."/nodes.lua")
 dofile(minetest.get_modpath("callbacks").."/entities.lua")
+dofile(minetest.get_modpath("callbacks").."/players.lua")

games/devtest/mods/callbacks/players.lua

@@ -0,0 +1,11 @@
+
+local message = function(msg)
+	minetest.log("action", "[callbacks] "..msg)
+	minetest.chat_send_all(msg)
+end
+
+core.register_on_punchplayer(function(player, hitter, time_from_last_punch, tool_capabilities, dir, damage)
+	if not hitter then
+		message("Player "..player:get_player_name().." punched without hitter.")
+	end
+end)

src/script/cpp_api/s_entity.cpp

@@ -262,8 +262,6 @@ bool ScriptApiEntity::luaentity_Punch(u16 id,
 {
 	SCRIPTAPI_PRECHECKHEADER
 
-	assert(puncher);
-
 	int error_handler = PUSH_ERROR_HANDLER(L);
 
 	// Get core.luaentities[id]
@@ -278,7 +276,10 @@ bool ScriptApiEntity::luaentity_Punch(u16 id,
 	}
 	luaL_checktype(L, -1, LUA_TFUNCTION);
 	lua_pushvalue(L, object);  // self
-	objectrefGetOrCreate(L, puncher);  // Clicker reference
+	if (puncher)
+		objectrefGetOrCreate(L, puncher);  // Puncher reference
+	else
+		lua_pushnil(L);
 	lua_pushnumber(L, time_from_last_punch);
 	push_tool_capabilities(L, *toolcap);
 	push_v3f(L, dir);

src/script/cpp_api/s_player.cpp

@@ -68,7 +68,10 @@ bool ScriptApiPlayer::on_punchplayer(ServerActiveObject *player,
 	lua_getfield(L, -1, "registered_on_punchplayers");
 	// Call callbacks
 	objectrefGetOrCreate(L, player);
-	objectrefGetOrCreate(L, hitter);
+	if (hitter)
+		objectrefGetOrCreate(L, hitter);
+	else
+		lua_pushnil(L);
 	lua_pushnumber(L, time_from_last_punch);
 	push_tool_capabilities(L, *toolcap);
 	push_v3f(L, dir);

src/script/lua_api/l_object.cpp

@@ -170,16 +170,21 @@ int ObjectRef::l_punch(lua_State *L)
 {
 	NO_MAP_LOCK_REQUIRED;
 	ObjectRef *ref = checkObject<ObjectRef>(L, 1);
-	ObjectRef *puncher_ref = checkObject<ObjectRef>(L, 2);
+	ObjectRef *puncher_ref = lua_isnoneornil(L, 2) ? nullptr : checkObject<ObjectRef>(L, 2);
 	ServerActiveObject *sao = getobject(ref);
-	ServerActiveObject *puncher = getobject(puncher_ref);
-	if (sao == nullptr || puncher == nullptr)
+	ServerActiveObject *puncher = puncher_ref ? getobject(puncher_ref) : nullptr;
+	if (sao == nullptr)
 		return 0;
 
 	float time_from_last_punch = readParam<float>(L, 3, 1000000.0f);
 	ToolCapabilities toolcap = read_tool_capabilities(L, 4);
-	v3f dir = readParam<v3f>(L, 5, sao->getBasePosition() - puncher->getBasePosition());
-	dir.normalize();
+	v3f dir;
+	if (puncher) {
+		dir = readParam<v3f>(L, 5, sao->getBasePosition() - puncher->getBasePosition());
+		dir.normalize();
+	} else {
+		dir = readParam<v3f>(L, 5, v3f(0));
+	}
 
 	u32 wear = sao->punch(dir, &toolcap, puncher, time_from_last_punch);
 	lua_pushnumber(L, wear);

src/server/luaentity_sao.cpp

@@ -332,16 +332,16 @@ u32 LuaEntitySAO::punch(v3f dir,
 		return 0;
 	}
 
-	FATAL_ERROR_IF(!puncher, "Punch action called without SAO");
-
 	s32 old_hp = getHP();
 	ItemStack selected_item, hand_item;
-	ItemStack tool_item = puncher->getWieldedItem(&selected_item, &hand_item);
+	ItemStack tool_item;
+	if (puncher)
+		tool_item = puncher->getWieldedItem(&selected_item, &hand_item);
 
 	PunchDamageResult result = getPunchDamage(
 			m_armor_groups,
 			toolcap,
-			&tool_item,
+			puncher ? &tool_item : nullptr,
 			time_from_last_punch,
 			initial_wear);
 
@@ -355,12 +355,16 @@ u32 LuaEntitySAO::punch(v3f dir,
 		}
 	}
 
-	actionstream << puncher->getDescription() << " (id=" << puncher->getId() <<
-			", hp=" << puncher->getHP() << ") punched " <<
-			getDescription() << " (id=" << m_id << ", hp=" << m_hp <<
-			"), damage=" << (old_hp - (s32)getHP()) <<
-			(damage_handled ? " (handled by Lua)" : "") << std::endl;
-
+	if (puncher) {
+		actionstream << puncher->getDescription() << " (id=" << puncher->getId() <<
+				", hp=" << puncher->getHP() << ")";
+	} else {
+		actionstream << "(none)";
+	}
+	actionstream << " punched " <<
+		  getDescription() << " (id=" << m_id << ", hp=" << m_hp <<
+		  "), damage=" << (old_hp - (s32)getHP()) <<
+		  (damage_handled ? " (handled by Lua)" : "") << std::endl;
 	// TODO: give Lua control over wear
 	return result.wear;
 }

src/server/player_sao.cpp

@@ -462,11 +462,9 @@ u32 PlayerSAO::punch(v3f dir,
 	if (!toolcap)
 		return 0;
 
-	FATAL_ERROR_IF(!puncher, "Punch action called without SAO");
-
 	// No effect if PvP disabled or if immortal
 	if (isImmortal() || !g_settings->getBool("enable_pvp")) {
-		if (puncher->getType() == ACTIVEOBJECT_TYPE_PLAYER) {
+		if (puncher && puncher->getType() == ACTIVEOBJECT_TYPE_PLAYER) {
 			// create message and add to list
 			sendPunchCommand();
 			return 0;
@@ -493,11 +491,16 @@ u32 PlayerSAO::punch(v3f dir,
 		}
 	}
 
-	actionstream << puncher->getDescription() << " (id=" << puncher->getId() <<
-		", hp=" << puncher->getHP() << ") punched " <<
-		getDescription() << " (id=" << m_id << ", hp=" << m_hp <<
-		"), damage=" << (old_hp - (s32)getHP()) <<
-		(damage_handled ? " (handled by Lua)" : "") << std::endl;
+	if (puncher) {
+		actionstream << puncher->getDescription() << " (id=" << puncher->getId() <<
+				", hp=" << puncher->getHP() << ")";
+	} else {
+		actionstream << "(none)";
+	}
+	actionstream << " puched " <<
+			getDescription() << " (id=" << m_id << ", hp=" << m_hp <<
+			"), damage=" << (old_hp - (s32)getHP()) <<
+			(damage_handled ? " (handled by Lua)" : "") << std::endl;
 
 	return hitparams.wear;
 }

src/tool.cpp

@@ -482,7 +482,7 @@ PunchDamageResult getPunchDamage(
 	{
 		HitParams hitparams = getHitParams(armor_groups, toolcap,
 				time_from_last_punch,
-				punchitem->wear);
+				punchitem ? punchitem->wear : 0);
 		result.did_punch = true;
 		result.wear = hitparams.wear;
 		result.damage = hitparams.hp;