Move around some group code and make the craftguide render well without a recipe

This also keeps recipes aligned to the right, close to the arrow.
It also calculates the craft's height.

api.lua

@@ -107,20 +107,29 @@ function unified_inventory.register_craft(options)
 	table.insert(unified_inventory.crafts_table[itemstack:get_name()],options)
 end
 
+
 local craft_type_defaults = {
 	width = 3,
 	height = 3,
 	uses_crafting_grid = false,
 }
-function unified_inventory.canonicalise_craft_type(name, options)
-	if not options.description then options.description = name end
+
+
+function unified_inventory.craft_type_defaults(name, options)
+	if not options.description then
+		options.description = name
+	end
 	setmetatable(options, {__index = craft_type_defaults})
 	return options
 end
+
+
 function unified_inventory.register_craft_type(name, options)
-	unified_inventory.registered_craft_types[name] = unified_inventory.canonicalise_craft_type(name, options)
+	unified_inventory.registered_craft_types[name] =
+			unified_inventory.craft_type_defaults(name, options)
 end
 
+
 unified_inventory.register_craft_type("normal", {
 	description = "Crafting",
 	width = 3,
@@ -128,6 +137,7 @@ unified_inventory.register_craft_type("normal", {
 	uses_crafting_grid = true,
 })
 
+
 unified_inventory.register_craft_type("shapeless", {
 	description = "Mixing",
 	width = 3,
@@ -135,26 +145,26 @@ unified_inventory.register_craft_type("shapeless", {
 	uses_crafting_grid = true,
 })
 
+
 unified_inventory.register_craft_type("cooking", {
 	description = "Cooking",
 	width = 1,
 	height = 1,
 })
 
+
 unified_inventory.register_craft_type("digging", {
 	description = "Digging",
 	width = 1,
 	height = 1,
 })
 
-function unified_inventory.register_group_representative_item(groupname, itemname)
-	unified_inventory.registered_group_representative_items[groupname] = itemname
-end
 
 function unified_inventory.register_page(name, def)
 	unified_inventory.pages[name] = def
 end
 
+
 function unified_inventory.register_button(name, def)
 	if not def.action then
 		def.action = function(player)
@@ -165,9 +175,11 @@ function unified_inventory.register_button(name, def)
 	table.insert(unified_inventory.buttons, def)
 end
 
+
 function unified_inventory.is_creative(playername)
 	if minetest.check_player_privs(playername, {creative=true}) or
 	   minetest.setting_getbool("creative_mode") then
 		return true
 	end
 end
+

group.lua

@@ -0,0 +1,66 @@
+
+unified_inventory.registered_group_items = {
+	mesecon_conductor_craftable = "mesecons:wire_00000000_off",
+	wool = "wool:white",
+}
+
+function unified_inventory.register_group_item(groupname, itemname)
+	unified_inventory.registered_group_items[groupname] = itemname
+end
+
+
+-- This is used when displaying craft recipes, where an ingredient is
+-- specified by group rather than as a specific item.  A single-item group
+-- is represented by that item, with the single-item status signalled
+-- in the "sole" field.  If the group contains no items at all, the item
+-- field will be nil.
+--
+-- Within a multiple-item group, we prefer to use an item that has the
+-- same specific name as the group, and if there are more than one of
+-- those items we prefer the one registered for the group by a mod.
+-- Among equally-preferred items, we just pick the one with the
+-- lexicographically earliest name.
+
+function compute_group_item(group_name)
+	local candidate_items = {}
+	for itemname, itemdef in pairs(minetest.registered_items) do
+		if (itemdef.groups.not_in_creative_inventory or 0) == 0 and
+				(itemdef.groups[group_name] or 0) ~= 0 then
+			table.insert(candidate_items, itemname)
+		end
+	end
+	local num_candidates = #candidate_items
+	if num_candidates == 0 then
+		return {sole = true}
+	elseif num_candidates == 1 then
+		return {item = candidate_items[1], sole = true}
+	end
+	local bestitem = ""
+	local bestpref = 0
+	for _, item in ipairs(candidate_items) do
+		local pref
+		if item == unified_inventory.registered_group_items[group_name] then
+			pref = 3
+		elseif item:gsub("^[^:]+:", "") == group_name then
+			pref = 2
+		else
+			pref = 1
+		end
+		if pref > bestpref or (pref == bestpref and item < bestitem) then
+			bestitem = item
+			bestpref = pref
+		end
+	end
+	return {item = bestitem, sole = false}
+end
+
+
+local group_item_cache = {}
+
+function unified_inventory.get_group_item(group_name)
+	if not group_item_cache[group_name] then
+		group_item_cache[group_name] = compute_group_item(group_name)
+	end
+	return group_item_cache[group_name]
+end
+

init.lua

@@ -12,7 +12,6 @@ unified_inventory.current_searchbox = {}
 unified_inventory.current_index = {}
 unified_inventory.current_item = {}
 unified_inventory.registered_craft_types = {}
-unified_inventory.registered_group_representative_items = {}
 unified_inventory.crafts_table = {}
 unified_inventory.crafts_table_count = 0
 unified_inventory.players = {}
@@ -39,6 +38,7 @@ if creative_inventory then
 end
 
 dofile(modpath.."/datastorage.lua")
+dofile(modpath.."/group.lua")
 dofile(modpath.."/api.lua")
 dofile(modpath.."/internal.lua")
 dofile(modpath.."/callbacks.lua")

register.lua

@@ -134,106 +134,39 @@ unified_inventory.register_page("craft", {
 	end,
 })
 
--- group_representative_item(): select representative item for a group
---
--- This is used when displaying craft recipes, where an ingredient is
--- specified by group rather than as a specific item.  A single-item group
--- is represented by that item, with the single-item status signalled
--- so that stack_image_button() can treat it as just the item.  If the
--- group contains no items at all, it will be treated as containing a
--- single unknown item.
---
--- Within a multiple-item group, we prefer to use an item that has the
--- same specific name as the group, and if there are more than one of
--- those items we prefer the one specified by the default mod if there
--- is one.  If this produces a bad result, the mod defining a group can
--- register its preference for which item should represent the group,
--- and we'll use that instead if possible.  Also, for a handful of groups
--- (predating this registration system) we have built-in preferences
--- that are used like registered preferences.  Among equally-preferred
--- items, we just pick the one with the lexicographically earliest name,
--- for determinism.
-local builtin_group_representative_items = {
-	mesecon_conductor_craftable = "mesecons:wire_00000000_off",
-	stone = "default:cobble",
-	wool = "wool:white",
-}
-local function compute_group_representative_item(groupspec)
-	local groupname = string.sub(groupspec, 7)
-	local candidate_items = {}
-	for itemname, itemdef in pairs(minetest.registered_items) do
-		if (itemdef.groups.not_in_creative_inventory or 0) == 0 and (itemdef.groups[groupname] or 0) ~= 0 then
-			table.insert(candidate_items, itemname)
-		end
-	end
-	if #candidate_items == 0 then return { item = "unobtainium!", sole = true } end
-	if #candidate_items == 1 then return { item = candidate_items[1], sole = true } end
-	local bestitem = ""
-	local bestpref = 0
-	for _, item in ipairs(candidate_items) do
-		local pref
-		if item == unified_inventory.registered_group_representative_items[groupname] then
-			pref = 5
-		elseif item == builtin_group_representative_items[groupname] then
-			pref = 4
-		elseif item == "default:"..groupname then
-			pref = 3
-		elseif item:gsub("^[^:]*:", "") == groupname then
-			pref = 2
-		else
-			pref = 1
-		end
-		if pref > bestpref or (pref == bestpref and item < bestitem) then
-			bestitem = item
-			bestpref = pref
-		end
-	end
-	return { item = bestitem, sole = false }
-end
-local group_representative_item_cache = {}
-local function group_representative_item(groupspec)
-	if not group_representative_item_cache[groupspec] then
-		group_representative_item_cache[groupspec] = compute_group_representative_item(groupspec)
-	end
-	return group_representative_item_cache[groupspec]
-end
 
 -- stack_image_button(): generate a form button displaying a stack of items
 --
 -- Normally a simple item_image_button[] is used.  If the stack contains
 -- more than one item, item_image_button[] doesn't have an option to
 -- display an item count in the way that an inventory slot does, so
--- we have to fake it using the label facility.  This doesn't let us
--- specify that the count should appear at bottom right, so we use some
--- dodgy whitespace to shift it away from the centre of the button.
--- Unfortunately the correct amount of whitespace depends on display
--- resolution, so the results from this will be variable.  This should be
--- replaced as soon as the engine adds support for a proper item count,
--- or at least label placement control, on buttons.
+-- we have to fake it using the label facility.
 --
 -- The specified item may be a group.  In that case, the group will be
 -- represented by some item in the group, along with a flag indicating
 -- that it's a group.  If the group contains only one item, it will be
 -- treated as if that item had been specified directly.
-local function stack_image_button(x, y, w, h, buttonname_prefix, stackstring)
-	local st = ItemStack(stackstring)
-	local specitem = st:get_name()
-	local c = st:get_count()
-	local clab = c == 1 and "       " or string.format("%7d", c)
-	local gflag, displayitem, selectitem
-	if string.sub(specitem, 1, 6) == "group:" then
-		local gri = group_representative_item(specitem)
-		gflag = not gri.sole
-		displayitem = gri.item
-		selectitem = gri.sole and gri.item or specitem
-	else
-		gflag = false
-		displayitem = specitem
-		selectitem = specitem
+
+local function stack_image_button(x, y, w, h, buttonname_prefix, item)
+	local name = item:get_name()
+	local count = item:get_count()
+	local show_is_group = false
+	local displayitem = name
+	local selectitem = name
+	if name:sub(1, 6) == "group:" then
+		local group_name = name:sub(7)
+		local group_item = unified_inventory.get_group_item(group_name)
+		show_group = not group_item.sole
+		displayitem = group_item.item or "unknown"
+		selectitem = group_item.sole and displayitem or name
 	end
-	local label = string.format("\n\n%s%7d", gflag and "G" or "  ", c):gsub(" 1$", " .")
-	if label == "\n\n        ." then label = "" end
-	return "item_image_button["..x..","..y..";"..w..","..h..";"..minetest.formspec_escape(displayitem)..";"..minetest.formspec_escape(buttonname_prefix..selectitem)..";"..label.."]"
+	-- Hackily shift the count to the bottom right
+	local shiftstr = "\n\n        "
+	return string.format("item_image_button[%u,%u;%u,%u;%s;%s;%s]",
+			x, y, w, h,
+			minetest.formspec_escape(displayitem),
+			minetest.formspec_escape(buttonname_prefix..selectitem),
+			count ~= 1 and shiftstr..tostring(count) or "")
 end
 
 unified_inventory.register_page("craftguide", {
@@ -241,14 +174,15 @@ unified_inventory.register_page("craftguide", {
 		local player_name = player:get_player_name()
 		local formspec = ""
 		formspec = formspec.."background[0,4.5;8,4;ui_main_inventory.png]"
+		formspec = formspec.."background[0,1;8,3;ui_craftguide_form.png]"
 		formspec = formspec.."label[0,0;Crafting Guide]"
 		formspec = formspec.."listcolors[#00000000;#00000000]"
 		local craftinv = minetest.get_inventory({
 			type = "detached",
 			name = player_name.."craftrecipe"
 		})
-		local item_name = unified_inventory.current_item[player_name]
-		if not item_name then return {formspec=formspec} end
+		local item_name = unified_inventory.current_item[player_name] or ""
+
 		formspec = formspec.."textarea[0.3,0.6;10,1;;Result: "..minetest.formspec_escape(item_name)..";]"
 		formspec = formspec.."list[detached:"..minetest.formspec_escape(player_name).."craftrecipe;output;6,1;1,1;]"
 
@@ -259,38 +193,47 @@ unified_inventory.register_page("craftguide", {
 			alternates = #crafts
 			craft = crafts[alternate]
 		end
-		if not craft then
-			craftinv:set_stack("output", 1, item_name)
-			formspec = formspec.."label[6,3.35;No recipes]"
-			return {formspec=formspec}
-		end
 
-		formspec = formspec.."background[0,1;8,3;ui_craftguide_form.png]"
-		craft_type = unified_inventory.registered_craft_types[craft.type] or unified_inventory.canonicalise_craft_type(craft.type, {})
-		formspec = formspec.."label[6,3.35;Method:]"
-		formspec = formspec.."label[6,3.75;"..minetest.formspec_escape(craft_type.description).."]"
+		if craft then
 			craftinv:set_stack("output", 1, craft.output)
-
-		-- fake buttons just to make grid
-		for y = 1, craft_type.height do
-		for x = 1, craft_type.width do
-			formspec = formspec.."image_button["
-				..(1.0 + x)..","..(0.0 + y)..";1.1,1.1;ui_blank_image.png;;]"
-		end
+			craft_type = unified_inventory.registered_craft_types[craft.type] or
+					unified_inventory.craft_type_defaults(craft.type, {})
+			formspec = formspec.."label[6,3.35;Method:]"
+			formspec = formspec.."label[6,3.75;"
+					..minetest.formspec_escape(craft_type.description).."]"
+		else
+			craftinv:set_stack("output", 1, item_name)
+			craft_type = unified_inventory.craft_type_defaults("", {})
+			formspec = formspec.."label[6,3.35;No recipes]"
 		end
 
-		local width = craft.width
+		local width = craft and craft.width or 0
 		if width == 0 then
 			-- Shapeless recipe
 			width = craft_type.width
 		end
 
+		local height = craft_type.height
+		if craft then
+			height = math.ceil(table.maxn(craft.items) / width)
+		end
+
 		local i = 1
-		for y = 1, craft_type.height do
+		-- This keeps recipes aligned to the right,
+		-- so that they're close to the arrow.
+		local xoffset = 1 + (3 - width)
+		for y = 1, height do
 		for x = 1, width do
-			local item = craft.items[i]
+			local item = craft and craft.items[i]
 			if item then
-				formspec = formspec..stack_image_button(1.0+x, 0.0+y, 1.1, 1.1, "item_button_", item)
+				formspec = formspec..stack_image_button(
+						xoffset + x, y, 1.1, 1.1,
+						"item_button_", ItemStack(item))
+			else
+				-- Fake buttons just to make grid
+				formspec = formspec.."image_button["
+					..tostring(xoffset + x)..","..tostring(y)
+					..";1,1;ui_blank_image.png;;]"
 			end
 			i = i + 1
 		end
@@ -303,7 +246,7 @@ unified_inventory.register_page("craftguide", {
 					.."button[7.2,2.5;0.6,0.5;craftguide_craft_max;All]"
 		end
 
-		if alternates > 1 then
+		if alternates and alternates > 1 then
 			formspec = formspec.."label[0,2.6;Recipe "
 					..tostring(alternate).." of "
 					..tostring(alternates).."]"