Check circular area hierarchy on startup

This also prevents 'areas:isAreaOwner' from getting stuck in an infinite loop.

README.md

@@ -101,10 +101,10 @@ Commands
   * `/area_pos {set,set1,set2,get}` -- Sets the area positions by punching
 	nodes or shows the current area positions.
 
-  * `/area_pos1 [X,Y,Z|X Y Z]` -- Sets area position one to your position or
+  * `/area_pos1 [X,Y,Z|X Y Z|X, Y, Z]` -- Sets area position one to your position or
 	the one supplied.
 
-  * `/area_pos2 [X,Y,Z|X Y Z]` -- Sets area position two to your position or
+  * `/area_pos2 [X,Y,Z|X Y Z|X, Y, Z]` -- Sets area position two to your position or
 	the one supplied.
 
   * `/areas_cleanup` -- Removes all ownerless areas.

api.lua

@@ -1,24 +1,5 @@
 local hudHandlers = {}
 
----plants to place in openfarming
-local plants = {
-	["farming:beetroot"]="air", ["farming:blueberries"]="air", ["farming:cabbage"]="air",
-	["farming:carrot"]="air", ["farming:chili_pepper"]="air", ["farming:coffee_beans"]="air",
-	["farming:corn"]="air", ["farming:cucumber"]="air", ["farming:garlic_clove"]="air",
-	["farming:melon_slice"]="air", ["farming:onion"]="air", ["default:papyrus"]="air",
-	["farming:pea_pod"]="air",	["farming:peppercorn"]="air", ["farming:pineapple_top"]="air",
-	["farming:potato"]="air", ["farming:pumpkin_slice"]="air", ["farming:raspberries"]="air",
-	["farming:rhubarb"]="air",
-	["farming:seed_barley"]="air", ["farming:seed_cotton"]="air", ["farming:seed_hemp"]="air",
-	["farming:seed_mint"]="air", ["farming:seed_oat"]="air", ["farming:seed_rice"]="air",
-	["farming:seed_rye"]="air", ["farming:seed_wheat"]="air",
-	["farming:tomato"]="air",
-	["farming:trellis"]="air", ["farming:grapes"]="farming:trellis",
-	["farming:beanpole"]="air", ["farming:beans"]="farming:beanpole",
-	["morefarming:seed_wildcarrot"]="air", ["morefarming:seed_teosinte"]="air",
-	["morefarming:seed_carrot"]="air", ["morefarming:seed_corn"]="air",
-}
-
 areas.registered_protection_conditions = {}
 areas.registered_on_adds = {}
 areas.registered_on_removes = {}
@@ -132,7 +113,7 @@ function areas:getSmallestAreaAtPos(pos)
 	return smallest_area, smallest_id
 end
 
--- Checks if the area is unprotected, open[farming], owned by player
+-- Checks if the area is unprotected, open, owned by player
 -- or player is part of faction of [smallest] area at position.
 function areas:canInteract(pos, name)
 	if minetest.check_player_privs(name, self.adminPrivs) then
@@ -146,26 +127,10 @@ function areas:canInteract(pos, name)
 		areas_list = self:getAreasAtPos(pos)
 	end
 	local owned = false
-	local player = minetest.get_player_by_name(name)
-	local node = minetest.get_node(pos).name
 	for _, area in pairs(areas_list) do
 		-- Player owns the area or area is open
 		if area.owner == name or area.open then
 			return true
-		elseif area.openfarming then -- If area is openfarming
-			if player and minetest.registered_nodes[node] then
-				local wstack = player:get_wielded_item():get_name()
-				if wstack == "" then wstack = "hand" end
-				-- on_dig
-				if minetest.get_item_group(node, "plant") == 1
-					and (wstack == "hand" or minetest.registered_tools[wstack]) then
-					return true
-				end
-				-- on_place
-				if plants[wstack] ~= nil and plants[wstack] == node then
-					return true
-				end
-			end
 		elseif areas.factions_available and area.faction_open then
 			if (factions.version or 0) < 2 then
 				local faction_name = factions.get_player_faction(name)

chatcommands.lua

@@ -317,28 +317,6 @@ minetest.register_chatcommand("area_open", {
 })
 
 
-minetest.register_chatcommand(
-	"area_openfarming", {
-		params = "<ID>",
-		description = "Toggle an area as open farming (anyone can harvest and plant) or closed",
-		func = function(name, param)
-			local id = tonumber(param)
-			if not id then
-				return false, "Invalid usage, see /help area_openfarming."
-			end
-			
-			if not areas:isAreaOwner(id, name) then
-				return false, "Area "..id.." does not exist"
-					.." or is not owned by you."
-			end
-			local open = not areas.areas[id].openfarming
-			-- Save false as nil to avoid inflating the DB.
-			areas.areas[id].openfarming = open or nil
-			areas:save()
-			return true, ("Area %s to farming."):format(open and "opened" or "closed")
-		end
-})
-
 if areas.factions_available then
 	minetest.register_chatcommand("area_faction_open", {
 		params = S("<ID> [faction_name]"),
@@ -448,9 +426,7 @@ minetest.register_chatcommand("area_info", {
 		elseif has_high_limit then
 			table.insert(lines,
 				S("You have extended area protection"..
-					  " limits (\"areas_high_limit\" privilege)."))
-		elseif privs.megabuilder then
-			table.insert(lines, "You are a megabuilder (\"megabuilder\" privilege).")
+				" limits (\"areas_high_limit\" privilege)."))
 		end
 
 		-- Area count
@@ -463,7 +439,7 @@ minetest.register_chatcommand("area_info", {
 		table.insert(lines, S("You have @1 areas.", area_num))
 
 		-- Area limit
-		local area_limit_line = (privs.areas or privs.megabuilder) and
+		local area_limit_line = privs.areas and
 			S("Limit: no area count limit") or
 			S("Limit: @1 areas", max_count)
 		table.insert(lines, area_limit_line)
@@ -484,9 +460,6 @@ minetest.register_chatcommand("area_info", {
 					limit, size_limit)
 				priv_limit_info("areas_high_limit",
 					limit_high, size_limit_high)
-				table.insert(lines, "Players with the \"megabuilder\" privilege can protect unlimited areas in size and number.")
-			elseif privs.megabuilder then
-				table.insert(lines, "You can protect areas unlimited in size and number.")
 			elseif has_prot_priv then
 				size_info(S("You can protect areas"), max_size)
 			end

hud.lua

@@ -44,8 +44,8 @@ minetest.register_globalstep(function(dtime)
 
 			table.insert(areaStrings, ("%s [%u] (%s%s%s)")
 					:format(area.name, id, area.owner,
-					area.open and S(":open") or area.openfarming and ":openfarming" or "",
-					faction_info and ":"..faction_info or ""))
+					area.open and S(":open") or "",
+					faction_info and ": "..faction_info or ""))
 		end
 
 		for i, area in pairs(areas:getExternalHudEntries(pos)) do

init.lua

@@ -35,10 +35,6 @@ minetest.register_privilege("areas_high_limit", {
 	description = S("Can protect more, bigger areas."),
 	give_to_singleplayer = false
 })
--- Mega_builder privilege -- MFF
-minetest.register_privilege("megabuilder", {
-	description = "Can protect an infinite amount of areas."
-})
 
 if not minetest.registered_privileges[areas.config.self_protection_privilege] then
 	minetest.register_privilege(areas.config.self_protection_privilege, {

internal.lua

@@ -60,6 +60,8 @@ function areas:load()
 	end
 	file:close()
 	self:populateStore()
+
+	areas:_checkHierarchy()
 end
 
 --- Checks an AreaStore ID.
@@ -207,6 +209,7 @@ function areas:isSubarea(pos1, pos2, id)
 end
 
 -- Returns a table (list) of children of an area given its identifier
+-- This is not recursive, meaning that only children and not grand-children are returned.
 function areas:getChildren(id)
 	local children = {}
 	for cid, area in pairs(self.areas) do
@@ -261,19 +264,16 @@ areas:registerProtectionCondition(function(pos1, pos2, name)
 end)
 
 -- check if the area is too big
--- NALC: megabuilders skip checks on size and number of areas
 areas:registerProtectionCondition(function(pos1, pos2, name)
 	local privs = minetest.get_player_privs(name)
-	if not privs.megabuilder then	
-		local max_size = privs.areas_high_limit and
+	local max_size = privs.areas_high_limit and
 			areas.config.self_protection_max_size_high or
 			areas.config.self_protection_max_size
-		if
+	if
 			(pos2.x - pos1.x + 1) > max_size.x or
 			(pos2.y - pos1.y + 1) > max_size.y or
 			(pos2.z - pos1.z + 1) > max_size.z then
-			return false, S("Area is too big.")
-		end
+		return false, S("Area is too big.")
 	end
 end)
 
@@ -345,10 +345,13 @@ function areas:isAreaOwner(id, name)
 	if cur and minetest.check_player_privs(name, self.adminPrivs) then
 		return true
 	end
-	while cur do
+	local seen = {}
+	while cur and not seen[cur] do
 		if cur.owner == name then
 			return true
 		elseif cur.parent then
+			-- Prevent lock-ups
+			seen[cur] = true
 			cur = self.areas[cur.parent]
 		else
 			return false
@@ -356,3 +359,56 @@ function areas:isAreaOwner(id, name)
 	end
 	return false
 end
+
+local function get_parent_chain_if_recursive(area, completed)
+	-- Get uppermost parent
+	local affected = {}
+	while area do
+		if affected[area] then
+			-- List of affected areas
+			return affected
+		end
+		if completed[area] then
+			-- Already checked by another function call --> all OK
+			return nil
+		end
+		affected[area] = true
+		completed[area] = true
+
+		area = areas.areas[area.parent]
+	end
+	return nil -- all OK
+end
+
+--- Internal function to ensure there are no circular parent/children occurrences
+function areas:_checkHierarchy()
+	local needs_save = false
+	local completed = {}
+	for _, area_1 in pairs(self.areas) do
+		local chain = get_parent_chain_if_recursive(area_1, completed)
+		if chain then
+			-- How can it be fixed if there is a longer chain?
+			local list = {}
+			for area, _ in pairs(chain) do
+				list[#list + 1] = area.parent
+			end
+
+			local instruction
+			if #list == 1 then
+				-- Trivial case, can be resolved in-place
+				instruction = "The issue was corrected automatically."
+				area_1.parent = nil
+				needs_save = true
+			else
+				instruction = "Please resolve this conflict manually. Expect issues."
+			end
+
+			core.log("error", "[areas] LOGIC ERROR! Detected a circular area hierarchy in the "
+				.. "following area ID(s): " .. table.concat(list, ", ") .. ". " .. instruction)
+		end
+	end
+	if needs_save then
+		-- Prevent repetitive spam upon startup
+		self:save()
+	end
+end

pos.lua

@@ -69,68 +69,53 @@ minetest.register_chatcommand("select_area", {
 	end,
 })
 
+local function area_pos_handler(name, param, nr)
+	local pos
+	local player = minetest.get_player_by_name(name)
+	if player then
+		pos = vector.round(player:get_pos())
+	end
+
+	-- Input parsing
+	local error_msg
+	local found, _, x_str, y_str, z_str = param:find(
+		"^(~?-?%d*)[, ] *(~?-?%d*)[, ] *(~?-?%d*)$")
+	if found then
+		pos, error_msg = parse_relative_pos(x_str, y_str, z_str, pos)
+	elseif param ~= "" then
+		return false, S("Invalid usage, see /help @1.", "area_pos" .. nr)
+	end
+	if not pos then
+		return false, error_msg or S("Unable to get position.")
+	end
+
+	-- Assign the position
+	pos = posLimit(vector.round(pos))
+	if nr == 1 then
+		areas:setPos1(name, pos)
+	else
+		areas:setPos2(name, pos)
+	end
+	return true, S("Area position @1 set to @2", tostring(nr),
+			minetest.pos_to_string(pos))
+end
+
 minetest.register_chatcommand("area_pos1", {
-	params = "[X Y Z|X,Y,Z]",
+	params = "[X Y Z|X,Y,Z|X, Y, Z]",
 	description = S("Set area protection region position @1 to your"
 		.." location or the one specified", "1"),
 	privs = {},
 	func = function(name, param)
-		local pos
-		local player = minetest.get_player_by_name(name)
-		if player then
-			pos = vector.round(player:get_pos())
-		end
-		local found, _, x_str, y_str, z_str = param:find(
-			"^(~?-?%d*)[, ](~?-?%d*)[, ](~?-?%d*)$")
-		if found then
-			local get_pos, reason = parse_relative_pos(x_str, y_str, z_str, pos)
-			if get_pos then
-				pos = get_pos
-			elseif not get_pos and reason then
-				return false, reason
-			end
-		elseif param ~= "" then
-			return false, S("Invalid usage, see /help @1.", "area_pos1")
-		end
-		if not pos then
-			return false, S("Unable to get position.")
-		end
-		pos = posLimit(vector.round(pos))
-		areas:setPos1(name, pos)
-		return true, S("Area position @1 set to @2", "1",
-				minetest.pos_to_string(pos))
+		return area_pos_handler(name, param, 1)
 	end,
 })
 
 minetest.register_chatcommand("area_pos2", {
-	params = "[X Y Z|X,Y,Z]",
+	params = "[X Y Z|X,Y,Z|X, Y, Z]",
 	description = S("Set area protection region position @1 to your"
 		.." location or the one specified", "2"),
 	func = function(name, param)
-		local pos
-		local player = minetest.get_player_by_name(name)
-		if player then
-			pos = vector.round(player:get_pos())
-		end
-		local found, _, x_str, y_str, z_str = param:find(
-			"^(~?-?%d*)[, ](~?-?%d*)[, ](~?-?%d*)$")
-		if found then
-			local get_pos, reason = parse_relative_pos(x_str, y_str, z_str, pos)
-			if get_pos then
-				pos = get_pos
-			elseif not get_pos and reason then
-				return false, reason
-			end
-		elseif param ~= "" then
-			return false, S("Invalid usage, see /help @1.", "area_pos2")
-		end
-		if not pos then
-			return false, S("Unable to get position.")
-		end
-		pos = posLimit(vector.round(pos))
-		areas:setPos2(name, pos)
-		return true, S("Area position @1 set to @2", "2",
-			minetest.pos_to_string(pos))
+		return area_pos_handler(name, param, 2)
 	end,
 })