Clarify that punch toolcaps are not optional (#16720)

doc/lua_api.md

@@ -2593,9 +2593,7 @@ Damage calculation:
 
 Client predicts damage based on damage groups. Because of this, it is able to
 give an immediate response when an entity is damaged or dies; the response is
-pre-defined somehow (e.g. by defining a sprite animation) (not implemented;
-TODO).
-Currently a smoke puff will appear when an entity dies.
+pre-defined and a smoke puff will appear when an entity dies.
 
 The group `immortal` completely disables normal damage.
 
@@ -2610,18 +2608,10 @@ entity:on_punch(puncher, time_from_last_punch, tool_capabilities, direction,
                 damage)
 ```
 
-This should never be called directly, because damage is usually not handled by
-the entity itself.
+This should **never** be called directly, because damage is usually not handled
+by the entity itself.
 
-* `puncher` is the object performing the punch. Can be `nil`. Should never be
-  accessed unless absolutely required, to encourage interoperability.
-* `time_from_last_punch` is time from last punch (by `puncher`) or `nil`.
-* `tool_capabilities` can be `nil`.
-* `direction` is a unit vector, pointing from the source of the punch to
-   the punched object.
-* `damage` damage that will be done to entity
-Return value of this function will determine if damage is done by this function
-(retval true) or shall be done by engine (retval false)
+(see "Registered entities" section for detailed description)
 
 To punch an entity/object in Lua, call:
 
@@ -2629,12 +2619,7 @@ To punch an entity/object in Lua, call:
 object:punch(puncher, time_from_last_punch, tool_capabilities, direction)
 ```
 
-* Return value is tool wear.
-* Parameters are equal to the above callback.
-* If `direction` equals `nil` and `puncher` does not equal `nil`, `direction`
-  will be automatically filled in based on the location of `puncher`.
-
-
+(see "`ObjectRef`" section for detailed description)
 
 
 Metadata
@@ -4307,13 +4292,13 @@ Helper functions
 * `core.pointed_thing_to_face_pos(placer, pointed_thing)`: returns a
   position.
     * returns the exact position on the surface of a pointed node
-* `core.get_tool_wear_after_use(uses [, initial_wear])`
+* `core.get_tool_wear_after_use(uses, initial_wear)`
     * Simulates a tool being used once and returns the added wear,
       such that, if only this function is used to calculate wear,
       the tool will break exactly after `uses` times of uses
     * `uses`: Number of times the tool can be used
     * `initial_wear`: The initial wear the tool starts with (default: 0)
-* `core.get_dig_params(groups, tool_capabilities [, wear])`:
+* `core.get_dig_params(groups, tool_capabilities, wear)`:
     Simulates an item that digs a node.
     Returns a table with the following fields:
     * `diggable`: `true` if node can be dug, `false` otherwise.
@@ -4324,7 +4309,7 @@ Helper functions
     * `groups`: Table of the node groups of the node that would be dug
     * `tool_capabilities`: Tool capabilities table of the item
     * `wear`: Amount of wear the tool starts with (default: 0)
-* `core.get_hit_params(groups, tool_capabilities [, time_from_last_punch [, wear]])`:
+* `core.get_hit_params(groups, tool_capabilities, time_from_last_punch, wear)`:
     Simulates an item that punches an object.
     Returns a table with the following fields:
     * `hp`: How much damage the punch would cause (between -65535 and 65535).
@@ -4332,12 +4317,10 @@ Helper functions
     Parameters:
     * `groups`: Damage groups of the object
     * `tool_capabilities`: Tool capabilities table of the item
-    * `time_from_last_punch`: time in seconds since last punch action
+    * `time_from_last_punch`: time in seconds since last punch action (can be `nil`)
     * `wear`: Amount of wear the item starts with (default: 0)
 
 
-
-
 Translations
 ============
 
@@ -5480,7 +5463,7 @@ Callbacks:
     * `puncher`: an `ObjectRef` (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`)
+    * `tool_capabilities`: capability table of used item
     * `dir`: unit vector of direction of punch. Always defined. Points from the
       puncher to the punched.
     * `damage`: damage that will be done to entity.
@@ -8478,11 +8461,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 or `nil`
-    * `dir`: direction vector of punch
-    * Other arguments: See `on_punch` for entities
-    * Arguments `time_from_last_punch`, `tool_capabilities`, and `dir`
-      will be replaced with a default value when the caller sets them to `nil`.
+    * `puncher`: another `ObjectRef` which punched the object (can be `nil`)
+    * `time_from_last_punch`: Meant for disallowing spamming of clicks
+      (can be `nil`)
+    * `tool_capabilities`: capability table of used item
+    * `dir`: direction vector. Points from the puncher to the punched (can be `nil`)
 * `right_click(clicker)`:
     * simulates using the 'place/use' key on the object
     * triggers all consequences as if a real player had done this

src/client/content_cao.cpp

@@ -1791,8 +1791,8 @@ bool GenericCAO::directReportPunch(v3f dir, const ItemStack *punchitem,
 	const ItemStack *hand_item, float time_from_last_punch)
 {
 	assert(punchitem);	// pre-condition
-	const ToolCapabilities *toolcap =
-			&punchitem->getToolCapabilities(m_client->idef(), hand_item);
+	const ToolCapabilities &toolcap =
+			punchitem->getToolCapabilities(m_client->idef(), hand_item);
 	PunchDamageResult result = getPunchDamage(
 			m_armor_groups,
 			toolcap,

src/network/serverpackethandler.cpp

@@ -1046,7 +1046,7 @@ void Server::handleCommand_Interact(NetworkPacket *pkt)
 
 		ItemStack selected_item, hand_item;
 		ItemStack tool_item = playersao->getWieldedItem(&selected_item, &hand_item);
-		ToolCapabilities toolcap =
+		const ToolCapabilities &toolcap =
 				tool_item.getToolCapabilities(m_itemdef, &hand_item);
 		v3f dir = (pointed_object->getBasePosition() -
 				(playersao->getBasePosition() + playersao->getEyeOffset())
@@ -1054,7 +1054,7 @@ void Server::handleCommand_Interact(NetworkPacket *pkt)
 		float time_from_last_punch =
 			playersao->resetTimeFromLastPunch();
 
-		u32 wear = pointed_object->punch(dir, &toolcap, playersao,
+		u32 wear = pointed_object->punch(dir, toolcap, playersao,
 				time_from_last_punch, tool_item.wear);
 
 		// Callback may have changed item, so get it again

src/script/common/c_content.cpp

@@ -1665,9 +1665,11 @@ ToolCapabilities read_tool_capabilities(
 		lua_State *L, int table)
 {
 	ToolCapabilities toolcap;
+	luaL_checktype(L, table, LUA_TTABLE);
 	getfloatfield(L, table, "full_punch_interval", toolcap.full_punch_interval);
 	getintfield(L, table, "max_drop_level", toolcap.max_drop_level);
 	getintfield(L, table, "punch_attack_uses", toolcap.punch_attack_uses);
+
 	lua_getfield(L, table, "groupcaps");
 	if(lua_istable(L, -1)){
 		int table_groupcaps = lua_gettop(L);
@@ -1686,7 +1688,7 @@ ToolCapabilities read_tool_capabilities(
 				float maxwear = 0;
 				if (getfloatfield(L, table_groupcap, "maxwear", maxwear)){
 					if (maxwear != 0)
-						groupcap.uses = 1.0/maxwear;
+						groupcap.uses = 1.0f/maxwear;
 					else
 						groupcap.uses = 0;
 					warningstream << "Field \"maxwear\" is deprecated; "
@@ -1748,7 +1750,7 @@ PointabilityType read_pointability_type(lua_State *L, int index)
 			return PointabilityType::POINTABLE_BLOCKING;
 		}
 	}
-	throw LuaError("Invalid pointable type.");
+	throw LuaError("Invalid pointable type");
 }
 
 /******************************************************************************/
@@ -1756,6 +1758,7 @@ Pointabilities read_pointabilities(lua_State *L, int index)
 {
 	Pointabilities pointabilities;
 
+	luaL_checktype(L, index, LUA_TTABLE);
 	lua_getfield(L, index, "nodes");
 	if(lua_istable(L, -1)){
 		int ti = lua_gettop(L);
@@ -1815,6 +1818,9 @@ void push_pointability_type(lua_State *L, PointabilityType pointable)
 	case PointabilityType::POINTABLE_BLOCKING:
 		lua_pushliteral(L, "blocking");
 		break;
+	default:
+		assert(false);
+		break;
 	}
 }
 

src/script/cpp_api/s_entity.cpp

@@ -247,7 +247,7 @@ void ScriptApiEntity::luaentity_Step(u16 id, float dtime,
 //                       tool_capabilities, direction, damage)
 bool ScriptApiEntity::luaentity_Punch(u16 id,
 		ServerActiveObject *puncher, float time_from_last_punch,
-		const ToolCapabilities *toolcap, v3f dir, s32 damage)
+		const ToolCapabilities &toolcap, v3f dir, s32 damage)
 {
 	SCRIPTAPI_PRECHECKHEADER
 
@@ -270,7 +270,7 @@ bool ScriptApiEntity::luaentity_Punch(u16 id,
 	else
 		lua_pushnil(L);
 	lua_pushnumber(L, time_from_last_punch);
-	push_tool_capabilities(L, *toolcap);
+	push_tool_capabilities(L, toolcap);
 	push_v3f(L, dir);
 	lua_pushnumber(L, damage);
 

src/script/cpp_api/s_entity.h

@@ -28,7 +28,7 @@ public:
 		const collisionMoveResult *moveresult);
 	bool luaentity_Punch(u16 id,
 			ServerActiveObject *puncher, float time_from_last_punch,
-			const ToolCapabilities *toolcap, v3f dir, s32 damage);
+			const ToolCapabilities &toolcap, v3f dir, s32 damage);
 	bool luaentity_on_death(u16 id, ServerActiveObject *killer);
 	void luaentity_Rightclick(u16 id, ServerActiveObject *clicker);
 	void luaentity_on_attach_child(u16 id, ServerActiveObject *child);

src/script/cpp_api/s_player.cpp

@@ -42,7 +42,7 @@ void ScriptApiPlayer::on_dieplayer(ServerActiveObject *player, const PlayerHPCha
 bool ScriptApiPlayer::on_punchplayer(ServerActiveObject *player,
 		ServerActiveObject *hitter,
 		float time_from_last_punch,
-		const ToolCapabilities *toolcap,
+		const ToolCapabilities &toolcap,
 		v3f dir,
 		s32 damage)
 {
@@ -57,7 +57,7 @@ bool ScriptApiPlayer::on_punchplayer(ServerActiveObject *player,
 	else
 		lua_pushnil(L);
 	lua_pushnumber(L, time_from_last_punch);
-	push_tool_capabilities(L, *toolcap);
+	push_tool_capabilities(L, toolcap);
 	push_v3f(L, dir);
 	lua_pushnumber(L, damage);
 	runCallbacks(6, RUN_CALLBACKS_MODE_OR);

src/script/cpp_api/s_player.h

@@ -30,7 +30,7 @@ public:
 	void on_leaveplayer(ServerActiveObject *player, bool timeout);
 	void on_cheat(ServerActiveObject *player, const std::string &cheat_type);
 	bool on_punchplayer(ServerActiveObject *player, ServerActiveObject *hitter,
-			float time_from_last_punch, const ToolCapabilities *toolcap,
+			float time_from_last_punch, const ToolCapabilities &toolcap,
 			v3f dir, s32 damage);
 	void on_rightclickplayer(ServerActiveObject *player, ServerActiveObject *clicker);
 	s32 on_player_hpchange(ServerActiveObject *player, s32 hp_change,

src/script/lua_api/l_object.cpp

@@ -204,7 +204,7 @@ int ObjectRef::l_punch(lua_State *L)
 		return 0;
 
 	float time_from_last_punch = readParam<float>(L, 3, 1000000.0f);
-	ToolCapabilities toolcap = read_tool_capabilities(L, 4);
+	ToolCapabilities toolcap = read_tool_capabilities(L, 4); // not optional!
 	v3f dir;
 	if (puncher) {
 		dir = readParam<v3f>(L, 5, sao->getBasePosition() - puncher->getBasePosition());
@@ -213,7 +213,7 @@ int ObjectRef::l_punch(lua_State *L)
 		dir = readParam<v3f>(L, 5, v3f(0));
 	}
 
-	u32 wear = sao->punch(dir, &toolcap, puncher, time_from_last_punch);
+	u32 wear = sao->punch(dir, toolcap, puncher, time_from_last_punch);
 	lua_pushnumber(L, wear);
 
 	return 1;

src/script/lua_api/l_util.cpp

@@ -195,12 +195,12 @@ int ModApiUtil::l_get_dig_params(lua_State *L)
 int ModApiUtil::l_get_hit_params(lua_State *L)
 {
 	NO_MAP_LOCK_REQUIRED;
-	std::unordered_map<std::string, int> groups;
+	ItemGroupList groups;
 	read_groups(L, 1, groups);
 	ToolCapabilities tp = read_tool_capabilities(L, 2);
 	float time_from_last_punch = readParam<float>(L, 3, 1000000);
 	int wear = readParam<int>(L, 4, 0);
-	push_hit_params(L, getHitParams(groups, &tp,
+	push_hit_params(L, getHitParams(groups, tp,
 		time_from_last_punch, wear));
 	return 1;
 }

src/server/luaentity_sao.cpp

@@ -323,7 +323,7 @@ void LuaEntitySAO::getStaticData(std::string *result) const
 }
 
 u32 LuaEntitySAO::punch(v3f dir,
-		const ToolCapabilities *toolcap,
+		const ToolCapabilities &toolcap,
 		ServerActiveObject *puncher,
 		float time_from_last_punch,
 		u16 initial_wear)

src/server/luaentity_sao.h

@@ -29,7 +29,7 @@ public:
 	bool shouldUnload() const { return true; }
 	void getStaticData(std::string *result) const;
 
-	u32 punch(v3f dir, const ToolCapabilities *toolcap = nullptr,
+	u32 punch(v3f dir, const ToolCapabilities &toolcap,
 			ServerActiveObject *puncher = nullptr,
 			float time_from_last_punch = 1000000.0f,
 			u16 initial_wear = 0);

src/server/player_sao.cpp

@@ -450,14 +450,11 @@ void PlayerSAO::setLookPitchAndSend(const float pitch)
 }
 
 u32 PlayerSAO::punch(v3f dir,
-	const ToolCapabilities *toolcap,
+	const ToolCapabilities &toolcap,
 	ServerActiveObject *puncher,
 	float time_from_last_punch,
 	u16 initial_wear)
 {
-	if (!toolcap)
-		return 0;
-
 	// No effect if PvP disabled or if immortal
 	if (isImmortal() || !g_settings->getBool("enable_pvp")) {
 		if (puncher && puncher->getType() == ACTIVEOBJECT_TYPE_PLAYER) {

src/server/player_sao.h

@@ -101,7 +101,7 @@ public:
 		Interaction interface
 	*/
 
-	u32 punch(v3f dir, const ToolCapabilities *toolcap, ServerActiveObject *puncher,
+	u32 punch(v3f dir, const ToolCapabilities &toolcap, ServerActiveObject *puncher,
 			float time_from_last_punch, u16 initial_wear = 0) override;
 	void rightClick(ServerActiveObject *clicker) override;
 	void setHP(s32 hp, const PlayerHPChangeReason &reason) override
@@ -141,7 +141,7 @@ public:
 	float resetTimeFromLastPunch()
 	{
 		float r = m_time_from_last_punch;
-		m_time_from_last_punch = 0.0;
+		m_time_from_last_punch = 0;
 		return r;
 	}
 	void noCheatDigStart(const v3s16 &p)

src/server/serveractiveobject.h

@@ -129,7 +129,7 @@ public:
 
 	// Returns added tool wear
 	virtual u32 punch(v3f dir,
-			const ToolCapabilities *toolcap = nullptr,
+			const ToolCapabilities &toolcap,
 			ServerActiveObject *puncher = nullptr,
 			float time_from_last_punch = 1000000.0f,
 			u16 initial_wear = 0)

src/tool.cpp

@@ -421,21 +421,21 @@ DigParams getDigParams(const ItemGroupList &groups,
 }
 
 HitParams getHitParams(const ItemGroupList &armor_groups,
-		const ToolCapabilities *tp, float time_from_last_punch,
+		const ToolCapabilities &tp, float time_from_last_punch,
 		u16 initial_wear)
 {
 	s32 damage = 0;
 	float result_wear = 0.0f;
 	float punch_interval_multiplier =
-			rangelim(time_from_last_punch / tp->full_punch_interval, 0.0f, 1.0f);
+			rangelim(time_from_last_punch / tp.full_punch_interval, 0.0f, 1.0f);
 
-	for (const auto &damageGroup : tp->damageGroups) {
+	for (const auto &damageGroup : tp.damageGroups) {
 		s16 armor = itemgroup_get(armor_groups, damageGroup.first);
 		damage += damageGroup.second * punch_interval_multiplier * armor / 100.0;
 	}
 
-	if (tp->punch_attack_uses > 0) {
-		result_wear = calculateResultWear(tp->punch_attack_uses, initial_wear);
+	if (tp.punch_attack_uses > 0) {
+		result_wear = calculateResultWear(tp.punch_attack_uses, initial_wear);
 		result_wear *= punch_interval_multiplier;
 	}
 	// Keep damage in sane bounds for simplicity
@@ -445,36 +445,30 @@ HitParams getHitParams(const ItemGroupList &armor_groups,
 	return {damage, wear_i};
 }
 
-HitParams getHitParams(const ItemGroupList &armor_groups,
-		const ToolCapabilities *tp)
-{
-	return getHitParams(armor_groups, tp, 1000000);
-}
-
 PunchDamageResult getPunchDamage(
 		const ItemGroupList &armor_groups,
-		const ToolCapabilities *toolcap,
+		const ToolCapabilities &toolcap,
 		const ItemStack *punchitem,
 		float time_from_last_punch,
 		u16 initial_wear
 ){
 	bool do_hit = true;
-	{
-		if (do_hit && punchitem) {
-			if (itemgroup_get(armor_groups, "punch_operable") &&
-					(toolcap == NULL || punchitem->name.empty()))
-				do_hit = false;
-		}
+	if (do_hit && punchitem) {
+		// FIXME: punch_operable is supposed to apply to "non-tool" items too
+		// 1. We don't have the itemdef available here to check
+		// 2. how is this supposed to interact with overridable toolcaps?
+		if (itemgroup_get(armor_groups, "punch_operable") &&
+				punchitem->name.empty())
+			do_hit = false;
+	}
 
-		if (do_hit) {
-			if(itemgroup_get(armor_groups, "immortal"))
-				do_hit = false;
-		}
+	if (do_hit) {
+		if (itemgroup_get(armor_groups, "immortal"))
+			do_hit = false;
 	}
 
 	PunchDamageResult result;
-	if(do_hit)
-	{
+	if (do_hit) {
 		HitParams hitparams = getHitParams(armor_groups, toolcap,
 				time_from_last_punch,
 				punchitem ? punchitem->wear : 0);

src/tool.h

@@ -149,11 +149,14 @@ struct HitParams
 };
 
 HitParams getHitParams(const ItemGroupList &armor_groups,
-		const ToolCapabilities *tp, float time_from_last_punch,
+		const ToolCapabilities &tp, float time_from_last_punch,
 		u16 initial_wear = 0);
 
-HitParams getHitParams(const ItemGroupList &armor_groups,
-		const ToolCapabilities *tp);
+inline HitParams getHitParams(const ItemGroupList &armor_groups,
+		const ToolCapabilities &tp)
+{
+	return getHitParams(armor_groups, tp, 1000000);
+}
 
 struct PunchDamageResult
 {
@@ -168,12 +171,13 @@ struct ItemStack;
 
 PunchDamageResult getPunchDamage(
 		const ItemGroupList &armor_groups,
-		const ToolCapabilities *toolcap,
+		const ToolCapabilities &toolcap,
 		const ItemStack *punchitem,
 		float time_from_last_punch,
 		u16 initial_wear = 0
 );
 
 u32 calculateResultWear(const u32 uses, const u16 initial_wear);
+
 f32 getToolRange(const ItemStack &wielded_item, const ItemStack &hand_item,
 		const IItemDefManager *itemdef_manager);