diff --git a/API.md b/API.md index acb51d0..c60c760 100644 --- a/API.md +++ b/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. `/` (required) + * Main skin texture +2. `/preview.png` (optional) + * Pre-generated preview image +3. `/../meta/.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 diff --git a/api.lua b/api.lua index b5a8211..c408810 100644 --- a/api.lua +++ b/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 diff --git a/init.lua b/init.lua index 976c014..836f2bc 100644 --- a/init.lua +++ b/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") + diff --git a/settingtypes.txt b/settingtypes.txt deleted file mode 100644 index e47be76..0000000 --- a/settingtypes.txt +++ /dev/null @@ -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 _ _,. \ No newline at end of file diff --git a/skinlist.lua b/skinlist.lua index dc8a155..66045ed 100644 --- a/skinlist.lua +++ b/skinlist.lua @@ -1,73 +1,151 @@ -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) + -- 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 - -- check preview suffix - if nameparts[#nameparts] == 'preview' then - is_preview = true - table.remove(nameparts, #nameparts) + 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 + + assert(path) + if path == ":UNITTEST:" then + path = nil + 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 - name = table.concat(nameparts, '_') + local filename_noext = prefix .. sep .. identifier - -- 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 - end + dbgprint("Register skin", filename_noext, playername, sort_id) - -- Get sort index - if tonumber(nameparts[#nameparts]) then - sort_id = sort_id + nameparts[#nameparts] - end + -- 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 + + 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() + 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) + skin_obj:set_meta("name", data[1]) + skin_obj:set_meta("author", data[2]) + skin_obj:set_meta("license", data[3]) + file:close() -- do not rely on delayed GC end + 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) - 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 - 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 + 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 local function skins_sort(skinslist) diff --git a/skins_updater.lua b/skins_updater.lua index 61b99d5..41bb48e 100644 --- a/skins_updater.lua +++ b/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 diff --git a/textures/readme.txt b/textures/readme.txt index e53e112..73885f1 100644 --- a/textures/readme.txt +++ b/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`. diff --git a/unittest.lua b/unittest.lua new file mode 100644 index 0000000..bf476d2 --- /dev/null +++ b/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() + diff --git a/updater/update_skins.py b/updater/update_skins.py index 00249e6..4c1c533 100644 --- a/updater/update_skins.py +++ b/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!")