Merge branch 'master' into contentdb

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

@@ -8,12 +8,6 @@ skins = {}
 skins.modpath = minetest.get_modpath(minetest.get_current_modname())
 skins.default = "character"
 
--- see skindsdb/textures/readme.txt to avoid playername with underscore problem
-skins.fsep = minetest.settings:get("skinsdb_fsep") or "_"
-if skins.fsep == "_" then
-	minetest.log("warning", "skinsdb filename seperator is set to " .. skins.fsep .. ", see skindsdb/textures/readme.txt to avoid problems with playernames containing underscore")
-end
-
 dofile(skins.modpath.."/skin_meta_api.lua")
 dofile(skins.modpath.."/api.lua")
 dofile(skins.modpath.."/skinlist.lua")
@@ -117,3 +111,6 @@ minetest.register_allow_player_inventory_action(function(player, action, inv, da
 		return 0
 	end
 end)
+
+--dofile(skins.modpath.."/unittest.lua")
+

settingtypes.txt

@@ -1,3 +0,0 @@
-# texture filename seperator, default "_"
-# see skindsdb/textures/readme.txt to avoid playername with underscore problem
-skinsdb_fsep (texture filename seperator) enum _ _,.

skinlist.lua

@@ -1,72 +1,150 @@
-local skins_dir_list = minetest.get_dir_list(skins.modpath.."/textures")
+local dbgprint = false and print or function() end
 
-for _, fn in pairs(skins_dir_list) do
-	local name, sort_id, is_preview, playername
-	local nameparts = string.gsub(fn, "[.]", skins.fsep):split(skins.fsep)
+--- @param path     Path to the "textures" directory, without tailing slash.
+--- @param filename Current file name, such as "player.groot.17.png".
+--- @return On error: false, error message. On success: true, skin key
+function skins.register_skin(path, filename)
+	-- See "textures/readme.txt" for allowed formats
 
-	-- check allowed prefix and file extension
-	if (nameparts[1] == 'player' or nameparts[1] == 'character') and
-			nameparts[#nameparts]:lower() == 'png' then
+	local prefix, sep, identifier, extension = filename:match("^(%a+)([_.])([%w_.-]+)%.(%a+)$")
+	--[[
+		prefix:     "character" or "player"
+		sep:        "." (new) or "_" (legacy)
+		identifier: number, name or (name + sep + number)
+			^ previews are explicity skipped
+		extension:  "png" only due `skins.get_skin_format`
+	]]
 
-		-- cut filename extension
-		table.remove(nameparts, #nameparts)
-
-		-- check preview suffix
-		if nameparts[#nameparts] == 'preview' then
-			is_preview = true
-			table.remove(nameparts, #nameparts)
+	-- Filter out files that do not match the allowed patterns
+	if not extension or extension:lower() ~= "png" then
+		return false, "invalid skin name"
+	end
+	if prefix ~= "player" and prefix ~= "character" then
+		return false, "unknown type"
 	end
 
-		-- Build technically skin name
-		name = table.concat(nameparts, '_')
-
-		-- Handle metadata from file name
-		if not is_preview then
-			-- Get player name
-			if nameparts[1] == "player" then
-				playername = nameparts[2]
-				table.remove(nameparts, 1)
-				sort_id = 0
-			else
-				sort_id = 5000
+	local preview_suffix = sep .. "preview"
+	if identifier:sub(-#preview_suffix) == preview_suffix then
+		-- The preview texture is added by the main skin texture (if exists)
+		return false, "preview texture"
 	end
 
-			-- Get sort index
-			if tonumber(nameparts[#nameparts]) then
-				sort_id = sort_id + nameparts[#nameparts]
-			end
+	assert(path)
+	if path == ":UNITTEST:" then
+		path = nil
 	end
 
-		local skin_obj = skins.get(name) or skins.new(name)
-		if is_preview then
-			skin_obj:set_preview(fn)
-		else
-			skin_obj:set_texture(fn)
+	dbgprint("Found skin", prefix, identifier, extension)
+
+	local sort_id    -- number, sorting "rank" in the skin list
+	local playername -- string, if player-specific
+	if prefix == "player" then
+		-- Allow "player.PLAYERNAME.png" and "player.PLAYERNAME.123.png"
+		local splits = identifier:split(sep)
+
+		playername = splits[1]
+		-- Put in front
+		sort_id = 0 + (tonumber(splits[2]) or 0)
+
+		if #splits > 1 and sep == "_" then
+			minetest.log("warning", "skinsdb: The skin name '" .. filename .. "' is ambigous." ..
+				" Please use the separator '.' to lock it down to the correct player name.")
+		end
+	else -- Public skin "character*"
+		-- Less priority
+		sort_id = 5000 + (tonumber(identifier) or 0)
+	end
+
+	local filename_noext = prefix .. sep .. identifier
+
+	dbgprint("Register skin", filename_noext, playername, sort_id)
+
+	-- Register skin texture
+	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
-			local file = io.open(skins.modpath.."/textures/"..fn, "r")
+
+	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)
 		skin_obj:set_meta("format", skin_format)
 		file:close()
+	end
+
 	skin_obj:set_hand_from_texture()
-			file = io.open(skins.modpath.."/meta/"..name..".txt", "r")
+	skin_obj:set_meta("name", identifier)
+
+	if path then
+		-- Optional skin information
+		local file = io.open(path .. "/../meta/" .. filename_noext .. ".txt", "r")
 		if file then
+			dbgprint("Found meta")
 			local data = string.split(file:read("*all"), "\n", 3)
-				file:close()
 			skin_obj:set_meta("name", data[1])
 			skin_obj:set_meta("author", data[2])
 			skin_obj:set_meta("license", data[3])
-			else
-				-- remove player / character prefix if further naming given
-				if nameparts[2] and not tonumber(nameparts[2]) then
-					table.remove(nameparts, 1)
-				end
-				skin_obj:set_meta("name", table.concat(nameparts, ' '))
+			file:close() -- do not rely on delayed GC
 		end
 	end
+
+	if path then
+		-- Optional preview texture
+		local preview_name = filename_noext .. sep .. "preview.png"
+		local fh = io.open(path .. "/" .. preview_name)
+		if fh then
+			dbgprint("Found preview", preview_name)
+			skin_obj:set_preview(preview_name)
+			fh:close() -- do not rely on delayed GC
+		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
+	-- Load skins from the current mod directory
+	local skins_path = skins.modpath.."/textures"
+	local skins_dir_list = minetest.get_dir_list(skins_path)
+
+	for _, fn in pairs(skins_dir_list) do
+		skins.register_skin(skins_path, fn)
 	end
 end
 

skins_updater.lua

@@ -50,9 +50,9 @@ end
 local root_url = "http://skinsdb.terraqueststudios.net"
 local page_url = root_url .. "/api/v1/content?client=mod&page=%i" -- [1] = Page#
 
-local mod_path = skins.modpath
-local meta_path = mod_path .. "/meta/"
-local skins_path = mod_path .. "/textures/"
+local download_path = skins.modpath
+local meta_path = download_path .. "/meta/"
+local skins_path = download_path .. "/textures/"
 
 -- Fancy debug wrapper to download an URL
 local function fetch_url(url, callback)
@@ -80,14 +80,22 @@ local function unsafe_file_write(path, contents)
 end
 
 -- Takes a valid skin table from the Skins Database and saves it
-local function safe_single_skin(skin)
+local function save_single_skin(skin)
 	local meta = {
 		skin.name,
 		skin.author,
 		skin.license
 	}
 
-	local name =  "character" .. skins.fsep .. skin.id
+	local name = "character." .. skin.id
+	do
+		local legacy_name = "character_" .. skin.id
+		local fh = ie.io.open(skins_path .. legacy_name .. ".png", "r")
+		-- Use the old name if either the texture ...
+		if fh then
+			name = legacy_name
+		end
+	end
 
 	-- core.safe_file_write does not work here
 	unsafe_file_write(
@@ -128,7 +136,7 @@ internal.fetch_function = function(pages_total, start_page, len)
 				assert(skin.id ~= "")
 
 				if skin.id ~= 1 then -- Skin 1 is bundled with skinsdb
-					safe_single_skin(skin)
+					save_single_skin(skin)
 				end
 			end
 

textures/readme.txt

@@ -1,25 +1,33 @@
-In this folder the skin files could be placed according the following file naming convention.
+This location is where you can put your custom skins.
 
-skinsdb uses an underscore as default seperator for filename splitting which can cause problems with playernames containing "_",
-see https://github.com/minetest-mods/skinsdb/issues/54.
-The config setting skinsdb_fsep (texture filename seperator) was added as a workaround which also offers "."(dot) as seperator,
-dot is the only character which is allowed in textures but not in playernames.
-To keep compatibility with older versions underscore is the default value.
 
-fresh install:
-you should change the seperator to "." to avoid that problem.
-existing install:
-- change the filenames according to the naming convention with dot as seperator instead of underscore
-- change the texture filename seperator in settings or add "skinsdb_fsep = ." to your minetest.conf before starting your server
+List of accepted texture names
+------------------------------
 
 Public skin available for all users:
-	character_[number-or-name].png
+	character.[number or name].png
+	^ The allowed characters in "[number or name]" are "[A-z0-9_.-]+".
 
-One or multiple private skins for player "nick":
-	player_[nick].png or
-	player_[nick]_[number-or-name].png
+One or multiple private skins for player "[nick]":
+	player.[nick].png
+	player.[nick].[number or name].png
 
-Preview files for public and private skins.
-Optional, overrides the generated preview
-	character_*_preview.png or
-	player_*_*_preview.png
+Skin previews for public and private skins:
+	character.[number or name].preview.png
+	player.[nick].preview.png
+	player.[nick].[number or name].preview.png
+
+	Note: This is optional and overrides automatically generated preciewws.
+
+
+Legacy texture names
+--------------------
+
+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]_[number or name].png
+
+... and corresponding previews that end in `_preview.png`.

unittest.lua

@@ -0,0 +1,51 @@
+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
+	assert(skins.register_skin(PATH, "player.DotSep.png"))
+	assert(skins.register_skin(PATH, "player._DotSep_666_.1.png"))
+	assert(skins.register_skin(PATH, "character._DotSep_With-Dash-.png"))
+
+	assert(get_skin("player.DotSep"))
+	assert(get_skin("player._DotSep_666_.1"))
+	assert(get_skin("player.DotSep.1") == nil)
+	assert(get_skin("character._DotSep_With-Dash-"))
+
+	-- -----
+	-- 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()
+

updater/update_skins.py

@@ -1,9 +1,4 @@
-import sys, requests, base64
-
-# filename seperator to use, either default "-" or ".". see skinsdb/textures/readme.txt
-#fsep = "_"
-fsep = "."
-
+import os.path, sys, requests, base64
 
 
 print("Downloading skins from skinsdb.terraqueststudio.net ...")
@@ -22,21 +17,27 @@ print("Writing skins")
 for json in data["skins"]:
     id = str(json["id"])
 
+    name = "character." + id
+    if True:
+        legacy_name = "character_" + id
+        if os.path.exists("../textures/" + legacy_name + ".png"):
+            name = legacy_name
+
+
     # Texture file
     raw_data = base64.b64decode(json["img"])
-    file = open("../textures/character" + fsep + id + ".png", "wb")
+    file = open("../textures/" + name + ".png", "wb")
     file.write(bytearray(raw_data))
     file.close()
 
     # Meta file
-    name = str(json["name"])
-    author = str(json["author"])
-    license = str(json["license"])
-    file = open("../meta/character_" + id + ".txt", "w")
-    file.write(name + "\n" + author + "\n" + license + "\n")
+    meta_name = str(json["name"])
+    meta_author = str(json["author"])
+    meta_license = str(json["license"])
+    file = open("../meta/" + name + ".txt", "w")
+    file.write(meta_name + "\n" + meta_author + "\n" + meta_license + "\n")
     file.close()
-    print("Added #%s Name: %s Author: %s License: %s" % (id, name, author, license))
+    print("Added #%s Name: %s Author: %s License: %s" % (id, meta_name, meta_author, meta_license))
     count += 1
 
-
 print("Fetched " + str(count) + " skins!")