improve

Previously, the players would have their selected skin reset after renaming
the skin textures to the dot separator. This commit implements skin name
migration to ease the transition for server owners.

See 'skins.__fuzzy_match_skin_name' for a detailed explanation.

API.md

@@ -27,6 +27,31 @@ Get all allowed skins for player. All public and all player's private skins. If
 Get all skins with metadata key is set to value. Example:
 skins.get_skinlist_with_meta("playername", playername) - Get all private skins (w.o. public) for playername
 
+## skins.register_skin(path, filename)
+Registers a new skin based on the texture file path specified by `path` and `filename`.
+
+ * `path` (string): points to the parent directory of the texture `filename`.
+   Generally, this should be in the format `mymod.modpath .. "/textures"`.
+ * `filename` (string): full file name, without any path specifications.
+   The file name must adhere to [one of the accepted naming formats](textures/readme.txt).
+
+Note: this function takes the following files into consideration:
+
+1. `<path>/<filename>` (required)
+    * Main skin texture
+2. `<path>/<filenamestem><separator>preview.png` (optional)
+    * Pre-generated preview image
+3. `<path>/../meta/<filenamestem>.txt` (optional)
+    * Metadata regarding the skin
+
+Return values:
+
+ * On failure: `false, reason`
+    * `reason` (string): human readable reason string (similar to `io.open` errors)
+ * On success: `true, key`
+    * `key`: unique skins key for use with e.g. `skins.get(key)` for subsequent
+      fine-tuning of the skin registration.
+
 
 ## skins.new(key, object)
 Create and register a new skin object for given key

api.lua

@@ -2,14 +2,26 @@
 local storage = minetest.get_mod_storage()
 
 function skins.get_player_skin(player)
+	local player_name = player:get_player_name()
 	local meta = player:get_meta()
 	if meta:get("skinsdb:skin_key") then
 		-- Move player data prior July 2018 to mod storage
-		storage:set_string(player:get_player_name(), meta:get_string("skinsdb:skin_key"))
+		storage:set_string(player_name, meta:get_string("skinsdb:skin_key"))
 		meta:set_string("skinsdb:skin_key", "")
 	end
-	local skin = storage:get_string(player:get_player_name())
-	return skins.get(skin) or skins.get(skins.default)
+
+	local skin_name = storage:get_string(player_name)
+	local skin = skins.get(skin_name)
+	if #skin_name > 0 and not skin then
+		-- Migration step to convert `_`-delimited skins to `.` (if possible)
+		skin = skins.__fuzzy_match_skin_name(player_name, skin_name, true)
+		if skin then
+			storage:set_string(player_name, skin:get_key())
+		else
+			storage:set_string(player_name, "")
+		end
+	end
+	return skin or skins.get(skins.default)
 end
 
 -- Assign skin to player

init.lua

@@ -111,3 +111,6 @@ minetest.register_allow_player_inventory_action(function(player, action, inv, da
 		return 0
 	end
 end)
+
+--dofile(skins.modpath.."/unittest.lua")
+

skinlist.lua

@@ -2,10 +2,11 @@ local dbgprint = false and print or function() end
 
 --- @param path     Path to the "textures" directory, without tailing slash.
 --- @param filename Current file name, such as "player.groot.17.png".
-local function process_skin_texture(path, filename)
+--- @return On error: false, error message. On success: true, skin key
+function skins.register_skin(path, filename)
 	-- See "textures/readme.txt" for allowed formats
 
-	local prefix, sep, identifier, extension = filename:match("^(%a+)([_.])([%w_]+)%.(%a+)$")
+	local prefix, sep, identifier, extension = filename:match("^(%a+)([_.])([%w_.]+)%.(%a+)$")
 	--[[
 		prefix:     "character" or "player"
 		sep:        "." (new) or "_" (legacy)
@@ -16,17 +17,21 @@ local function process_skin_texture(path, filename)
 
 	-- Filter out files that do not match the allowed patterns
 	if not extension or extension:lower() ~= "png" then
-		return -- Not a skin texture
+		return false, "invalid skin name"
 	end
 	if prefix ~= "player" and prefix ~= "character" then
-		return -- Unknown type
+		return false, "unknown type"
 	end
 
 	local preview_suffix = sep .. "preview"
 	if identifier:sub(-#preview_suffix) == preview_suffix then
-		-- skip preview textures
-		-- This is added by the main skin texture (if exists)
-		return
+		-- The preview texture is added by the main skin texture (if exists)
+		return false, "preview texture"
+	end
+
+	assert(path)
+	if path == ":UNITTEST:" then
+		path = nil
 	end
 
 	dbgprint("Found skin", prefix, identifier, extension)
@@ -58,12 +63,16 @@ local function process_skin_texture(path, filename)
 	local skin_obj = skins.get(filename_noext) or skins.new(filename_noext)
 	skin_obj:set_texture(filename)
 	skin_obj:set_meta("_sort_id", sort_id)
+	if sep ~= "_" then
+		skin_obj._legacy_name = filename_noext:gsub("[._]+", "_")
+	end
+
 	if playername then
 		skin_obj:set_meta("assignment", "player:"..playername)
 		skin_obj:set_meta("playername", playername)
 	end
 
-	do
+	if path then
 		-- Get type of skin based on dimensions
 		local file = io.open(path .. "/" .. filename, "r")
 		local skin_format = skins.get_skin_format(file)
@@ -74,7 +83,7 @@ local function process_skin_texture(path, filename)
 	skin_obj:set_hand_from_texture()
 	skin_obj:set_meta("name", identifier)
 
-	do
+	if path then
 		-- Optional skin information
 		local file = io.open(path .. "/../meta/" .. filename_noext .. ".txt", "r")
 		if file then
@@ -86,7 +95,7 @@ local function process_skin_texture(path, filename)
 		end
 	end
 
-	do
+	if path then
 		-- Optional preview texture
 		local preview_name = filename_noext .. sep .. "preview.png"
 		local fh = io.open(path .. "/" .. preview_name)
@@ -95,6 +104,36 @@ local function process_skin_texture(path, filename)
 			skin_obj:set_preview(preview_name)
 		end
 	end
+
+	return true, skin_obj:get_key()
+end
+
+--- Internal function. Fallback/migration code for `.`-delimited skin names that
+--- were equipped between d3c7fa7 and 312780c (master branch).
+--- During this period, `.`-delimited skin names were internally registered with
+--- `_` delimiters. This function tries to find a matching skin.
+--- @param player_name (string)
+--- @param skin_name   (string) e.g. `player_foo_mc_bar`
+--- @param be_noisy    (boolean) whether to print a warning in case of mismatches`
+--- @return On match, the new skin (skins.skin_class) or `nil` if nothing matched.
+function skins.__fuzzy_match_skin_name(player_name, skin_name, be_noisy)
+	if select(2, skin_name:gsub("%.", "")) > 0 then
+		-- Not affected by ambiguity
+		return
+	end
+
+	for _, skin in pairs(skins.meta) do
+		if skin._legacy_name == skin_name then
+			dbgprint("Match", skin_name, skin:get_key())
+			return skin
+		end
+		--dbgprint("Try match", skin_name, skin:get_key(), skin._legacy_name)
+	end
+
+	if be_noisy then
+		minetest.log("warning", "skinsdb: cannot find matching skin '" ..
+			skin_name .. "' for player '" .. player_name .. "'.")
+	end
 end
 
 do
@@ -103,7 +142,7 @@ do
 	local skins_dir_list = minetest.get_dir_list(skins_path)
 
 	for _, fn in pairs(skins_dir_list) do
-		process_skin_texture(skins_path, fn)
+		skins.register_skin(skins_path, fn)
 	end
 end
 

textures/readme.txt

@@ -26,7 +26,7 @@ The character `_` is accepted in player names, thus it is not recommended to
 use such file names. For compatibility reasons, they are still recognized.
 
 	character_[number or name].png
-	player_[nick]_png
+	player_[nick].png
 	player_[nick]_[number or name].png
 
 ... and corresponding previews that end in `_preview.png`.

unittest.lua

@@ -0,0 +1,50 @@
+local function get_skin(skin_name)
+	local skin = skins.get(skin_name)
+		or skins.__fuzzy_match_skin_name("(unittest)", skin_name, true)
+	return skin and skin:get_key() or nil
+end
+
+local function run_unittest()
+	local PATH = ":UNITTEST:"
+
+	-- -----
+	-- `.`: Simple register + retrieve operations
+	skins.register_skin(PATH, "player.DotSep.png")
+	skins.register_skin(PATH, "player._DotSep_666_.1.png")
+
+	assert(get_skin("player.DotSep"))
+	assert(get_skin("player._DotSep_666_.1"))
+	assert(get_skin("player.DotSep.1") == nil)
+
+	-- -----
+	-- Ambiguous skin names (filenames without extension). Register + retrieve
+	skins.new("player_AmbSki")
+	skins.new("player_AmbSki_1")
+	skins.new("player_AmbSki_666_1")
+
+	assert(get_skin("player_AmbSki"))
+	assert(get_skin("player_AmbSki_") == nil)
+	assert(get_skin("player_AmbSki_1"))
+	assert(get_skin("player_AmbSki_666_1"))
+	-- There are no `__` patterns as they were silently removed by string.split
+
+
+	-- -----
+	-- Mod Storage backwards compatibility
+	-- Match the old `_` notation to `.`-separated skins
+	skins.register_skin(PATH, "player.ComPat42.png")
+	skins.register_skin(PATH, "player.ComPat42.5.png")
+	skins.register_skin(PATH, "player._Com_Pat_42.png")
+	skins.register_skin(PATH, "player._Com_Pat_42.1.png")
+
+	assert(get_skin("player_ComPat42") == "player.ComPat42")
+	assert(get_skin("player_ComPat42_5") == "player.ComPat42.5")
+	assert(get_skin("player_Com_Pat_42") == "player._Com_Pat_42")
+	assert(get_skin("player_Com_Pat_42_1") == "player._Com_Pat_42.1")
+
+
+	error("Unittest passed! Please disable them now.")
+end
+
+run_unittest()
+