Clean up skin listing (#100)

Supersedes the 'fsep' setting by automatically detecting
the texture name in both, the skin list and the updater
scripts.
New, automatically fetched skins will now always use the
'.' delimiter to avoid player name issues.
In case of ambiguous texture names, a warning is logged.

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")

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,75 +1,112 @@
-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
+--- @param path     Path to the "textures" directory, without tailing slash.
-	local name, sort_id, is_preview, playername
+--- @param filename Current file name, such as "player.groot.17.png".
-	local nameparts = string.gsub(fn, "[.]", skins.fsep):split(skins.fsep)
+local function process_skin_texture(path, filename)
+	-- See "textures/readme.txt" for allowed formats
 
-	-- check allowed prefix and file extension
+	local prefix, sep, identifier, extension = filename:match("^(%a+)([_.])([%w_]+)%.(%a+)$")
-	if (nameparts[1] == 'player' or nameparts[1] == 'character') and
+	--[[
-			nameparts[#nameparts]:lower() == 'png' then
+		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
+	-- Filter out files that do not match the allowed patterns
-		table.remove(nameparts, #nameparts)
+	if not extension or extension:lower() ~= "png" then
+		return -- Not a skin texture
+	end
+	if prefix ~= "player" and prefix ~= "character" then
+		return -- Unknown type
+	end
 
-		-- check preview suffix
+	local preview_suffix = sep .. "preview"
-		if nameparts[#nameparts] == 'preview' then
+	if identifier:sub(-#preview_suffix) == preview_suffix then
-			is_preview = true
+		-- skip preview textures
-			table.remove(nameparts, #nameparts)
+		-- This is added by the main skin texture (if exists)
+		return
+	end
+
+	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
 
-		-- Build technically skin name
+	local filename_noext = prefix .. sep .. identifier
-		name = table.concat(nameparts, '_')
 
-		-- Handle metadata from file name
+	dbgprint("Register skin", filename_noext, playername, sort_id)
-		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
-			end
 
-			-- Get sort index
+	-- Register skin texture
-			if tonumber(nameparts[#nameparts]) then
+	local skin_obj = skins.get(filename_noext) or skins.new(filename_noext)
-				sort_id = sort_id + nameparts[#nameparts]
+	skin_obj:set_texture(filename)
-			end
+	skin_obj:set_meta("_sort_id", sort_id)
+	if playername then
+		skin_obj:set_meta("assignment", "player:"..playername)
+		skin_obj:set_meta("playername", playername)
+	end
+
+	do
+		-- 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()
+	skin_obj:set_meta("name", identifier)
+
+	do
+		-- 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)
+			skin_obj:set_meta("name", data[1])
+			skin_obj:set_meta("author", data[2])
+			skin_obj:set_meta("license", data[3])
 		end
+	end
 
-		local skin_obj = skins.get(name) or skins.new(name)
+	do
-		if is_preview then
+		-- Optional preview texture
-			skin_obj:set_preview(fn)
+		local preview_name = filename_noext .. sep .. "preview.png"
-		else
+		local fh = io.open(path .. "/" .. preview_name)
-			skin_obj:set_texture(fn)
+		if fh then
-			skin_obj:set_meta("_sort_id", sort_id)
+			dbgprint("Found preview", preview_name)
-			if playername then
+			skin_obj:set_preview(preview_name)
-				skin_obj:set_meta("assignment", "player:"..playername)
-				skin_obj:set_meta("playername", playername)
-			end
-			local file = io.open(skins.modpath.."/textures/"..fn, "r")
-			local skin_format = skins.get_skin_format(file)
-			skin_obj:set_meta("format", skin_format)
-			file:close()
-			skin_obj:set_hand_from_texture()
-			file = io.open(skins.modpath.."/meta/"..name..".txt", "r")
-			if file then
-				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, ' '))
-			end
 		end
 	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
+		process_skin_texture(skins_path, fn)
+	end
+end
+
 local function skins_sort(skinslist)
 	table.sort(skinslist, function(a,b)
 		local a_id = a:get_meta("_sort_id") or 10000

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 download_path = skins.modpath
-local meta_path = mod_path .. "/meta/"
+local meta_path = download_path .. "/meta/"
-local skins_path = mod_path .. "/textures/"
+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,32 @@
-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:
+List of accepted texture names
-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
 
 Public skin available for all users:
-	character_[number-or-name].png
+	character.[number or name].png
 
-One or multiple private skins for player "nick":
+One or multiple private skins for player "[nick]":
-	player_[nick].png or
+	player.[nick].png
-	player_[nick]_[number-or-name].png
+	player.[nick].[number or name].png
 
-Preview files for public and private skins.
+Skin previews for public and private skins:
-Optional, overrides the generated preview
+	character.[number or name].preview.png
-	character_*_preview.png or
+	player.[nick].preview.png
-	player_*_*_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`.

updater/update_skins.py

@@ -1,9 +1,4 @@
-import sys, requests, base64
+import os.path, sys, requests, base64
-
-# filename seperator to use, either default "-" or ".". see skinsdb/textures/readme.txt
-#fsep = "_"
-fsep = "."
-
 
 
 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"])
+    meta_name = str(json["name"])
-    author = str(json["author"])
+    meta_author = str(json["author"])
-    license = str(json["license"])
+    meta_license = str(json["license"])
-    file = open("../meta/character_" + id + ".txt", "w")
+    file = open("../meta/" + name + ".txt", "w")
-    file.write(name + "\n" + author + "\n" + license + "\n")
+    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!")