lualoader: improve the design of the brand-/logo- mechanism

[kevans]

Summary

In the previous world order, any brand/logo was forced to pull in the drawer and call drawer.add{Brand,Logo} with the name their brand/logo is taking and a table describing it.

In the new world order, these files just need to return a table that maps out graphics types to a table of the exact same format as what was previously being passed back into the drawer. The appeal here is not needing to grab a reference back to the drawer module and having a cleaner data-driven looking format for these. The format has been renamed to 'gfx-*' and each one can provide a logo and a brand.

drawer.addBrand/drawer.addLogo will remain in place until FreeBSD 13, as there's no overhead to them and it's not yet worth the break in compatibility with any pre-existing brands and logos.

[freqlabs]

I like the direction toward the artwork being proper Lua modules, but I think to be more idiomatic we should simply return the table for the module, rather than exporting it as a name. This should simplify loading a bit (should be possible to use try_include?).

[kevans]

In D24966#549765, @freqlabs wrote:

I like the direction toward the artwork being proper Lua modules, but I think to be more idiomatic we should simply return the table for the module, rather than exporting it as a name. This should simplify loading a bit (should be possible to use try_include?).

Sure- I had avoided this because the original author of these files for some unknown reason made them all return true...but this is lua and I can just check the type. It's incredibly unlikely anyone's written something otherwise incompatible with that.

[kevans]

Simplify even further, just return the table. Consolidate a little bit and just pass in the addLogo/addBrand functions to a single processor that does both, since the interfaces are equivalent.

[freqlabs]

stand/lua/logo-orbbw.lua

31

If you want to eventually combine logo and brand into one module, it might be a better to have this be brand or logo as appropriate, instead of orbbw (which is just duplicating information we already have in the module name). Then an artwork module would look like this:

return {
    brand = {
        graphic = {"cool ascii brand art",}
        shift = {x = 0, y = 0},
    },
    logo = {
        graphic = {"cool ascii logo art",},
        shift = {x = 4, y = 2},
    },
}

And loading might go something like this:

local function loadArtwork(filename)
	if name == nil then
		return false, "Missing name"
	end

	-- This would probably need to try a few different filenames
	-- for legacy compatibility.
	local filename = name .. '.lua'
	local ret = try_include(filename)
	if ret == nil then
		return false, "Nothing returned"
	end

	-- Legacy format
	if type(ret) ~= "table" then
		return true
	end

	if ret.brand ~= nil then
		drawer.addBrand(name, ret.brand)
	end
	if ret.logo ~= nil then
		draw.addLogo(name, ret.logo)
	end
end

[kevans]

I lost the branch this was on- oops.

Improvement time! Return a table of graphics types mapped to definitions, have processFile() just generically process the file and iterate over what's been returned and setup as appropriate. All of the logo files have now been renamed to the agnostic gfx-* names and now return the new format, compatibility is maintained for the older ones to give downstreams time to adjust. I'll remove it after FreeBSD 13 branches.

[freqlabs]

stand/lua/drawer.lua

71

"Nothing returned" is not a reasonably descriptive error message.

85

Odd to print the error message return true, instead of returning false and the error message as done above.

95

It would be good to check for false and print the error.

111

Likewise re: error handling

[kevans]

Address error handling concerns (maybe)