Fix the possibility to put fire in a protected area

[andriyndev]

It's currently possible to put fire in a protected area which is adjacent to a non-protected one. That's because when putting fire with a "Flint and Steel", it only checks the protection of the area in which the pointed node resides, but doesn't check the protection of the area, in which the fire will be actually put in. So, it's possible to put fire in a protected area if the pointed node at the same time resides in a non-protected one.

[tenplus1]

Since the flame will always appear at pointed_thing.above why not remove the protection check for pointed_thing.below ?

[andriyndev]

Since the flame will always appear at pointed_thing.above why not remove the protection check for pointed_thing.below ?

I decided to leave it because despite the fire will appear in a non-protected area, using Flint and Steel can be interpreted as igniting the pointed object, and I think players shouldn't be able to ignite protected objects, at least directly. Maybe it'd be even better to disable the possibility to ignite voxels adjacent to protected ones but it's controversial and requires discussing/consideration, and I think it's unnecessary if the fire won't be able to spread to the protected area.

[appgurueu]

Since the flame will always appear at pointed_thing.above why not remove the protection check for pointed_thing.below ?

Your assumption is unfortunately wrong. If there is an on_ignite method of the pointed node, that will be called. One notable instance of this in MTG is TNT. Boom!
(Furthermore, external mods may also define on_ignite to e.g. swap nodes with burnt / burning variants.)

It's currently possible to put fire in a protected area which is adjacent to a non-protected one.

Indeed, but does it really help to disallow this in flint and steel code? Since fire spreads, I can just light an adjacent node on fire, which will spread to the protected area?

Disallowing setting adjacent nodes on fire more generally doesn't help either: I can just build a "chain" of flammable nodes.

The only "proper" solution I see would be to assign "owners" to flames, but that'd need some discussion and is definitely out of scope for this PR.

Nitpick: You check eagerly whether the "above" node is protected. This means that now, igniting a node in an unprotected area won't be possible if there is a protected area right above, even if no flame is placed in the above node. For example you couldn't ignite TNT in your own area below a foreign protected area.

Currently I don't see a significant benefit in this PR, but I also see no significant harm, so I'm mostly neutral.

[tenplus1]

How about:

if pointed_thing.type == "node" then
	local node_under = minetest.get_node(pointed_thing.under).name
	local nodedef = minetest.registered_nodes[node_under]
	if not nodedef then
		return
	end
	if minetest.is_protected(pointed_thing.under, player_name) then
		return
	end
	if nodedef.on_ignite then
		nodedef.on_ignite(pointed_thing.under, user)
	end
	if minetest.is_protected(pointed_thing.above, player_name) then
		return
	end
	if minetest.get_item_group(node_under, "flammable") >= 1
			and minetest.get_node(pointed_thing.above).name == "air" then
		minetest.set_node(pointed_thing.above, {name = "fire:basic_flame"})
	end
end

[appgurueu]

Moving the check to right before the set_node is the proper way to address my "nitpick". What you suggest would have ignition of the node fall through: Igniting TNT would now also place a flame above the TNT.

[tenplus1]

It does respect protection on both sides though and yes will allow a flame if the node beside the tnt is unprotected which should be fine. This is how I've done it in Fire Redo:

https://codeberg.org/tenplus1/fire/src/branch/master/init.lua#L166

[Emojigit]

I would recommend firing on violation callbacks too, i.e.:

if minetest.is_protected(pointed_thing.under, player_name) then
	minetest.record_protection_violation(pointed_thing.under, player_name)
	return
elseif minetest.is_protected(pointed_thing.above, player_name) then
	minetest.record_protection_violation(pointed_thing.above, player_name)
	return
end

For most protection mods, they do the notification for you. Don't worry about the "This area is protected" message.

[andriyndev]

Done. Thank you!

[andriyndev]

Indeed, but does it really help to disallow this in flint and steel code? Since fire spreads, I can just light an adjacent node on fire, which will spread to the protected area? Disallowing setting adjacent nodes on fire more generally doesn't help either: I can just build a "chain" of flammable nodes.

Yes, it can theoretically spread to the protected area but I think this problem is out of scope of the current patch since the patch addresses the specific behavior of "Flint and Steel" while spreading of fire isn't related to the tool. Also, many server owners disable spreading of fire for understandable reasons which will prevent fire enter a protected area through spreading, while this bug will still allow to place fire in an actual protected area.

The only "proper" solution I see would be to assign "owners" to flames, but that'd need some discussion and is definitely out of scope for this PR.

As another solution - don't allow fire to cross the boundary from an unprotected to a protected area, or between two protected areas owned by different players, however it's controversial and requires additional discussion (for example, it could allow to cheat by protecting area around uncontrolled spreading fire). Also, it's unclear if it can be considered a bug at all, and players which play on servers where fire can spread should probably take it into account when building and protecting area.

Nitpick: You check eagerly whether the "above" node is protected. This means that now, igniting a node in an unprotected area won't be possible if there is a protected area right above, even if no flame is placed in the above node. For example you couldn't ignite TNT in your own area below a foreign protected area.

The above and under fields of pointed_thing aren't relative to the world coordinates but to the pointed face, and above means in front of the pointed face (where the fire will appear) while under means behind the pointed face (the pointed node itself).

[andriyndev]

How about:

if pointed_thing.type == "node" then
	local node_under = minetest.get_node(pointed_thing.under).name
	local nodedef = minetest.registered_nodes[node_under]
	if not nodedef then
		return
	end
	if minetest.is_protected(pointed_thing.under, player_name) then
		return
	end
	if nodedef.on_ignite then
		nodedef.on_ignite(pointed_thing.under, user)
	end
	if minetest.is_protected(pointed_thing.above, player_name) then
		return
	end
	if minetest.get_item_group(node_under, "flammable") >= 1
			and minetest.get_node(pointed_thing.above).name == "air" then
		minetest.set_node(pointed_thing.above, {name = "fire:basic_flame"})
	end
end

Thank you, I'll move the check to the latest condition since we don't need it when calling on_ignite().

[andriyndev]

• Fixed the patch for using on_ignite()
• Replaced printing to chat with calling minetest.record_protection_violation()

[appgurueu]

Looks good to me