Fix crash in `spawnpoint.save` (triggered by /setspawn) (#3)

* Remove load logging
* Remove save logging
* Log on set spawnpoint
* Log on configure variable
* Fix get configuration for `do_not_move`

README.md

@@ -6,9 +6,9 @@ Static Spawnpoint [spawnpoint]
 * [Download Latest Version](https://github.com/octacian/spawnpoint/archive/master.zip)
 * ...or browse the code on [GitHub](https://github.com/octacian/spawnpoint)
 
-This is a rather simple mod introducing two commands to set a static spawnpoint and to teleport to it. Yes, I know you can set this in `minetest.conf`, however, doing so causes the spawnpoint to be the same across all of your worlds (very inconvenient). Instead of using `minetest.conf`, this mod stores the spawnpoint (and other settings) as a multi-line string within a file called `spawnpoint.conf` in the world directory. This allows each and every world to have a different spawnpoint.
+This is a rather simple mod introducing two commands to set a static spawnpoint and to teleport to it. Yes, I know you can set this in `minetest.conf`, however, doing so causes the spawnpoint to be the same across all of your worlds (very inconvenient). Instead of using `minetest.conf`, this mod stores the spawnpoint (and other settings) within the world directory itself. This allows each and every world to have a different spawnpoint and configuration.
 
-The most unique thing about this spawn mod is that it includes a feature allowing you to set the time between executing the command until the player is actually teleported. By default, the teleportation will be interrupted if the player moves within that time. The time and the feature requiring players to stand still can be configured as documented below in the configuration section.
+The most unique thing about this spawn mod is that it includes a feature allowing you to set the time between executing the command until the player is actually teleported. You can also enable a setting which causes the teleportation to be interrupted if the player moves within the original time.
 
 ### Commands
 - `/spawnpoint`: Display spawnpoint position if set (also see configuration section)
@@ -18,9 +18,14 @@ The most unique thing about this spawn mod is that it includes a feature allowin
 __Note:__ If no spawnpoint is specified and a player attempts to execute `/spawn`, he/she will be told "No spawnpoint set!"
 
 ### Configuration
-The different "variables" of SpawnPoint can be configured per-world using the `/spawnpoint` command (requires server privilege). By default this command displays the spawnpoint, but when providing a setting name as well, the value of the setting is returned (assuming such a setting exists). If a setting name and value is provided, the setting is changed. Valid setting names are listed below.
+The different "variables" of SpawnPoint can be configured per-world using the `/spawnpoint` command (requires server privilege). This command displays the spawnpoint if no parameters are provided, but when a setting name is provided, the value of the setting is returned (assuming such a setting exists). If a setting name and value is provided, the setting is changed. Valid setting names are listed below.
 
 * `time`: Time before teleportation is completed (if `0` teleportation is immediate)
 * `do_not_move`: Whether a player should be required to not move to allow teleportation to be successful
+* `pos`: Position in the format `(<x>,<y>,<z>)` - can only be set via `/setspawn` or manually in configuration files
 
-Screenshot was taken at spawn on the awesome [HOMETOWN](https://forum.minetest.net/viewtopic.php?f=10&t=16699) server!
+This per-world configuration (including the spawn position itself) is stored in the world directory. If Minetest 0.4.16's new modstorage is available, SpawnPoint will use that to store configuration. Otherwise, configuration will be handled by the Minetest `Settings` API and placed in a `spawnpoint.conf` file. If you would like to configure SpawnPoint manually, create a `spawnpoint.conf` file in the world directory and assign values to the applicable settings as documented above, each setting on a new line in the format `setting_name = setting_value`.
+
+Initially, SpawnPoint stored all settings in a multi-line `spawnpoint.conf` file, however, this made very little sense as setting weren't named. With the new configuration scheme as documented above, compatibility code has been implemented causing all the old settings to be imported into the newest format possible. When the formatted settings method is used with `spawnpoint.conf`, all configuration is automatically imported to Minetest modstorage as well when it becomes available. After importing takes place from `spawnpoint.conf`, the file is removed.
+
+Screenshot was taken at spawn on the awesome [HOMETOWN](https://forum.minetest.net/viewtopic.php?f=10&t=16699) server!

init.lua

@@ -2,13 +2,19 @@
 
 spawnpoint = {}
 
+local storage
 local path = minetest.get_worldpath().."/spawnpoint.conf"
+local data = Settings(path)
+
+if minetest.get_mod_storage then
+	storage = minetest.get_mod_storage()
+end
 
 -- [function] Log
 function spawnpoint.log(content, log_type)
-  if not content then return false end
+	if not content then return false end
-  if log_type == nil then log_type = "action" end
+	if log_type == nil then log_type = "action" end
-  minetest.log(log_type, "[SpawnPoint] "..content)
+	minetest.log(log_type, "[SpawnPoint] "..content)
 end
 
 ----------------------
@@ -20,152 +26,209 @@ local huds  = {}
 local pos   = {}
 
 minetest.register_globalstep(function(dtime)
-  for _, player in pairs(minetest.get_connected_players()) do
+	for _, player in pairs(minetest.get_connected_players()) do
-    local name = player:get_player_name()
+		local name = player:get_player_name()
 
-    if pos[name] and spawnpoint.do_not_move then
+		if pos[name] and spawnpoint.do_not_move then
-      if not moved[name] and not vector.equals(pos[name], player:getpos()) then
+			if not moved[name] and not vector.equals(pos[name], player:getpos()) then
-        moved[name] = true
+				moved[name] = true
 
-        player:hud_remove(huds[name])
+				player:hud_remove(huds[name])
-        minetest.chat_send_player(name, "Teleportation interrupted! (Player moved)")
+				minetest.chat_send_player(name, "Teleportation interrupted! (Player moved)")
-      end
+			end
-    end
+		end
-  end
+	end
 end)
 
 ----------------------
 -- HELPER FUNCTIONS --
 ----------------------
 
--- [function] Clean Position
+-- [local function] Count table contents
-function spawnpoint.pos_clean(pos)
+local function count(t)
-  pos.x = math.floor(pos.x)
+	local count = 0
-  pos.y = math.floor(pos.y)
+	for _, i in pairs(t) do
-  pos.z = math.floor(pos.z)
+		count = count + 1
+	end
+	return count
+end
 
-  return pos
+-- [local function] Check if table is empty
+local function is_empty(t)
+	if t.fields then
+		return count(t.fields) == 0
+	else
+		return count(t) == 0
+	end
 end
 
 -- [function] Load
 function spawnpoint.load()
-  local res = io.open(path, "r")
+	if data and not is_empty(data:to_table()) then
-  if res then
+		spawnpoint.time = tonumber(data:get("time"))
-    res = res:read("*all"):split("\n", true)
+		spawnpoint.do_not_move = data:get_bool("do_not_move")
 
-    spawnpoint.time        = tonumber(res[1]) or 3
+		local pos = data:get("pos")
-    spawnpoint.do_not_move = not not res[2] or true
+		if pos then
+			spawnpoint.pos = minetest.string_to_pos(pos)
+		end
 
-    if res[3] then
+		if storage then
-      spawnpoint.pos = minetest.string_to_pos(res[3])
+			os.remove(path)
-    end
+		end
-  else
+	elseif storage and not is_empty(storage:to_table()) then
-    spawnpoint.time        = 3
+		local pos = storage:get_string("pos")
-    spawnpoint.do_not_move = true
+		if pos then
-  end
+			spawnpoint.pos = minetest.string_to_pos(pos)
+		end
+
+		local do_not_move = storage:get_string("do_not_move")
+		if do_not_move == "true" or do_not_move == true then
+			spawnpoint.do_not_move = true
+		else
+			spawnpoint.do_not_move = false
+		end
+
+		spawnpoint.time = storage:get_float("time")
+	else
+		local f = io.open(path, "r")
+		if f then
+			local res = f:read("*all"):split("\n", true)
+
+			spawnpoint.time = tonumber(res[1])
+
+			if res[2] == "true" or res[2] == true then
+				spawnpoint.do_not_move = true
+			else
+				spawnpoint.do_not_move = false
+			end
+
+			if res[3] then
+				spawnpoint.pos = minetest.string_to_pos(res[3])
+			end
+
+			f:close()
+			-- Clear file
+			os.remove(path)
+		end
+	end
 end
 
 -- [function] Save
 function spawnpoint.save()
-  local str = tostring(spawnpoint.time)..
+	if storage then
-    "\n"..tostring(spawnpoint.do_not_move) or ""
+		storage:set_float("time", spawnpoint.time or 0)
+		storage:set_string("do_not_move", tostring(spawnpoint.do_not_move))
 
-  if spawnpoint.pos then
+		if spawnpoint.pos then
-    str = str.."\n"..minetest.pos_to_string(spawnpoint.pos)
+			storage:set_string("pos", minetest.pos_to_string(spawnpoint.pos))
-  end
+		end
 
-  io.open(path, "w"):write(str)
+		return true
+	elseif data then
+		data:set("time", tostring(spawnpoint.time))
+		data:set_bool("do_not_move", spawnpoint.do_not_move)
+
+		if spawnpoint.pos then
+			data:set("pos", minetest.pos_to_string(spawnpoint.pos))
+		end
+
+		data:write()
+		return true
+	end
 end
 
 -- [function] Set
 function spawnpoint.set(pos)
-  if type(pos) == "string" then
+	if type(pos) == "string" then
-    pos = minetest.string_to_pos(pos)
+		pos = minetest.string_to_pos(pos)
-  end
+	end
 
-  if type(pos) == "table" then
+	if type(pos) == "table" then
-    spawnpoint.pos = spawnpoint.pos_clean(pos)
+		spawnpoint.pos = pos
-  end
+		spawnpoint.save()
+		spawnpoint.log("Set spawnpoint to "..minetest.pos_to_string(pos))
+	end
 end
 
 -- [function] Bring
 function spawnpoint.bring(player)
-  if type(player) == "string" then
+	if type(player) == "string" then
-    player = minetest.get_player_by_name(player)
+		player = minetest.get_player_by_name(player)
-  end
+	end
 
-  if player and spawnpoint.pos then
+	if player and spawnpoint.pos then
-    local pos = spawnpoint.pos
+		local pos = spawnpoint.pos
-    player:setpos({x=pos.x, y=pos.y+0.5, z=pos.z})
+		player:setpos({x=pos.x, y=pos.y+0.5, z=pos.z})
-  end
+	end
 end
 
 -- [function] Begin Countdown
 function spawnpoint.begin(player, time)
-  if not time then
+	if not time then
-    time = spawnpoint.time
+		time = spawnpoint.time
-  end
+	end
 
-  if type(player) == string then
+	if type(player) == string then
-    player = minetest.get_player_by_name(player)
+		player = minetest.get_player_by_name(player)
-  end
+	end
 
-  local name = player:get_player_name()
+	local name = player:get_player_name()
 
-  if player and time and time ~= 0 then
+	if player and time and time ~= 0 then
-    local move = "Do not move!"
+		local move = "Do not move!"
-    if spawnpoint.do_not_move ~= true then
+		if spawnpoint.do_not_move ~= true then
-      move = ""
+			move = ""
-    end
+		end
 
-    local seconds = "s"
+		local seconds = "s"
-    if time < 2 then
+		if time < 2 then
-      seconds = ""
+			seconds = ""
-    end
+		end
 
-    -- Send to chat
+		-- Send to chat
-    minetest.chat_send_player(name, "Teleportation will be complete in "..time..
+		minetest.chat_send_player(name, "Teleportation will be complete in "..time..
-      " second"..seconds..". "..move)
+			" second"..seconds..". "..move)
 
-    -- Add initial HUD
+		-- Add initial HUD
-    huds[name] = player:hud_add({
+		huds[name] = player:hud_add({
-      hud_elem_type = "text",
+			hud_elem_type = "text",
-      text = "Teleportation Progress: "..time.." seconds remaining!",
+			text = "Teleportation Progress: "..time.." seconds remaining!",
-      position = {x = 0.5, y = 0.5},
+			position = {x = 0.5, y = 0.5},
-      number = 0xFFFFFF,
+			number = 0xFFFFFF,
-    })
+		})
 
-    local hud   = huds[name]
+		local hud   = huds[name]
-    pos[name]   = player:getpos()
+		pos[name]   = player:getpos()
-    moved[name] = false
+		moved[name] = false
 
-    -- Register update callbacks
+		-- Register update callbacks
-    for i = 1, time do
+		for i = 1, time do
-      if i == time then
+			if i == time then
-        minetest.after(i, function()
+				minetest.after(i, function()
-          if not moved[name] then
+					if not moved[name] then
-            player:hud_remove(hud)
+						player:hud_remove(hud)
-            spawnpoint.bring(player)
+						spawnpoint.bring(player)
 
-            -- Send to chat
+						-- Send to chat
-            minetest.chat_send_player(name, "Teleportation successful!")
+						minetest.chat_send_player(name, "Teleportation successful!")
 
-            -- Prevent further callbacks from globalstep
+						-- Prevent further callbacks from globalstep
-            moved[name] = true
+						moved[name] = true
-          end
+					end
-        end)
+				end)
-      else
+			else
-        minetest.after(i, function()
+				minetest.after(i, function()
-          if not moved[name] then
+					if not moved[name] then
-            player:hud_change(hud, "text", "Teleportation Progress: "..time - i.." seconds remaining!")
+						player:hud_change(hud, "text", "Teleportation Progress: "..time - i.." seconds remaining!")
-          end
+					end
-        end)
+				end)
-      end
+			end
-    end
+		end
-  elseif player then
+	elseif player then
-    minetest.chat_send_player(name, "Teleporting to spawn")
+		minetest.chat_send_player(name, "Teleporting to spawn")
-    spawnpoint.bring(player)
+		spawnpoint.bring(player)
-  end
+	end
 end
 
 -------------------
@@ -179,12 +242,12 @@ minetest.register_on_shutdown(spawnpoint.save)
 
 -- [register] On Respawn Player
 minetest.register_on_respawnplayer(function(player)
-  spawnpoint.bring(player)
+	spawnpoint.bring(player)
 end)
 
 -- [register] On New Player
 minetest.register_on_newplayer(function(player)
-  spawnpoint.bring(player)
+	spawnpoint.bring(player)
 end)
 
 -- [register priv] Spawn
@@ -192,81 +255,85 @@ minetest.register_privilege("spawn", "Ability to teleport to spawn at will with
 
 -- [register cmd] Set spawn
 minetest.register_chatcommand("setspawn", {
-  description = "Set spawn",
+	description = "Set spawn",
-  privs = {server=true},
+	privs = {server=true},
-  func = function(name, param)
+	func = function(name, param)
-    local pos = minetest.get_player_by_name(name):getpos()
+		local pos = minetest.get_player_by_name(name):getpos()
-    if param then
+		if param then
-      local ppos = minetest.string_to_pos(param)
+			local ppos = minetest.string_to_pos(param)
-      if type(ppos) == "table" then
+			if type(ppos) == "table" then
-        pos = ppos
+				pos = ppos
-      end
+			end
-    end
+		end
+		pos = vector.round(pos)
 
-    spawnpoint.set(pos)
+		spawnpoint.set(pos)
 
-    return true, "Set spawnpoint to "..minetest.pos_to_string(pos)
+		return true, "Set spawnpoint to "..minetest.pos_to_string(pos)
-  end,
+	end,
 })
 
 -- [register cmd] Teleport to spawn
 minetest.register_chatcommand("spawn", {
-  description = "Teleport to spawn",
+	description = "Teleport to spawn",
-  privs = {spawn=true},
+	privs = {spawn=true},
-  func = function(name, param)
+	func = function(name, param)
-    local player = minetest.get_player_by_name(name)
+		local player = minetest.get_player_by_name(name)
-    if param ~= "" then
+		if param ~= "" then
-      local pplayer = minetest.get_player_by_name(param)
+			local pplayer = minetest.get_player_by_name(param)
-      if pplayer and minetest.check_player_privs(pplayer, {bring=true}) then
+			if pplayer and minetest.check_player_privs(pplayer, {bring=true}) then
-        player = pplayer
+				player = pplayer
-      else
+			else
-        return false, "Cannot teleport another player to spawn without bring privilege"
+				return false, "Cannot teleport another player to spawn without bring privilege"
-      end
+			end
-    end
+		end
 
-    if not spawnpoint.pos then
+		if not spawnpoint.pos then
-      return false, "No spawnpoint set!"
+			return false, "No spawnpoint set!"
-    end
+		end
 
-    spawnpoint.begin(player)
+		spawnpoint.begin(player)
-  end,
+	end,
 })
 
 -- [register cmd] Manage spawnpoint
 minetest.register_chatcommand("spawnpoint", {
-  description = "Get/Set SpawnPoint information",
+	description = "Get/Set SpawnPoint information",
-  func = function(name, param)
+	func = function(name, param)
-    if not param or param == "" then
+		if not param or param == "" then
-      local pos = "Not set!"
+			local pos = "Not set!"
-      if spawnpoint.pos then
+			if spawnpoint.pos then
-        pos = minetest.pos_to_string(spawnpoint.pos)
+				pos = minetest.pos_to_string(spawnpoint.pos)
-      end
+			end
 
-      return true, "SpawnPoint Position: "..pos
+			return true, "SpawnPoint Position: "..pos
-    elseif minetest.check_player_privs(minetest.get_player_by_name(name), {server=true}) then
+		elseif minetest.check_player_privs(minetest.get_player_by_name(name), {server=true}) then
-      local p = param:split(" ")
+			local p = param:split(" ")
 
-      if p[1] == "time" then
+			if p[1] == "time" then
-        local num = tonumber(p[2])
+				local num = tonumber(p[2])
 
-        if not num then
+				if not num then
-          return true, "SpawnPoint->time: "..spawnpoint.time
+					return true, "SpawnPoint->time: "..dump(spawnpoint.time)
-        elseif num == spawnpoint.time then
+				elseif num == spawnpoint.time then
-          return false, "Time already set to "..p[2].."!"
+					return false, "Time already set to "..p[2].."!"
-        else
+				else
-          spawnpoint.time = num
+					spawnpoint.time = num
-          return true, "Set time to "..tostring(num)
+					spawnpoint.save()
-        end
+					spawnpoint.log("Set time to "..dump(num))
-      elseif p[1] == "do_not_move" then
+					return true, "Set time to "..dump(num)
-        local move = minetest.is_yes(p[2])
+				end
-        minetest.log("action", dump(p[2])..", "..dump(move))
+			elseif p[1] == "do_not_move" then
-        if move == nil then
+				local move = minetest.is_yes(p[2])
-          return true, "SpawnPoint->do_not_move: "..tostring(spawnpoint.do_not_move)
+				if move == nil or not p[2] then
-        else
+					return true, "SpawnPoint->do_not_move: "..dump(spawnpoint.do_not_move)
-          spawnpoint.do_not_move = move
+				else
-          return true, "Set do_not_move to "..tostring(move)
+					spawnpoint.do_not_move = move
-        end
+					spawnpoint.save()
-      end
+					spawnpoint.log("Set do_not_move to "..dump(move))
-    end
+					return true, "Set do_not_move to "..dump(move)
-  end,
+				end
+			end
+		end
+	end,
 })