Add relative numbers for commands by prepending ~ (#9588)

* Add relative numbers for commands by prepending ~

* Some builtin code cleanup

* Disallow nan and inf in minetest.string_to_area

* Remove unused local variable teleportee (makes Luacheck happy)

* Clean up core.string_to_pos

* Make area parsing less permissive

* Rewrite tests as busted tests

* /time: Fix negative minutes not working

Co-authored-by: Lars Mueller <appgurulars@gmx.de>

3 files changed, 252 insertions, 47 deletions

diff --git a/builtin/common/misc_helpers.lua b/builtin/common/misc_helpers.lua
index f5f89acd7..83f17da7b 100644
--- a/builtin/common/misc_helpers.lua
+++ b/builtin/common/misc_helpers.lua
@@ -425,54 +425,50 @@ function core.string_to_pos(value)
 		return nil
 	end
 
-	local x, y, z = string.match(value, "^([%d.-]+)[, ] *([%d.-]+)[, ] *([%d.-]+)$")
-	if x and y and z then
-		x = tonumber(x)
-		y = tonumber(y)
-		z = tonumber(z)
-		return vector.new(x, y, z)
-	end
-	x, y, z = string.match(value, "^%( *([%d.-]+)[, ] *([%d.-]+)[, ] *([%d.-]+) *%)$")
+	value = value:match("^%((.-)%)$") or value -- strip parentheses
+
+	local x, y, z = value:trim():match("^([%d.-]+)[,%s]%s*([%d.-]+)[,%s]%s*([%d.-]+)$")
 	if x and y and z then
 		x = tonumber(x)
 		y = tonumber(y)
 		z = tonumber(z)
 		return vector.new(x, y, z)
 	end
+
 	return nil
 end
 
 
 --------------------------------------------------------------------------------
-function core.string_to_area(value)
-	local p1, p2 = unpack(value:split(") ("))
-	if p1 == nil or p2 == nil then
-		return nil
-	end
 
-	p1 = core.string_to_pos(p1 .. ")")
-	p2 = core.string_to_pos("(" .. p2)
-	if p1 == nil or p2 == nil then
-		return nil
+do
+	local rel_num_cap = "(~?-?%d*%.?%d*)" -- may be overly permissive as this will be tonumber'ed anyways
+	local num_delim = "[,%s]%s*"
+	local pattern = "^" .. table.concat({rel_num_cap, rel_num_cap, rel_num_cap}, num_delim) .. "$"
+
+	local function parse_area_string(pos, relative_to)
+		local pp = {}
+		pp.x, pp.y, pp.z = pos:trim():match(pattern)
+		return core.parse_coordinates(pp.x, pp.y, pp.z, relative_to)
 	end
 
-	return p1, p2
-end
+	function core.string_to_area(value, relative_to)
+		local p1, p2 = value:match("^%((.-)%)%s*%((.-)%)$")
+		if not p1 then
+			return
+		end
 
-local function test_string_to_area()
-	local p1, p2 = core.string_to_area("(10.0, 5, -2) (  30.2,   4, -12.53)")
-	assert(p1.x == 10.0 and p1.y == 5 and p1.z == -2)
-	assert(p2.x == 30.2 and p2.y == 4 and p2.z == -12.53)
+		p1 = parse_area_string(p1, relative_to)
+		p2 = parse_area_string(p2, relative_to)
 
-	p1, p2 = core.string_to_area("(10.0, 5, -2  30.2,   4, -12.53")
-	assert(p1 == nil and p2 == nil)
+		if p1 == nil or p2 == nil then
+			return
+		end
 
-	p1, p2 = core.string_to_area("(10.0, 5,) -2  fgdf2,   4, -12.53")
-	assert(p1 == nil and p2 == nil)
+		return p1, p2
+	end
 end
 
-test_string_to_area()
-
 --------------------------------------------------------------------------------
 function table.copy(t, seen)
 	local n = {}
@@ -701,3 +697,71 @@ end
 function core.is_nan(number)
 	return number ~= number
 end
+
+--[[ Helper function for parsing an optionally relative number
+of a chat command parameter, using the chat command tilde notation.
+
+Parameters:
+* arg: String snippet containing the number; possible values:
+    * "<number>": return as number
+    * "~<number>": return relative_to + <number>
+    * "~": return relative_to
+    * Anything else will return `nil`
+* relative_to: Number to which the `arg` number might be relative to
+
+Returns:
+A number or `nil`, depending on `arg.
+
+Examples:
+* `core.parse_relative_number("5", 10)` returns 5
+* `core.parse_relative_number("~5", 10)` returns 15
+* `core.parse_relative_number("~", 10)` returns 10
+]]
+function core.parse_relative_number(arg, relative_to)
+	if not arg then
+		return nil
+	elseif arg == "~" then
+		return relative_to
+	elseif string.sub(arg, 1, 1) == "~" then
+		local number = tonumber(string.sub(arg, 2))
+		if not number then
+			return nil
+		end
+		if core.is_nan(number) or number == math.huge or number == -math.huge then
+			return nil
+		end
+		return relative_to + number
+	else
+		local number = tonumber(arg)
+		if core.is_nan(number) or number == math.huge or number == -math.huge then
+			return nil
+		end
+		return number
+	end
+end
+
+--[[ Helper function to parse coordinates that might be relative
+to another position; supports chat command tilde notation.
+Intended to be used in chat command parameter parsing.
+
+Parameters:
+* x, y, z: Parsed x, y, and z coordinates as strings
+* relative_to: Position to which to compare the position
+
+Syntax of x, y and z:
+* "<number>": return as number
+* "~<number>": return <number> + player position on this axis
+* "~": return player position on this axis
+
+Returns: a vector or nil for invalid input or if player does not exist
+]]
+function core.parse_coordinates(x, y, z, relative_to)
+	if not relative_to then
+		x, y, z = tonumber(x), tonumber(y), tonumber(z)
+		return x and y and z and { x = x, y = y, z = z }
+	end
+	local rx = core.parse_relative_number(x, relative_to.x)
+	local ry = core.parse_relative_number(y, relative_to.y)
+	local rz = core.parse_relative_number(z, relative_to.z)
+	return rx and ry and rz and { x = rx, y = ry, z = rz }
+end
diff --git a/builtin/common/tests/misc_helpers_spec.lua b/builtin/common/tests/misc_helpers_spec.lua
index b11236860..f6ad96619 100644
--- a/builtin/common/tests/misc_helpers_spec.lua
+++ b/builtin/common/tests/misc_helpers_spec.lua
@@ -67,6 +67,96 @@ describe("pos", function()
 	end)
 end)
 
+describe("area parsing", function()
+	describe("valid inputs", function()
+		it("accepts absolute numbers", function()
+			local p1, p2 = core.string_to_area("(10.0, 5, -2) (  30.2 4 -12.53)")
+			assert(p1.x == 10 and p1.y == 5 and p1.z == -2)
+			assert(p2.x == 30.2 and p2.y == 4 and p2.z == -12.53)
+		end)
+
+		it("accepts relative numbers", function()
+			local p1, p2 = core.string_to_area("(1,2,3) (~5,~-5,~)", {x=10,y=10,z=10})
+			assert(type(p1) == "table" and type(p2) == "table")
+			assert(p1.x == 1 and p1.y == 2 and p1.z == 3)
+			assert(p2.x == 15 and p2.y == 5 and p2.z == 10)
+
+			p1, p2 = core.string_to_area("(1 2 3) (~5 ~-5 ~)", {x=10,y=10,z=10})
+			assert(type(p1) == "table" and type(p2) == "table")
+			assert(p1.x == 1 and p1.y == 2 and p1.z == 3)
+			assert(p2.x == 15 and p2.y == 5 and p2.z == 10)
+		end)
+	end)
+	describe("invalid inputs", function()
+		it("rejects too few numbers", function()
+			local p1, p2 = core.string_to_area("(1,1) (1,1,1,1)", {x=1,y=1,z=1})
+			assert(p1 == nil and p2 == nil)
+		end)
+
+		it("rejects too many numbers", function()
+			local p1, p2 = core.string_to_area("(1,1,1,1) (1,1,1,1)", {x=1,y=1,z=1})
+			assert(p1 == nil and p2 == nil)
+		end)
+
+		it("rejects nan & inf", function()
+			local p1, p2 = core.string_to_area("(1,1,1) (1,1,nan)", {x=1,y=1,z=1})
+			assert(p1 == nil and p2 == nil)
+
+			p1, p2 = core.string_to_area("(1,1,1) (1,1,~nan)", {x=1,y=1,z=1})
+			assert(p1 == nil and p2 == nil)
+
+			p1, p2 = core.string_to_area("(1,1,1) (1,~nan,1)", {x=1,y=1,z=1})
+			assert(p1 == nil and p2 == nil)
+
+			p1, p2 = core.string_to_area("(1,1,1) (1,1,inf)", {x=1,y=1,z=1})
+			assert(p1 == nil and p2 == nil)
+
+			p1, p2 = core.string_to_area("(1,1,1) (1,1,~inf)", {x=1,y=1,z=1})
+			assert(p1 == nil and p2 == nil)
+
+			p1, p2 = core.string_to_area("(1,1,1) (1,~inf,1)", {x=1,y=1,z=1})
+			assert(p1 == nil and p2 == nil)
+
+			p1, p2 = core.string_to_area("(nan,nan,nan) (nan,nan,nan)", {x=1,y=1,z=1})
+			assert(p1 == nil and p2 == nil)
+
+			p1, p2 = core.string_to_area("(nan,nan,nan) (nan,nan,nan)")
+			assert(p1 == nil and p2 == nil)
+
+			p1, p2 = core.string_to_area("(inf,inf,inf) (-inf,-inf,-inf)", {x=1,y=1,z=1})
+			assert(p1 == nil and p2 == nil)
+
+			p1, p2 = core.string_to_area("(inf,inf,inf) (-inf,-inf,-inf)")
+			assert(p1 == nil and p2 == nil)
+		end)
+
+		it("rejects words", function()
+			local p1, p2 = core.string_to_area("bananas", {x=1,y=1,z=1})
+			assert(p1 == nil and p2 == nil)
+
+			p1, p2 = core.string_to_area("bananas", "foobar")
+			assert(p1 == nil and p2 == nil)
+
+			p1, p2 = core.string_to_area("bananas")
+			assert(p1 == nil and p2 == nil)
+
+			p1, p2 = core.string_to_area("(bananas,bananas,bananas)")
+			assert(p1 == nil and p2 == nil)
+
+			p1, p2 = core.string_to_area("(bananas,bananas,bananas) (bananas,bananas,bananas)")
+			assert(p1 == nil and p2 == nil)
+		end)
+
+		it("requires parenthesis & valid numbers", function()
+			local p1, p2 = core.string_to_area("(10.0, 5, -2  30.2,   4, -12.53")
+			assert(p1 == nil and p2 == nil)
+
+			p1, p2 = core.string_to_area("(10.0, 5,) -2  fgdf2,   4, -12.53")
+			assert(p1 == nil and p2 == nil)
+		end)
+	end)
+end)
+
 describe("table", function()
 	it("indexof()", function()
 		assert.equal(1, table.indexof({"foo", "bar"}, "foo"))
diff --git a/builtin/game/chat.lua b/builtin/game/chat.lua
index c4fb6314e..cfb497cb0 100644
--- a/builtin/game/chat.lua
+++ b/builtin/game/chat.lua
@@ -130,8 +130,13 @@ local function parse_range_str(player_name, str)
 			return false, S("Unable to get position of player @1.", player_name)
 		end
 	else
-		p1, p2 = core.string_to_area(str)
-		if p1 == nil then
+		local player = core.get_player_by_name(player_name)
+		local relpos
+		if player then
+			relpos = player:get_pos()
+		end
+		p1, p2 = core.string_to_area(str, relpos)
+		if p1 == nil or p2 == nil then
 			return false, S("Incorrect area format. "
 				.. "Expected: (x1,y1,z1) (x2,y2,z2)")
 		end
@@ -570,10 +575,15 @@ core.register_chatcommand("teleport", {
 	description = S("Teleport to position or player"),
 	privs = {teleport=true},
 	func = function(name, param)
+		local player = core.get_player_by_name(name)
+		local relpos
+		if player then
+			relpos = player:get_pos()
+		end
 		local p = {}
-		p.x, p.y, p.z = param:match("^([%d.-]+)[, ] *([%d.-]+)[, ] *([%d.-]+)$")
-		p = vector.apply(p, tonumber)
-		if p.x and p.y and p.z then
+		p.x, p.y, p.z = string.match(param, "^([%d.~-]+)[, ] *([%d.~-]+)[, ] *([%d.~-]+)$")
+		p = core.parse_coordinates(p.x, p.y, p.z, relpos)
+		if p and p.x and p.y and p.z then
 			return teleport_to_pos(name, p)
 		end
 
@@ -587,9 +597,19 @@ core.register_chatcommand("teleport", {
 			"other players (missing privilege: @1).", "bring")
 
 		local teleportee_name
+		p = {}
 		teleportee_name, p.x, p.y, p.z = param:match(
-				"^([^ ]+) +([%d.-]+)[, ] *([%d.-]+)[, ] *([%d.-]+)$")
+				"^([^ ]+) +([%d.~-]+)[, ] *([%d.~-]+)[, ] *([%d.~-]+)$")
+		if teleportee_name then
+			local teleportee = core.get_player_by_name(teleportee_name)
+			if not teleportee then
+				return
+			end
+			relpos = teleportee:get_pos()
+			p = core.parse_coordinates(p.x, p.y, p.z, relpos)
+		end
 		p = vector.apply(p, tonumber)
+
 		if teleportee_name and p.x and p.y and p.z then
 			if not has_bring_priv then
 				return false, missing_bring_msg
@@ -842,7 +862,7 @@ core.register_chatcommand("spawnentity", {
 	description = S("Spawn entity at given (or your) position"),
 	privs = {give=true, interact=true},
 	func = function(name, param)
-		local entityname, p = string.match(param, "^([^ ]+) *(.*)$")
+		local entityname, pstr = string.match(param, "^([^ ]+) *(.*)$")
 		if not entityname then
 			return false, S("EntityName required.")
 		end
@@ -856,11 +876,15 @@ core.register_chatcommand("spawnentity", {
 		if not core.registered_entities[entityname] then
 			return false, S("Cannot spawn an unknown entity.")
 		end
-		if p == "" then
+		local p
+		if pstr == "" then
 			p = player:get_pos()
 		else
-			p = core.string_to_pos(p)
-			if p == nil then
+			p = {}
+			p.x, p.y, p.z = string.match(pstr, "^([%d.~-]+)[, ] *([%d.~-]+)[, ] *([%d.~-]+)$")
+			local relpos = player:get_pos()
+			p = core.parse_coordinates(p.x, p.y, p.z, relpos)
+			if not (p and p.x and p.y and p.z) then
 				return false, S("Invalid parameters (@1).", param)
 			end
 		end
@@ -1019,6 +1043,13 @@ core.register_chatcommand("status", {
 	end,
 })
 
+local function get_time(timeofday)
+	local time = math.floor(timeofday * 1440)
+	local minute = time % 60
+	local hour = (time - minute) / 60
+	return time, hour, minute
+end
+
 core.register_chatcommand("time", {
 	params = S("[<0..23>:<0..59> | <0..24000>]"),
 	description = S("Show or set time of day"),
@@ -1037,9 +1068,14 @@ core.register_chatcommand("time", {
 			return false, S("You don't have permission to run "
 				.. "this command (missing privilege: @1).", "settime")
 		end
-		local hour, minute = param:match("^(%d+):(%d+)$")
-		if not hour then
-			local new_time = tonumber(param) or -1
+		local relative, negative, hour, minute = param:match("^(~?)(%-?)(%d+):(%d+)$")
+		if not relative then -- checking the first capture against nil suffices
+			local new_time = core.parse_relative_number(param, core.get_timeofday() * 24000)
+			if not new_time then
+				new_time = tonumber(param) or -1
+			else
+				new_time = new_time % 24000
+			end
 			if new_time ~= new_time or new_time < 0 or new_time > 24000 then
 				return false, S("Invalid time (must be between 0 and 24000).")
 			end
@@ -1047,14 +1083,29 @@ core.register_chatcommand("time", {
 			core.log("action", name .. " sets time to " .. new_time)
 			return true, S("Time of day changed.")
 		end
+		local new_time
 		hour = tonumber(hour)
 		minute = tonumber(minute)
-		if hour < 0 or hour > 23 then
-			return false, S("Invalid hour (must be between 0 and 23 inclusive).")
-		elseif minute < 0 or minute > 59 then
-			return false, S("Invalid minute (must be between 0 and 59 inclusive).")
+		if relative == "" then
+			if hour < 0 or hour > 23 then
+				return false, S("Invalid hour (must be between 0 and 23 inclusive).")
+			elseif minute < 0 or minute > 59 then
+				return false, S("Invalid minute (must be between 0 and 59 inclusive).")
+			end
+			new_time = (hour * 60 + minute) / 1440
+		else
+			if minute < 0 or minute > 59 then
+				return false, S("Invalid minute (must be between 0 and 59 inclusive).")
+			end
+			local current_time = core.get_timeofday()
+			if negative == "-" then -- negative time
+				hour, minute = -hour, -minute
+			end
+			new_time = (current_time + (hour * 60 + minute) / 1440) % 1
+			local _
+			_, hour, minute = get_time(new_time)
 		end
-		core.set_timeofday((hour * 60 + minute) / 1440)
+		core.set_timeofday(new_time)
 		core.log("action", ("%s sets time to %d:%02d"):format(name, hour, minute))
 		return true, S("Time of day changed.")
 	end,