Page MenuHomeFreeBSD

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

Authored by kevans on May 22 2020, 8:45 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 5, 6:24 PM
Unknown Object (File)
Thu, Nov 28, 8:15 AM
Unknown Object (File)
Thu, Nov 21, 11:43 AM
Unknown Object (File)
Nov 19 2024, 7:55 AM
Unknown Object (File)
Nov 19 2024, 7:52 AM
Unknown Object (File)
Nov 19 2024, 4:33 AM
Unknown Object (File)
Nov 19 2024, 2:24 AM
Unknown Object (File)
Nov 14 2024, 11:58 PM
Subscribers

Details

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.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kevans retitled this revision from lualoader: imrpove the design of the brand-/logo- mechanism to lualoader: improve the design of the brand-/logo- mechanism .May 22 2020, 8:53 PM

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?).

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 edited the summary of this revision. (Show Details)

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.

stand/lua/logo-orbbw.lua
31 ↗(On Diff #72263)

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 edited the summary of this revision. (Show Details)

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.

stand/lua/drawer.lua
71 ↗(On Diff #77654)

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

85 ↗(On Diff #77654)

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

101 ↗(On Diff #77654)

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

122 ↗(On Diff #77654)

Likewise re: error handling

kevans marked 2 inline comments as done.

Address error handling concerns (maybe)

This revision is now accepted and ready to land.Oct 4 2020, 10:29 PM