mod storage (#67)

* persistence in mod-storage wip

* remove `travelnet.restore_data`

* refactor saving/loading with mod_storage

* migration test

* fix typo / handle some edge cases

---------

Co-authored-by: BuckarooBanzay <[email protected]>

.github/workflows/test.yml

@@ -9,7 +9,7 @@ jobs:
     timeout-minutes: 10
     strategy:
       matrix:
-        ENGINE_VERSION: [5.3.0, 5.4.0, 5.5.0, 5.6.0, latest]
+        ENGINE_VERSION: [5.3.0, 5.4.0, 5.5.0, 5.6.0, 5.7.0, latest]
 
     steps:
     - uses: actions/checkout@v3

actions/add_station.lua

@@ -45,7 +45,12 @@ return function (node_info, fields, player)
 			"the network of someone else. Aborting.")
 	end
 
-	local network = travelnet.get_or_create_network(owner_name, station_network)
+	local travelnets = travelnet.get_travelnets(owner_name)
+	local network = travelnets[station_network]
+	if not network then
+		network = {}
+		travelnets[station_network] = network
+	end
 
 	-- lua doesn't allow efficient counting here
 	local station_count = 1  -- start at one, assume the station about to be created already exists
@@ -88,7 +93,7 @@ return function (node_info, fields, player)
 					tostring(station_name), tostring(station_network), tostring(owner_name)))
 
 		-- save the updated network data in a savefile over server restart
-		travelnet.save_data(owner_name)
+		travelnet.set_travelnets(owner_name, travelnets)
 
 		return true, { formspec = travelnet.formspecs.primary, options = {
 			station_name = station_name,

actions/change_order.lua

@@ -13,7 +13,8 @@ return function (node_info, fields, player)
 			or (minetest.get_player_privs(player_name)[travelnet.attach_priv])
 		)
 	then
-		local network = travelnet.get_network(node_info.props.owner_name, node_info.props.station_network)
+		local travelnets = travelnet.get_travelnets(node_info.props.owner_name)
+		local network = travelnets[node_info.props.station_network]
 
 		if not network then
 			return false, S("This station does not have a network.")
@@ -61,7 +62,7 @@ return function (node_info, fields, player)
 			end
 
 			-- store the changed order
-			travelnet.save_data(player_name)
+			travelnet.set_travelnets(player_name, travelnets)
 			return true, { formspec = travelnet.formspecs.primary }
 		end
 	end

actions/repair_station.lua

@@ -15,25 +15,31 @@ return function (node_info, _, player)
 		return false, S("Update failed! Resetting this box on the travelnet.")
 	end
 
-	-- if the station got lost from the network for some reason (savefile corrupted?) then add it again
-	if not travelnet.get_station(owner_name, station_network, station_name) then
-		local network = travelnet.get_or_create_network(owner_name, station_network)
+	local travelnets = travelnet.get_travelnets(owner_name)
+	local network = travelnets[station_network]
+	if not network then
+		network = {}
+		travelnets[station_network] = network
+	end
 
-		local zeit = node_info.meta:get_int("timestamp")
-		if not zeit or type(zeit) ~= "number" or zeit < 100000 then
-			zeit = os.time()
+	-- if the station got lost from the network for some reason (savefile corrupted?) then add it again
+	if not network[station_name] then
+		local timestamp = node_info.meta:get_int("timestamp")
+		if not timestamp or type(timestamp) ~= "number" or timestamp < 100000 then
+			timestamp = os.time()
 		end
 
 		-- add this station
 		network[station_name] = {
 			pos = node_info.pos,
-			timestamp = zeit
+			timestamp = timestamp
 		}
 
 		minetest.chat_send_player(owner_name,
 				S("Station '@1'" .. " " ..
 					"has been reattached to the network '@2'.", station_name, station_network))
-		travelnet.save_data(owner_name)
+
+		travelnet.set_travelnets(owner_name, travelnets)
 	end
 	return true, { formspec = travelnet.formspecs.primary }
 end

actions/update_elevator.lua

@@ -54,7 +54,13 @@ return function (node_info, fields, player)
 			)
 	end
 
-	local network = travelnet.get_network(owner_name, station_network)
+	local travelnets = travelnet.get_travelnets(owner_name)
+	local network = travelnets[station_network]
+	if not network then
+		network = {}
+		travelnets[station_name] = network
+	end
+
 	-- does a station with the new name already exist?
 	if network[fields.station_name] then
 		return false, S('Station "@1" already exists on network "@2".',
@@ -84,7 +90,7 @@ return function (node_info, fields, player)
 				tostring(fields.station_name), tostring(station_network), tostring(owner_name)))
 
 	-- save the updated network data in a savefile over server restart
-	travelnet.save_data(owner_name)
+	travelnet.set_travelnets(owner_name, travelnets)
 
 	return true, { formspec = travelnet.formspecs.primary, options = {
 		station_name = fields.station_name

actions/update_travelnet.lua

@@ -70,7 +70,6 @@ return function (node_info, fields, player)
 			)
 	end
 
-	local network
 	local timestamp = os.time()
 	if owner_name ~= fields.owner_name then
 		if not minetest.get_player_privs(player_name)[travelnet.attach_priv]
@@ -82,31 +81,44 @@ return function (node_info, fields, player)
 		-- new owner -> remove station from old network then add to new owner
 		-- but only if there is space on the network
 		-- get the new network
-		network = travelnet.get_or_create_network(fields.owner_name, fields.station_network)
+
+		local old_travelnets = travelnet.get_travelnets(owner_name)
+		local new_travelnets = travelnet.get_travelnets(fields.owner_name)
+
+		local new_network = new_travelnets[fields.station_network]
+		if not new_network then
+			new_network = {}
+			new_travelnets[fields.station_network] = new_network
+		end
+
 		-- does a station with the new name already exist?
-		if network[fields.station_name] then
+		if new_network[fields.station_name] then
 			return false, S('Station "@1" already exists on network "@2" of player "@3".',
 					fields.station_name, fields.station_network, fields.owner_name)
 		end
+
 		-- does the new network have space at all?
-		if travelnet.MAX_STATIONS_PER_NETWORK ~= 0 and 1 + #network > travelnet.MAX_STATIONS_PER_NETWORK then
+		if travelnet.MAX_STATIONS_PER_NETWORK ~= 0 and 1 + #new_network > travelnet.MAX_STATIONS_PER_NETWORK then
 			return false,
 				S('Network "@1", already contains the maximum number (@2) of '
 					.. 'allowed stations per network. Please choose a '
 					.. 'different network name.', fields.station_network,
 						travelnet.MAX_STATIONS_PER_NETWORK)
 		end
+
 		-- get the old network
-		local old_network = travelnet.get_network(owner_name, station_network)
+		local old_network = old_travelnets[station_network]
 		if not old_network then
 			print("TRAVELNET: failed to get old network when re-owning "
 				.. "travelnet/elevator at pos " .. minetest.pos_to_string(pos))
 			return false, S("Station does not have network.")
 		end
+
 		-- remove old station from old network
 		old_network[station_name] = nil
 		-- add new station to new network
-		network[fields.station_name] = { pos = pos, timestamp = timestamp }
+		new_network[fields.station_name] = { pos = pos, timestamp = timestamp }
+
 		-- update meta
 		meta:set_string("station_name",    fields.station_name)
 		meta:set_string("station_network", fields.station_network)
@@ -124,16 +136,27 @@ return function (node_info, fields, player)
 		new_owner_name = fields.owner_name
 		new_station_network = fields.station_network
 		new_station_name = fields.station_name
+
+		travelnet.set_travelnets(owner_name, old_travelnets)
+		travelnet.set_travelnets(fields.owner_name, new_travelnets)
+
 	elseif station_network ~= fields.station_network then
 		-- same owner but different network -> remove station from old network
 		-- but only if there is space on the new network and no other station with that name
 		-- get the new network
-		network = travelnet.get_or_create_network(owner_name, fields.station_network)
+		local travelnets = travelnet.get_travelnets(owner_name)
+		local network = travelnets[fields.station_network]
+		if not network then
+			network = {}
+			travelnets[fields.station_network] = network
+		end
+
 		-- does a station with the new name already exist?
 		if network[fields.station_name] then
 			return false, S('Station "@1" already exists on network "@2".',
 					fields.station_name, fields.station_network)
 		end
+
 		-- does the new network have space at all?
 		if travelnet.MAX_STATIONS_PER_NETWORK ~= 0 and 1 + #network > travelnet.MAX_STATIONS_PER_NETWORK then
 			return false,
@@ -143,7 +166,7 @@ return function (node_info, fields, player)
 						travelnet.MAX_STATIONS_PER_NETWORK)
 		end
 		-- get the old network
-		local old_network = travelnet.get_network(owner_name, station_network)
+		local old_network = travelnets[station_network]
 		if not old_network then
 			print("TRAVELNET: failed to get old network when re-networking "
 				.. "travelnet/elevator at pos " .. minetest.pos_to_string(pos))
@@ -166,9 +189,18 @@ return function (node_info, fields, player)
 
 		new_station_network = fields.station_network
 		new_station_name = fields.station_name
+
+		travelnet.set_travelnets(owner_name, travelnets)
+
 	else
 		-- only name changed -> change name but keep timestamp to preserve order
-		network = travelnet.get_network(owner_name, station_network)
+		local travelnets = travelnet.get_travelnets(owner_name)
+		local network = travelnets[station_network]
+		if not network then
+			network = {}
+			travelnets[station_network] = network
+		end
+
 		-- does a station with the new name already exist?
 		if network[fields.station_name] then
 			return false, S('Station "@1" already exists on network "@2".',
@@ -192,6 +224,7 @@ return function (node_info, fields, player)
 				station_name, fields.station_name, station_network))
 
 		new_station_name = fields.station_name
+		travelnet.set_travelnets(owner_name, travelnets)
 	end
 
 	meta:set_string("infotext",
@@ -203,12 +236,6 @@ return function (node_info, fields, player)
 				tostring(new_owner_name or owner_name)
 			))
 
-	-- save the updated network data
-	travelnet.save_data(owner_name)
-	if new_owner_name then
-		travelnet.save_data(new_owner_name)
-	end
-
 	return true, { formspec = travelnet.formspecs.primary, options = {
 		station_name = new_station_name or station_name,
 		station_network = new_station_network or station_network,

doc/api.md

@@ -21,26 +21,11 @@ Sets and saves the updated travelnet data for the player
 -- retrieve the player-data
 local travelnets = travelnet.get_travelnets(playername)
 -- add a station stub
-travelnets["my_networks"] = {}
--- save the modified data (calls `travelnet.save_data()` to persist the data)
+travelnets["my_network"] = {}
+-- save the modified data
 travelnet.set_travelnets(playername, travelnets)
 ```
 
-## travelnet.save_data(playername)
-
-Saves the runtime travelnet data to disk
-Can be used in place of `travelnet.set_travelnets` to save all player travelnet data that was modified
-
-```lua
--- save 
-local travelnets = travelnet.get_travelnets(playername)
--- call other function that might modify the data
-other_fn(travelnet)
--- call "save_data" directly to persist changes of the runtime data
-travelnet.save_data(playername)
-```
-
-
 ## travelnet.register_travelnet_box
 
 Lets you register your own travelnet boxes with a custom color, name and dye ingredient

init.lua

@@ -25,7 +25,6 @@ end
 travelnet = {}
 
 travelnet.player_formspec_data = {}
-travelnet.targets = {}
 travelnet.path = minetest.get_modpath(minetest.get_current_modname())
 
 local function mod_dofile(filename)
@@ -135,9 +134,9 @@ if travelnet.enable_abm then
 end
 
 -- upon server start, read the savefile
-travelnet.restore_data()
 travelnet.player_formspec_data = nil
 
 if minetest.get_modpath("mtt") and mtt.enabled then
 	mod_dofile("mtt")
+	mod_dofile("persistence.spec")
 end

persistence.lua

@@ -2,57 +2,48 @@ local S = minetest.get_translator("travelnet")
 
 local mod_data_path = minetest.get_worldpath() .. "/mod_travelnet.data"
 
--- called whenever a station is added or removed
-function travelnet.save_data()
-	local data = minetest.write_json(travelnet.targets)
+local storage = minetest.get_mod_storage()
 
-	local success = minetest.safe_file_write(mod_data_path, data)
-	if not success then
-		print(S("[Mod travelnet] Error: Savefile '@1' could not be written.", mod_data_path))
-	end
-end
-
-
-function travelnet.restore_data()
+-- migrate file-based storage to mod-storage
+local function migrate_file_storage()
 	local file = io.open(mod_data_path, "r")
 	if not file then
-		print(S("[Mod travelnet] Error: Savefile '@1' not found.", mod_data_path))
 		return
 	end
 
+	-- load from file
 	local data = file:read("*all")
+	local old_targets
 	if data:sub(1, 1) == "{" then
-		travelnet.targets = minetest.parse_json(data)
+		minetest.log("info", S("[travelnet] migrating from json-file to mod-storage"))
+		old_targets = minetest.parse_json(data)
 	else
-		travelnet.targets = minetest.deserialize(data)
+		minetest.log("info", S("[travelnet] migrating from serialize-file to mod-storage"))
+		old_targets = minetest.deserialize(data)
 	end
 
-	if not travelnet.targets then
-		local backup_file = mod_data_path .. ".bak"
-		print(S("[Mod travelnet] Error: Savefile '@1' is damaged." .. " " ..
-				"Saved the backup as '@2'.", mod_data_path, backup_file))
-
-		minetest.safe_file_write(backup_file, data)
-		travelnet.targets = {}
+	for playername, player_targets in pairs(old_targets) do
+		storage:set_string(playername, minetest.write_json(player_targets))
 	end
-	file:close()
+
+	-- rename old file
+	os.rename(mod_data_path, mod_data_path .. ".bak")
 end
 
--- getter/setter for the legacy `travelnet.targets` table
--- use those methods to access the per-player data, direct table access is deprecated
--- and will be removed in the future
+-- migrate old data as soon as possible
+migrate_file_storage()
 
 -- returns the player's travelnets
-function travelnet.get_travelnets(playername, create)
-	if not travelnet.targets[playername] and create then
-		-- create a new entry
-		travelnet.targets[playername] = {}
+function travelnet.get_travelnets(playername)
+	local json = storage:get_string(playername)
+	if not json or json == "" or json == "null" then
+		-- default to empty object
+		json = "{}"
 	end
-	return travelnet.targets[playername]
+	return minetest.parse_json(json)
 end
 
 -- saves the player's modified travelnets
 function travelnet.set_travelnets(playername, travelnets)
-	travelnet.targets[playername] = travelnets
-	travelnet.save_data(playername)
+	storage:set_string(playername, minetest.write_json(travelnets))
 end

persistence.spec.lua

@@ -0,0 +1,15 @@
+mtt.register("migration", function(callback)
+    local travelnets = travelnet.get_travelnets("singleplayer")
+    assert(type(travelnets) == "table")
+    assert(type(travelnets["net1"]) == "table")
+    assert(type(travelnets["net1"]["station1"]) == "table")
+    assert(type(travelnets["net1"]["station1"].pos) == "table")
+    assert(travelnets["net1"]["station1"].timestamp == 1663767532)
+    assert(travelnets["net1"]["station1"].pos.x == 1022)
+    assert(travelnets["net1"]["station1"].pos.y == 100)
+    assert(travelnets["net1"]["station1"].pos.z == -856)
+
+    travelnet.set_travelnets("singleplayer", travelnets)
+
+    callback()
+end)