Merge pull request #201 from dacmot/main

Fix eating double bug. Fix skinsdb intermittent error.

mod_sources.txt

@@ -228,7 +228,7 @@ origin https://codeberg.org/Wuzzy/minetest_hbarmor.git (fetch)
 Mod: player/hbarmor
 
 origin https://codeberg.org/Wuzzy/minetest_hbhunger.git (fetch)
-* f5c6c945a87ae9ec548eb0fa2ead1bbf9398c5dd Version 1.1.4
+* 3cb245c1499b185ee46ea801afff432d2b3454ad Version 1.1.6
 Mod: player/hbhunger
 
 origin https://github.com/minetest-mods/hbsprint (fetch)
@@ -252,7 +252,7 @@ origin https://repo.or.cz/minetest_show_wielded_item.git (fetch)
 Mod: player/show_wielded_item
 
 origin https://github.com/minetest-mods/skinsdb (fetch)
-* b7cd514cea0c5592ef110ea2b136b8eb703ad8ff API: make skins.register_skin public (#106)
+* df62f2042d061bfb963f35429d8193935d6cb9f4 skinlist: avoid 'Too many open files' errors
 Mod: player/skinsdb
 
 origin https://github.com/bell07/minetest-smart_sfinv (fetch)

mods/player/hbhunger/README.md

@@ -1,6 +1,6 @@
 # Hunger with HUD Bar [`hbhunger`]
 
-* Version: 1.1.4
+* Version: 1.1.6
 
 ## Using the mod
 

mods/player/hbhunger/hunger.lua

@@ -6,21 +6,18 @@ function hbhunger.load_hunger(player)
 	hbhunger.get_hunger_raw(player)
 end
 
--- wrapper for minetest.item_eat (this way we make sure other mods can't break this one)
-local orig_eat = minetest.do_item_eat
-minetest.do_item_eat = function(hp_change, replace_with_item, itemstack, user, pointed_thing)
-	local itemstack_init = ItemStack(itemstack)
-	local itemstack_work = ItemStack(itemstack)
-	-- Force other callbacks from other mods to run first
-	local result = orig_eat(hp_change, replace_with_item, itemstack_init, user, pointed_thing)
-	if result ~= nil then
-		itemstack_work = result
-	else
-		itemstack_work = itemstack_init
-	end
-	-- Our own item eat handler
-	return hbhunger.eat(hp_change, replace_with_item, itemstack_work, user, pointed_thing)
-end
+-- HACK: We register our on_item_eat handler after the other mods have loaded
+-- so their on_item_eat handlers run first. This is because Minetest refuses
+-- to run ANY further on_item_eat handler once one of the callback functions
+-- has returned an itemstack. (as of Minetest 5.8.0)
+-- FIXME: Remove the register_on_mods_loaded as soon Minetest handles
+-- on_item_eat events in a less weird manner.
+minetest.register_on_mods_loaded(function()
+	minetest.register_on_item_eat(function(hp_change, replace_with_item, itemstack, user, pointed_thing)
+		-- Our own item eat handler
+		return hbhunger.eat(hp_change, replace_with_item, ItemStack(itemstack), user, pointed_thing)
+	end)
+end)
 
 -- food functions
 local food = hbhunger.food

mods/player/skinsdb/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

mods/player/skinsdb/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

mods/player/skinsdb/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")
+

mods/player/skinsdb/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
+
+	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)
 
-		-- check preview suffix
-		if nameparts[#nameparts] == 'preview' then
-			is_preview = true
-			table.remove(nameparts, #nameparts)
+		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
+
+	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
 
-		-- 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
-			end
-
-			-- Get sort index
-			if tonumber(nameparts[#nameparts]) then
-				sort_id = sort_id + nameparts[#nameparts]
-			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)