monome / norns

norns is many sound instruments.
http://monome.org
GNU General Public License v3.0
630 stars 145 forks source link

lua module organization #87

Closed catfact closed 6 years ago

catfact commented 6 years ago

coming back to norns lua, i found it highly confusing to navigate the stuff in lua/ and to understand what is happening on launch.

all the new stuff has really broken modularity, seems a little haphazardly arranged to my eyes, and lacks documentation.

to take just one example, file.lua has luadoc declaring it as a module, but it's not structured like a module at all (there's not even a table called file), and in fact defines brand new tables in norns and functions in those tables (norns.script.foo, norns.state.foo, &c)... why aren't "State" and "Script" broken out into modules? (or if that seems like overkill, put in norns.lua?)

startup.lua is another problem zone. i figured this as a place for users to conveniently put shared setup for ll their scripts - e.g. customize what their imports look like, such as e = require 'engine' (people have very different tastes re: terseness vs readibiliity.) so it should maybe properly go in norns/lua/scripts because user scripts will depend on these decisions. (and i guess the idea is for the scripts/ dir to be installed in a different manner at some point maybe)

but the current startup.lua does some other random (not heavy) things like (again) defining some new entries in the top-level norns module.

and hey, if we don't want to use modules at all, that's fine, let's say so, but we need some way of knowing where some norns.foo is defined (especially if it has no luadoc) and some general shared model of how things are structured. totally freeform lua is like javascript - it immediately turns into spaghetti if we're not a little bit careful.

tehn commented 6 years ago

yes, it's confusing as there's no documentation and it has been work in progress.

and yes, there are several files that are not modules-- just breaking up functionality.

the boot/startup process is necessarily complicated to establish a sort of boilerplate environment that facilitates clean script loading and co-existence of the menu system. prior to adding a bunch of this each user script required a lot of redundant housekeeping. the current state of things has a lot of default behavior which makes user scripts very lightweight-- closer to teletype in some ways-- which was the goal.

and yeah, i've been treating a lot of the lua like structs and function pointers-- i'd need to better understand what i means (and why) the move to modules makes sense?

i apologize it's a mess-- i'll restructure and document right away. hopefully the template.lua is sufficient to actually use the system as is in the meantime.

catfact commented 6 years ago

well, it's on me too- there are a couple of weird-looking layered decisions in the basic system.

the weirdest one is that there is a norns table, and it has entries like norns.grid, that is just an alias to the grid module. and the grid module itself updates that entry when it is loaded.

the norns table is declared directly with global scope, instead of local Norns = {}, return Norns. so it's not really a module in the usual sense, and doesn't function like one - it acts more like a global namespace. (and maybe i should remove the module keyword from its luadoc declaration.)

there is a single, specific reason for this - the norns table holds functions that we want to call from C, which have to be available to the LVM at global scope. i don't see any way around this. the odd wrapping arrangement is to maintain separation of concerns: grid.lua defines global functions in norns such as norns.grid.add, because that module is responsible for maintaining a list of available grids, so this function is the concern of that module. if i change something about the implementation of the grid listing, i want to be able to change all concerned functions in the same place.

user scripts and other parts of the system should probably not use norns.foo functions to access system modules, but instead make local aliases directly to those modules with require - as we do.

making global, terse aliases in startup.lua also seems fine - this is a necesssary compromise to make them easily available in the REPL for live development.

i went through kind of a lot of work to get to this arrangement; odd as it is i think it is a pretty good one.


what are modules? they are just tables that are organized in a specific way so that their functionality can be safely imported into scripts using require. there are actually several different patterns for this, i just settled on what seemed like the most obvious one.


why use modules? the usual reasons - separation of concerns and scope.

scope example: in file.lua you define some functions like scandir (a file-specific utilty) and tablelength ( a general utility.) lacking the "local" keyword, these are global definitions.

this is a stupid example, but let's say i have a user script that redefines scandir at global scope. scandir in file.lua will be overwritten and anything relying on it will break!

if you make a "file" module and structure it the same as the others (local File = {}, File.scandir = function() { ... }, return File) then this can't happen. scanning directories is a concern of the File module and is done with a function scoped to that module. if my script wants to scan a directory, it is totally natural to import the File module and use a function from it: local file = require 'file', local dirs = file.scandir('someplace')

now, i'm not saying everything needs to be a module. stuff that really should execute globally, exactly once, or has no reason to ever be imported from another script, doesn't need to be structured as a module. (an example is config.lua, where package and script locations are declared. ) that circumstance should be pretty uncommon.

but File, Script and State all seem like good candidates.


the boilerplate functionality is fine and good, i agree it's necessary, it just isn't easy to understand right now (or to maintain / modify.)

anyways, don't worry about working on this right away - it's definitely usable right now. it just took me two tries of sitting down with it to understand the flow of logic in the template script - which replies on functionality defined in startup.lua and file.lua. all i want to do is use the system to test some new engine stuff, and use the encoders &c.


i am of course completely open to suggestions on how to structure everything differently, more concisely, more DRYly, &c.

tehn commented 6 years ago

totally agreed on the points made. things like global 'scandir' were quick fixes that need proper placement, so i'll do that.

also helpful, i'm going to make a dev readme that outlines how startup works, what lua is called when etc. doing that now.

catfact commented 6 years ago

ok the TL/DR is: really think twice before adding stuff to the global norns table. that is really for functions that have to be global because they are called from C, it's not just a grab-bag of global variables. (*)

i'd strongly consider making components like system.lua, script.lua &c, into modules. and startup.lua should mostly be a bunch of imports.

in addition to being good for easily navigating the lua source, this is less typing for users. in startup:sys = require 'system', and in scripts: power = sys.power, instead of prefixing everything with norns.

( * i realize that this is different from the structure i first proposed, before i knew anything about lua. )

catfact commented 6 years ago

@tehn really, don't put too much time into documenting the template / startup sequence right now. it is not that hard to figure out, it just takes more grepping the source than is good for end users i think.

i can take a pass at cleaning things up, to give some idea. i would rather put my limited hours into the audio side and SC glue, but cleaning up the lua won't take long.

tehn commented 6 years ago

oh i already outlined the startup sequence, it's 10 lines, will push it later, was super quick. just for reference.

i've started in on cleaning things up but again want to minimize the footprint on global namespace. ie, made "log" into a class. so now

log = require log

which is fine but i'm considering in startup.lua

sys = {}
sys.log = require 'log'
sys.file = require 'file'

etc, to keep these common keywords free. but another thing i'm trying to figure out-- (i'm going to rename "map" -> "menu" (duh)) say the menu needs to use the log messages. should the menu just be loaded at the end of startup.lua and use sys.log directly?

menu doesn't need to be a module, given it's basically a management script in itself, correct?


and yes definitely let me fix this stuff, your time is so much better spent elsewhere

catfact commented 6 years ago

i'm gonna close this ranty issue and make new ones, but i'm in a hurrry, so notes here for now

management scripts

menu doesn't need to be a module, given it's basically a management script in itself, correct?

sure, i was thinking of having something like norns/boostrap.lua <- or init.lua or whatever- run-once stuff that the system needs norns/scripts/startup.lua <- run-once stuff that the user wants, e,g, customized import names, or their own custom boilerplate. it will be run just before event loop starts ticking

global variables in scripts.

they are ok in something like startup.lua that will only be run once in program lifetime. but if you let scripts add global variables freely, then runnign different scripts will cause global environment table to grow arbtirarily large, which isn't good.

however, we dno't want users to have to declare varfiables with local.

solution: in the script module or whatever, when user script is run, give it a new environment cloned from the global environment, and delete that when the script is unloaded. http://lua-users.org/wiki/EnvironmentsTutorial (not 100% sure this will work when changing entries cloned from _G, but that should be easy to test/workaround)

globals outside of script

i'm also adding a globals.lua, to declare things that modules expect to be defined like key and engine and whatever. IMO these should all be put in one place so they can have luadoc tags.

tehn commented 6 years ago

all of this makes sense and is a good direction.

you mentioned adding globals.lua already? shall i hold off until you push something? i've started module-izing but i don't want to create conflicts....

catfact commented 6 years ago

eh no, don't hold off if you've a mind. i'll just be more careful with branch/merge stretegy

catfact commented 6 years ago

was it decided that i should encapsulate the whole menu system inside of the norns namespace?

i don't think i was trying to suggest that. i think menu actually looks really good right now.

primary reason for global norns namespace is to hold all the handler functions that weaver needs to call.

for example norns.key is called from weaver. (why isn't norns.key defined in norns.lua? where is it defined? how is it glued to the global key callback that user scripts redefine? where is that defined? &c)

i think there's very good reason for not allowing user scripts to touch norns namespace directly at all. by doing so they can e.g. break all the c->lua glue, break menu, break power button, &c.

that's why when a handler/callback function should be redefined by a script, i tend to put it in a module and clearly mark it. because maybe the top-level handler should be doing something else in addition to running a script-defined callback (this is true for engine command reports, for example)

tehn commented 6 years ago

it's much better now. i'm still up for suggestions, new issues are fine.

one thing i will do is put descriptions at the top of each file and then add a module overview inside of norns.lua.