Many quality of life improvements (#22)

Put the name of the check in the "QA check ... started." and "QA check ... finished."
Enable logging output to a file
Add many configuration parameters to control various settings like logging to chat, logging to a file, and log file options
Sort the outputs of several quality checks so that it can be easily compared with a previous check for regressions

README.md

@@ -1,18 +1,19 @@
-QA Block to run checking scripts for beter quality assurance of code
+QA Block to run checking scripts for better quality assurance of code
 =======
 
 License GPL-V3: https://www.gnu.org/licenses/gpl-3.0.html
 
-This is a developer helper mod, allow run any lua code for testing reason. The mod can list and run lua-scripts placed in checks subfolder. some check scrips provided.
-The second part is able to display global lua-tables tree
+This is a developer helper mod. It allows to run any lua code for testing reason. The mod can list and run lua-scripts placed in `checks` subfolder. Some check scripts are provided with the mod.
+The second part allows to display global lua-tables variables tree for debugging.
 
-# Features
-- redirection of print() output to minetest chat. (can be disabled by changing code line "print_to_chat")
+## Features
+- redirection of print() output to minetest chat
+- redirection of print() output to a log file in the world directory. The output is sorted so that it can be compared to find regressions
 - robust call of the scripts trough "pcall" does not crash the game in case of syntax- or runtime errors
-- all functionality available trough chat commands and the QA-Block node
+- all functionality available through chat commands and the QA-Block node
 - refresh and list the checks script list at runtime
 - edit the code before calling them
-- type code and run them
+- type code and run it
 - explore global variables/lua tables
 
 ![Screenshot](screenshot_20170121_012152.png)
@@ -20,27 +21,32 @@ The second part is able to display global lua-tables tree
 
 https://forum.minetest.net/viewtopic.php?f=11&t=15759
 
-# Dependencies
+## Dependencies
 - none
   - smartfs(provided) - GUI for check selection and manipulation. Optional, but without smartfs there is limited functionality 
 
 
-# Provided check modules
+## Provided check modules
+- broken_recipe - Find crafting recipes which require unknown items
 - empty - Empty file for your own checks
+- get_item_csv - Export all registered items in a .CSV file
 - global_variables - List suspicious global variables
+- graphviz_recipes_all - Make a graphviz .dot file of all items in a recipe dependency tree
 - is_ground_content - This checker lists all nodes for which is_ground_content == true
-- light_source_15 - List all nodes for which light_source >= 15
 - list_entities - Lists all the registered entities (except builtin)
 - list_spawning_mobs - List entities that are mobs from mobs_redo or compatible framework
+- no_doc_items_help - Lists all items without item usage help or long description
 - no_item_description - Lists all items without description
+- no_sounds - Find nodes that don't have any sounds associated when dug, walked on, etc.
+- redundant_items - Lists items which seem redundant
 - same_recipe - Find duplicate crafting recipes
 - unobtainable_items - Lists items which seem to be unobtainable
 - useless_items - Lists all items which are probably useless
 
-# How to use:
+## How to use:
 add the mod to the game you like to test
 
-## Using chat command /qa
+### Using chat command /qa
 - /qa help - print available chat commands
 - /qa ls - list all available check modules
 - /qa set checkname - set default check
@@ -48,7 +54,7 @@ add the mod to the game you like to test
 - /qa checkname - run check
 - /qa - run default check
 
-## Using the block
+### Using the block
 1. get the QA-Block from creative inventory
 2. place the block somewhere
 3a - without smartfs - wait till the default check is finished and the block disappears
@@ -56,6 +62,13 @@ add the mod to the game you like to test
 
 In all cases - check the debug.txt for test results
 
+### Minetest Configuration Parameters
+- `qa_block.print_to_chat`, bool. Output QA check messages to chat
+- `qa_block.log_to_file`: bool. Output QA check messages to a file
+- `qa_block.overwrite_log`: bool. Overwrite the file at every game launch
+- `qa_block.log_date_time`: bool. Prepend a date and time stamp to log messages
+- `qa_block.date_and_time_format`: string. Date and time stamp format
 
-# Credits
-- Wuzzy2 - thanks for ideas, code optimizations and the most check scripts
+## Credits
+- [Wuzzy2](https://codeberg.org/Wuzzy) - thanks for ideas, code optimizations and most check scripts
+- [dacmot](https://www.github.com/dacmot) - for adding output to log file and configuration parameters

checks/broken_recipe.lua

@@ -0,0 +1,72 @@
+-- Find crafting recipes which require unknown items or groups
+
+-- If true, will also check groups
+local check_groups = true
+local pairsByKeys = qa_block.pairsByKeys
+
+local known_bad_items = {}
+local known_groups = {}
+local known_bad_groups = {}
+
+local function item_exists(itemstring)
+	if itemstring == "" then
+		return true
+	elseif minetest.registered_items[itemstring] then
+		return true
+	end
+	return false
+end
+
+local function group_exists(groupname)
+	return known_groups[groupname] == true
+end
+
+-- Get groups
+if check_groups then
+	for name, def in pairs(minetest.registered_items) do
+		if def.groups then
+			for g,r in pairs(def.groups) do
+				known_groups[g] = true
+			end
+		end
+	end
+end
+
+local check_item = function(itemstring, bad_item_msg, bad_group_msg, is_output)
+	local item = ItemStack(itemstring):get_name()
+	local modname = qa_block.modutils.get_modname_by_itemname(item)
+	if modname ~= "group" then
+		if not item_exists(item) and not known_bad_items[item] then
+			known_bad_items[item] = true
+			print(bad_item_msg .. ": \"" .. item .. "\"")
+		end
+	elseif check_groups then
+		local groupstr = string.sub(item, 7)
+		local groups = string.split(groupstr, ",")
+		if is_output and #groups > 0 then
+			print(bad_group_msg .. ". Full string: \""..item.."\")")
+			return
+		end
+		for g in pairsByKeys(groups) do
+			local group = groups[g]
+			if not group_exists(group) and not known_bad_groups[group] then
+				print(bad_group_msg .. ": \"" .. group .. "\" (full string: \""..item.."\")")
+				known_bad_groups[group] = true
+			end
+		end
+	end
+end
+
+-- Check recipes for unknown items and groups
+for name, def in pairsByKeys(minetest.registered_items) do
+	local recipes_for_item = minetest.get_all_craft_recipes(name)
+	for id, recipe in pairsByKeys(recipes_for_item) do
+		for i in pairsByKeys(recipe.items) do
+			local item = recipe.items[i]
+			if item and item ~= "" then
+				check_item(item, "Unknown input item", "Input group without any items")
+			end
+		end
+	end
+end
+

checks/global_variables.lua

@@ -18,6 +18,7 @@ local blacklist = {
 	getfenv = true,
 	getmetatable = true,
 	ipairs = true,
+	jit = true,
 	load = true,
 	loadfile = true,
 	loadstring = true,
@@ -58,6 +59,7 @@ local blacklist = {
 	core = true,
 	dump = true,
 	dump2 = true,
+	Raycast = true,
 }
 
 -- part of minetest builtin, but needs to be discussed if it right or wrong

checks/graphviz_recipes_all.lua

@@ -0,0 +1,31 @@
+local cache = smart_inventory.cache
+local crecipes = smart_inventory.crecipes
+
+local filename = minetest.get_worldpath().."/recipes.dot"
+local file = io.open(filename, "w")
+
+-- header
+file:write("digraph G {\nrankdir=LR;\n")
+
+-- dependencies
+local touched_connections = {}
+
+for itemname, citem in pairs(smart_inventory.cache.citems) do
+	local root_name = string.gsub(itemname, ":", "_")
+	if not touched_connections[itemname] then
+		touched_connections[itemname] = {}
+	end
+	for _, recipe in ipairs(cache.citems[itemname].in_craft_recipe) do
+		if crecipes.crecipes[recipe] then
+			local child_itemname = string.gsub(crecipes.crecipes[recipe].out_item.name, ":", "_")
+			if not touched_connections[itemname][child_itemname] then
+				file:write(root_name.." -> "..child_itemname..";\n")
+				touched_connections[itemname][child_itemname] = true
+			end
+		end
+	end
+end
+
+-- close file
+file:write('}\n')
+file:close()

checks/list_groups.lua

@@ -0,0 +1,44 @@
+-- Lists all the groups that items are in
+
+-- If true, will also list all items that are members of the group
+local list_group_members = false
+
+local all_groups = {}
+for id, def in pairs(minetest.registered_items) do
+	if def.groups then
+		for group, rating in pairs(def.groups) do
+			if rating ~= 0 then
+				if not all_groups[group] then
+					all_groups[group] = {}
+				end
+				table.insert(all_groups[group], id)
+			end
+		end
+	end
+end
+
+-- Sort groups
+local flat_groups = {}
+for group, _ in pairs(all_groups) do
+	table.insert(flat_groups, group)
+end
+table.sort(flat_groups)
+
+-- Output
+for g=1, #flat_groups do
+	local group = flat_groups[g]
+	local items = all_groups[group]
+	if list_group_members then
+		-- Print all groups and the group members
+		print("Group \""..group.."\":")
+		table.sort(items)
+		for i=1, #items do
+			-- Print item name and group rating
+			local def = minetest.registered_items[items[i]]
+			print("  " .. items[i] .. " (" .. def.groups[group]..")")
+		end
+	else
+		-- Print groups only
+		print("Group \""..group.."\": "..(#items).." item(s)")
+	end
+end

checks/no_item_description.lua

@@ -2,11 +2,16 @@
 
 -- Setting the description is optional, but recommended.
 
+local t = {}
 for name, def in pairs(minetest.registered_items) do
 	-- Hand gets a free pass
 	if name ~= "" then
 		if (def.description == "" or def.description == nil) and def.groups.not_in_creative_inventory ~= 1 then
-			print(name)
+			table.insert(t, name)
 		end
 	end
 end
+table.sort(t)
+for i=1, #t do
+	print(t[i])
+end

checks/no_sounds.lua

@@ -2,7 +2,7 @@
 
 -- Sounds are always optional, but most nodes SHOULD have sounds for good quality.
 
-for name, def in pairs(minetest.registered_nodes) do
+for name, def in qa_block.pairsByKeys(minetest.registered_nodes) do
 	local complaints = {}
 	local failed = false
 	-- Air and ignore are allowed to be silent

checks/redundant_items.lua

@@ -11,7 +11,7 @@ local blacklist = {
 	wool = true,
 }
 
-for item, def in pairs(minetest.registered_items) do
+for item, def in qa_block.pairsByKeys(minetest.registered_items) do
 	if item:find(':') then
 		local mod_name, item_name = unpack(item:split(':'))
 		if not blacklist[mod_name] and not def.groups.not_in_creative_inventory then

checks/same_recipe.lua

@@ -13,10 +13,7 @@ local enable_dependency_check = true
 local print_no_recipe = false
 -------------
 
-local modutils
-if enable_dependency_check then
-	modutils = dofile(minetest.get_modpath("qa_block").."/modutils.lua")
-end
+local modutils = qa_block.modutils
 
 local function dependency_exists(item1, item2)
 
@@ -134,7 +131,7 @@ local known_recipes = {}
 
 -- load and execute file each click
 --for name, def in pairs(minetest.registered_nodes) do
-for name, def in pairs(minetest.registered_items) do
+for name, def in qa_block.pairsByKeys(minetest.registered_items) do
 
 	if (not def.groups.not_in_creative_inventory or
 	   def.groups.not_in_creative_inventory == 0) and

checks/unobtainable_items.lua

@@ -26,7 +26,7 @@ for item, def in pairs(minetest.registered_items) do
 				local dropname = dropstack:get_name()
 				items[dropname] = true
 			end
-		elseif type(def.drop) == "table" then
+		elseif type(def.drop) == "table" and def.drop.items then
 			for i=1,#def.drop.items do
 				for j=1,#def.drop.items[i].items do 
 					items[def.drop.items[i].items[j]] = true
@@ -36,7 +36,7 @@ for item, def in pairs(minetest.registered_items) do
 	end
 end
 
-for item, obtainable in pairs(items) do
+for item, obtainable in qa_block.pairsByKeys(items) do
 	if obtainable == false then
 		print(item)
 	end

checks/useless_items.lua

@@ -14,7 +14,7 @@ for k,v in pairs(minetest.registered_items) do
 				if recp.width == 0 then
 					table_length = #recp.items
 				else
-					table_length = math.pow(recp.width, 2)
+					table_length = recp.width * recp.width
 				end
 				for i=1, table_length do
 					if recp.items[i] ~= nil then
@@ -80,9 +80,9 @@ local check = function(name, def)
 	print(name)
 end
 
-for name, def in pairs(minetest.registered_tools) do
+for name, def in qa_block.pairsByKeys(minetest.registered_tools) do
 	check(name, def)
 end
-for name, def in pairs(minetest.registered_craftitems) do
+for name, def in qa_block.pairsByKeys(minetest.registered_craftitems) do
 	check(name, def)
 end

init.lua

@@ -2,14 +2,23 @@
 -- Some hardcoded settings and constants
 -----------------------------------------------
 local defaultmodule = "empty"
-local print_to_chat = true
+
+local print_to_chat = minetest.settings:get_bool("qa_block.print_to_chat", true)
+local log_to_file = minetest.settings:get_bool("qa_block.log_to_file", false)
+local overwritelog = minetest.settings:get_bool("qa_block.overwrite_log", false)
+local logdatetime = minetest.settings:get_bool("qa_block.log_date_time", false)
+local datetimeformat = minetest.settings:get("qa_block.date_time_format") or "%Y-%m-%d %H:%M:%S"
+
+local logfilename = minetest.get_worldpath() .. "/qa_block.log"
 
 -----------------------------------------------
 -- Load external libs and other files
 -----------------------------------------------
 qa_block = {}
 qa_block.modpath = minetest.get_modpath(minetest.get_current_modname())
-local filepath = qa_block.modpath.."/checks/"
+qa_block.modutils = dofile(qa_block.modpath.."/modutils.lua")
+
+local checks_path = qa_block.modpath.."/checks/"
 
 --[[ --temporary buildin usage (again)
 local smartfs_enabled = false
@@ -25,7 +34,33 @@ end
 local smartfs = dofile(qa_block.modpath.."/smartfs.lua")
 qa_block.smartfs = smartfs
 dofile(qa_block.modpath.."/smartfs_forms.lua")
-smartfs_enabled = true
+local smartfs_enabled = true
+
+
+-----------------------------------------------
+-- Return a list of keys sorted - Useful when looking for regressions
+-- https://www.lua.org/pil/19.3.html
+-- Additional helper to be used in checks
+-----------------------------------------------
+function qa_block.pairsByKeys (t)
+	if not t then
+		return function()
+			return nil
+		end
+	end
+
+	local a = {}
+	for n in pairs(t) do table.insert(a, n) end
+	table.sort(a)
+	local i = 0      -- iterator variable
+	local iter = function ()   -- iterator function
+			i = i + 1
+			if a[i] == nil then return nil
+			else return a[i], t[a[i]]
+			end
+	end
+	return iter
+end
 
 -----------------------------------------------
 -- QA-Block functionality - list checks
@@ -33,7 +68,7 @@ smartfs_enabled = true
 qa_block.get_checks_list = function()
 	local out = {}
 	local files
-	files = minetest.get_dir_list(filepath, false)
+	files = minetest.get_dir_list(checks_path, false)
 	for f=1, #files do
 		local filename = files[f]
 		local outname, _ext = filename:match("(.*)(.lua)$")
@@ -44,22 +79,57 @@ qa_block.get_checks_list = function()
 end
 
 -----------------------------------------------
--- QA-Block functionality - redefine print - reroute output to chat window
+-- QA-Block functionality - write log message to a file
 -----------------------------------------------
-if print_to_chat then
+function qa_block.write_log(msg)
+	local f = io.open(logfilename, "a")
+	if f then
+		if logdatetime then
+			f:write("[")
+			f:write(os.date(datetimeformat))
+			f:write("]  ")
+		end
+		f:write(msg)
+		f:write("\n")
+		f:close()
+	else
+		minetest.log("error", "could not open chatlog file for writing: " .. logfilename)
+	end
+end
+
+-----------------------------------------------
+-- QA-Block functionality - redefine print - reroute output to chat window and/or log file
+-----------------------------------------------
+if print_to_chat or log_to_file then
+
 	local function do_print_redefinition()
+
 		local old_print = print
 		print = function(...)
-			local outsting = ""
+			local outstring = ""
 			local out
 			local x
 			for x, out in ipairs({...}) do
-				outsting = (outsting..tostring(out)..'\t')
+				outstring = (outstring..tostring(out)..'\t')
 			end
-			old_print(outsting)
-			minetest.chat_send_all(outsting)
+			old_print(outstring)
+			if print_to_chat then
+				minetest.chat_send_all(outstring)
+			end
+			if log_to_file then
+				qa_block.write_log(outstring)
 			end
 		end
+
+		if overwritelog then
+			local f = io.open(logfilename, "w")
+			if f then
+				f:close()
+			end
+		end
+
+	end
+
 	minetest.after(0, do_print_redefinition)
 end
 
@@ -67,7 +137,7 @@ end
 -- QA-Block functionality - get the source of a module
 -----------------------------------------------
 function qa_block.get_source(check)
-	local file = filepath..check..".lua"
+	local file = checks_path..check..".lua"
 	local f=io.open(file,"r")
 	if not f then
 		return ""

settingtypes.txt

@@ -0,0 +1,5 @@
+qa_block.print_to_chat (Output qa check messages to chat) bool true
+qa_block.log_to_file (Output qa check messages to a file) bool false
+qa_block.overwrite_log (Overwrite log file on game launch) bool false
+qa_block.log_date_time (Prepend a date and time stamp to log messages) bool false
+qa_block.date_time_format (Date and time stamp format) string %Y-%m-%d %H:%M:%S

smartfs.lua

@@ -75,9 +75,9 @@ end
 -- Smartfs Interface - Returns the name of an installed and supported inventory mod that will be used above, or nil
 ------------------------------------------------------
 function smartfs.inventory_mod()
-	if unified_inventory then
+	if minetest.global_exists("unified_inventory") then
 		return "unified_inventory"
-	elseif inventory_plus then
+	elseif minetest.global_exists("inventory_plus") then
 		return "inventory_plus"
 	else
 		return nil
@@ -255,6 +255,7 @@ smartfs._ldef.nodemeta = {
 							local meta = minetest.get_meta(state.location.pos)
 							meta:set_string("formspec", state:_buildFormspec_(true))
 							meta:set_string("smartfs_name", state.def.name)
+							meta:mark_as_private("smartfs_name")
 						end
 					end, state)
 				end
@@ -424,12 +425,6 @@ function smartfs._makeState_(form, params, location, newplayer)
 		return self
 	end
 
-	local compat_is_inv
-	if location.type == "inventory" then
-		compat_is_inv = true
-	else
-		compat_is_inv = false
-	end
 
 	------------------------------------------------------
 	-- State - create returning state object
@@ -439,8 +434,8 @@ function smartfs._makeState_(form, params, location, newplayer)
 		def = form,
 		players = _make_players_(newplayer),
 		location = location,
-		is_inv = compat_is_inv, -- obsolete / compatibility
-		player = newplayer, -- obsolete / compatibility
+		is_inv = (location.type == "inventory"), -- obsolete. Please use location.type="inventory" instead
+		player = newplayer, -- obsolete. Please use location.player
 		param = params or {},
 		get = function(self,name)
 			return self._ele[name]
@@ -473,7 +468,7 @@ function smartfs._makeState_(form, params, location, newplayer)
 			end
 			for key,val in pairs(self._ele) do
 				if val:getVisible() then
-					res = res .. val:getBackgroundString() .. val:build()
+					res = res .. val:getBackgroundString() .. val:build() .. val:getTooltipString()
 				end
 			end
 			return res
@@ -513,20 +508,24 @@ function smartfs._makeState_(form, params, location, newplayer)
 		end,
 		-- Receive fields and actions from formspec
 		_sfs_on_receive_fields_ = function(self, player, fields)
-			-- fields assignment
+
+			local fields_todo = {}
 			for field, value in pairs(fields) do
 				local element = self:_get_element_recursive_(field)
 				if element then
-					element:setValue(value)
+					fields_todo[field] = { element = element, value = value }
 				end
 			end
-			-- process onInput hooks
+
+			for field, todo in pairs(fields_todo) do
+				todo.element:setValue(todo.value)
+			end
+
 			self:_sfs_process_oninput_(fields, player)
-			-- do actions
-			for field, value in pairs(fields) do
-				local element = self:_get_element_recursive_(field)
-				if element and element.submit then
-					element:submit(value, player)
+
+			for field, todo in pairs(fields_todo) do
+				if todo.element.submit then
+					todo.element:submit(todo.value, player)
 				end
 			end
 			-- handle key_enter
@@ -536,7 +535,7 @@ function smartfs._makeState_(form, params, location, newplayer)
 					element:submit_key_enter(fields[fields.key_enter_field], player)
 				end
 			end
-			-- handle quit/exit
+
 			if not fields.quit and not self.closed and not self.obsolete then
 				self:show()
 			else
@@ -657,6 +656,19 @@ function smartfs._makeState_(form, params, location, newplayer)
 				setValue = function(self, value)
 					self.data.value = value
 				end,
+				setTooltip = function(self,text)
+					self.data.tooltip = minetest.formspec_escape(text)
+				end,
+				getTooltip = function(self)
+					return self.data.tooltip
+				end,
+				getTooltipString = function(self)
+					if self.data.tooltip then
+						return "tooltip["..self:getAbsName()..";"..self:getTooltip().."]"
+					else
+						return ""
+					end
+				end,
 			}
 
 			ele.data.visible = true --visible by default
@@ -879,9 +891,6 @@ smartfs.element("button", {
 		end
 		specstring = specstring..self:getAbsName()..";"..
 				minetest.formspec_escape(self.data.value).."]"
-		if self.data.tooltip then
-			specstring = specstring.."tooltip["..self:getAbsName()..";"..self.data.tooltip.."]"
-		end
 		return specstring
 	end,
 	submit = function(self, field, player)
@@ -915,12 +924,6 @@ smartfs.element("button", {
 	getItem = function(self)
 		return self.data.item
 	end,
-	setTooltip = function(self,text)
-		self.data.tooltip = minetest.formspec_escape(text)
-	end,
-	getTooltip = function(self)
-		return self.data.tooltip
-	end,
 	setClose = function(self,bool)
 		self.data.closes = bool
 	end,

smartfs_forms.lua

@@ -34,18 +34,21 @@ local function _check_selection_dialog(state)
 	update_fileslist(listbox)
 
 	listbox:onClick(function(self, state, index)
-		state:get("textarea"):setText(qa_block.get_source(self:getItem(index)))
+		qa_block.selection = self:getItem(index)
+		state:get("textarea"):setText(qa_block.get_source(qa_block.selection))
 	end)
 
 	listbox:onDoubleClick(function(self,state, index)
-		state:get("textarea"):setText(qa_block.get_source(self:getItem(index)))
-		qa_block.do_module(self:getItem(index))
+		qa_block.selection = self:getItem(index)
+		state:get("textarea"):setText(qa_block.get_source(qa_block.selection))
+		qa_block.do_module(qa_block.selection)
 	end)
 
 	-- Run Button
 	local runbutton = state:button(10,7,2,0.5,"Run","Run")
 	runbutton:onClick(function(self)
-		qa_block.do_source(textarea:getText(), "from textarea")
+		local check = qa_block.selection or ""
+		qa_block.do_source(textarea:getText(), check.." from textarea")
 	end)
 
 	-- Refersh Button

storage.lua

@@ -8,6 +8,7 @@ function storage.new(location)
 		if self.location.type == "nodemeta" then
 			local meta = minetest.get_meta(self.location.pos)
 			meta:set_string("qa_block_data", minetest.serialize(self.data))
+			meta:mark_as_private("qa_block_data")
 		end
 	end