Fix protection regarding tracks (track_builder and railway_operator) and documentation on privileges

advtrains/init.lua

@@ -11,7 +11,7 @@ end
 
 --advtrains
 
-DUMP_DEBUG_SAVE = true
+DUMP_DEBUG_SAVE = false
 
 --Constant for maximum connection value/division of the circle
 AT_CMAX = 16

advtrains/protection.lua

@@ -61,50 +61,96 @@ Wagon coupling:
 
 local boo = minetest.settings:get_bool("advtrains_allow_build_to_owner")
 
-
+-- temporarily prevent scanning for neighboring rail nodes recursively
 local nocheck
+
+local old_is_protected = minetest.is_protected
+
 -- Check if the node we are about to check is in the range of a track that is protected from a player
---WARN: true means here that the action is forbidden!
-function advtrains.check_track_protection(pos, pname)
+minetest.is_protected = function(pos, pname)
+	
+	-- old_is_protected:
+	-- If an earlier callback decided that pos is protected, we wouldn't have been called
+	-- if a later callback decides it, get that here.
+	-- this effectively puts this function into a final-choice position
+	local oprot = old_is_protected(pos, pname)
+	if oprot then
+		return true
+	end
+	
 	if nocheck or pname=="" then
 		return false
 	end
-	nocheck=true --prevent recursive calls, never check this again if we're already in
+	
+	-- Special exception: to allow seamless rail connections between 2 separately protected
+	-- networks, rails itself are not affected by the radius setting. So, if the node here is
+	-- a rail, we skip the check and just use check_track_protection on same pos.
+	local node = minetest.get_node(pos)
+	if minetest.get_item_group(node.name, "advtrains_track") > 0 then
+		-- by here, we know that no other protection callback has this protected, we can safely pass "false".
+		-- hope this doesn't lead to bugs!
+		return not advtrains.check_track_protection(pos, pname, nil, false)
+	end
+	
 	local r, vr = 1, 3
 	local nodes = minetest.find_nodes_in_area(
 		{x = pos.x - r, y = pos.y - vr, z = pos.z - r},
-		{x = pos.x + r, y = pos.y, z = pos.z + r},
+		{x = pos.x + r, y = pos.y + 1, z = pos.z + r},
 		{"group:advtrains_track"})
 	for _,npos in ipairs(nodes) do
-		if not minetest.check_player_privs(pname, {track_builder = true}) then
-			if boo and not minetest.is_protected(npos, pname) and minetest.is_protected(npos, "*dummy*") then
-				nocheck = false
-				return false
-			else
-				minetest.chat_send_player(pname, "You are not allowed to dig or place nodes near tracks (missing track_builder privilege)")
-				minetest.log("action", pname.." tried to dig/place nodes near the track at "..minetest.pos_to_string(npos).." but does not have track_builder")
-				nocheck = false
+		if not advtrains.check_track_protection(npos, pname, pos) then
 			return true
 		end
 	end
-		if not minetest.check_player_privs(pname, {protection_bypass = true}) then
-			if minetest.is_protected(npos, pname) then
-				nocheck = false
-				minetest.record_protection_violation(pos, pname)
-				return true
-			end
-		end
-	end
 	nocheck=false
 	return false
 end
 
-local old_is_protected = minetest.is_protected
-minetest.is_protected = function(pos, pname)
-	if advtrains.check_track_protection(pos, pname) then
+-- Check whether the player is permitted to modify this track
+-- Shall be called only for nodes that are or are about to become tracks.
+-- The range check from is_track_near_protected is disabled here.
+-- this splits in 1. track_builder privilege and 2. is_protected
+-- also respects the allow_build_to_owner property.
+--WARN: true means here that the action is allowed!
+function advtrains.check_track_protection(pos, pname, near, prot_p)
+	-- Parameter "near" is an optional position, the original node that the player
+	-- was about to affect, while "pos" represents the checked rail node
+	-- if "near" is not set, pos is the same node.
+	local nears = near and "near " or ""
+	local apos = near or pos
+	
+	-- note that having protection_bypass implicitly implies having track_builder, because else it would be possible to dig rails
+	-- (only checked by is_protected, which is not respected) but not place them.
+	-- We won't impose restrictions on protection_bypass owners.
+	if minetest.check_player_privs(pname, {protection_bypass = true}) then
 		return true
 	end
-	return old_is_protected(pos, pname)
+	
+	nocheck = true
+	local priv = minetest.check_player_privs(pname, {track_builder = true})
+	
+	-- note: is_protected above already checks the is_protected value against the current player, so checking it again is useless.
+	local prot = prot_p
+	if prot==nil then
+		 prot = advtrains.is_protected(pos, pname)
+	end
+	local dprot = minetest.is_protected(pos, "*dummy*")
+	nocheck = false
+	
+	--atdebug("CTP: ",pos,pname,near,prot_p,"priv=",priv,"prot=",prot,"dprot=",dprot)
+	
+	if not priv and (not boo or prot or not dprot) then
+		minetest.chat_send_player(pname, "You are not allowed to build "..nears.."tracks without track_builder privilege")
+		minetest.log("action", pname.." tried to modify terrain "..nears.."track at "..minetest.pos_to_string(apos).." but is not permitted to (no privilege)")
+		return false
+	end
+	if prot then
+		minetest.chat_send_player(pname, "You are not allowed to build "..nears.."tracks at protected position!")
+		minetest.record_protection_violation(pos, pname)
+		minetest.log("action", pname.." tried to modify "..nears.."track at "..minetest.pos_to_string(apos).." but position is protected!")
+		return false
+	end
+	return true
 end
 
 --WARN: true means here that the action is allowed!
@@ -126,7 +172,7 @@ end
 function advtrains.check_turnout_signal_protection(pos, pname)
 	nocheck=true
 	if not minetest.check_player_privs(pname, {railway_operator = true}) then
-		if boo and not minetest.is_protected(pos, pname) and minetest.is_protected(pos, "*dummy*") then
+		if boo and not advtrains.is_protected(pos, pname) and minetest.is_protected(pos, "*dummy*") then
 			nocheck=false
 			return true
 		else
@@ -136,13 +182,11 @@ function advtrains.check_turnout_signal_protection(pos, pname)
 			return false
 		end
 	end
-	if not minetest.check_player_privs(pname, {protection_bypass = true}) then
-		if minetest.is_protected(pos, pname) then
+	if advtrains.is_protected(pos, pname) then
 		minetest.record_protection_violation(pos, pname)
 		nocheck=false
 		return false
 	end
-	end
 	nocheck=false
 	return true
 end

advtrains/trackplacer.lua

@@ -277,8 +277,7 @@ function tp.register_track_placer(nnprefix, imgprefix, dispname)
 				if pointed_thing.type=="node" then
 					local pos=pointed_thing.above
 					local upos=vector.subtract(pointed_thing.above, {x=0, y=1, z=0})
-					if advtrains.is_protected(pos,name) then
-						minetest.record_protection_violation(pos, name)
+					if not advtrains.check_track_protection(pos, name) then
 						return itemstack, false
 					end
 					if minetest.registered_nodes[minetest.get_node(pos).name] and minetest.registered_nodes[minetest.get_node(pos).name].buildable_to
@@ -286,7 +285,7 @@ function tp.register_track_placer(nnprefix, imgprefix, dispname)
 --						minetest.chat_send_all(nnprefix)
 						local yaw = placer:get_look_horizontal()
 						tp.placetrack(pos, nnprefix, placer, itemstack, pointed_thing, yaw)
-						if not minetest.settings:get_bool("creative_mode") then
+						if not advtrains.is_creative(name) then
 							itemstack:take_item()
 						end
 					end
@@ -314,8 +313,7 @@ minetest.register_craftitem("advtrains:trackworker",{
 			local has_aux1_down = placer:get_player_control().aux1
 			if pointed_thing.type=="node" then
 				local pos=pointed_thing.under
-				if advtrains.is_protected(pos, name) then
-					minetest.record_protection_violation(pos, name)
+				if not advtrains.check_track_protection(pos, name) then
 					return
 				end
 				local node=minetest.get_node(pos)
@@ -369,8 +367,7 @@ minetest.register_craftitem("advtrains:trackworker",{
 			if pointed_thing.type=="node" then
 				local pos=pointed_thing.under
 				local node=minetest.get_node(pos)
-				if advtrains.is_protected(pos, name) then
-					minetest.record_protection_violation(pos, name)
+				if not advtrains.check_track_protection(pos, name) then
 					return
 				end
 				

privilege_guide.txt

@@ -2,6 +2,7 @@
 ### Advtrains Privilege Guide
 All privileges are automatically granted to singleplayer, but for
 multiplayer servers this might be interesting.
+In this document, "protected from" means the player does NOT have access to the area, while "protected by" means the player has (semi-)exclusive access to the area.
 There are 3 groups of privileges introduced by advtrains:
 
 ## Trains
@@ -17,10 +18,10 @@ destroy any train.
 The area 1 node around and 4 nodes up from each track node is protected.
 Players that don't have the 'track_builder' privilege can not build or
 dig (or modify) anything inside this area.
-If any player tries to modify anything that is in the area of a track
-node and this track node is protected from him, he also can not do this.
+If a player has this privilege and tries to modify anything that is in the area of a track node which is protected from him, he also can not do this.
 (that said, while checking protection, the area around a track is
 treated as the track node itself)
+Note that having 'protection_bypass' automatically implies 'track_builder' due to internal engine mechanics. (see comments in source code)
 
 ## Turnouts and Signals*
 Players without the 'railway_operator' privilege can not operate signals
@@ -31,3 +32,12 @@ an exception applies to players missing the required privileges when
 they are in a protected area that they have access to. Whether the
 area is protected from others is checked by checking for protection
 against a dummy player called '*dummy*'
+
+## Privileges of extensions:
+
+* atlatc:
+This privilege allows to create and modify LUA code in LuaATC rails added by the advtrains_luaautomation mod, as well as to create and manage code environments.
+
+* interlocking:
+This privilege allows to build, set up, configure and control all sorts of interlocking equipment.
+Players without this privilege are still allowed to set and cancel routes (under the condition that they have train_operator).