Open Nick-Mazuk opened 2 years ago
That compare() function is one I wrote, so now I'm wondering if there's some other 'compare' that you're referring to that I wasn't aware of. The function I wrote iterates through the table and returns true if it matches the input code (or one of the input codes). Maybe I can rename it compare_code()
? Anyway, that lets you input a bunch of different things to get the same function, so for example entering a starting dynamic of piano used to be code 0007... But now you can input '0007', 'p', or 'piano' and they will all execute the function.
As for the other thing...
I was actually wondering about the need for a middleman function for a lot of these, which is the way CJ has it set up. In the example above, 0403 calls up barline_right_dashed() and 0404 calls up barline_right_thick(). BUT, if you were to look at those, I'd bet good money that those are essentially just small little launcher functions along the lines of barline("right", "dashed")
. I've been wondering if that's really necessary, and just having 0403 = barline("right", "dashed")
.
Taking out those middleman functions alone would probably eliminate hundreds of lines of code. CJ, your thoughts?
Occasionally there are a few functions launched by a single code... Nick, how would that situation be handled using the code_map table above? Perhaps in those instances the middleman function is truly necessary?
Finally, I have now been properly schooled by Patterson and realize that I could do:
if compare_code({"0402", "||"}) then
and NOT compare explicitly to true
!!!
Is there a fast and efficient way to assign multiple variables (or keys) to the same value or function? I just tried this, so I know it doesn't work:
local x, y, z = 42
x is 42, but y and z come out as nil
... But is there a way to assign them all at once? So far Google hasn't turned anything up...
Anyway, that lets you input a bunch of different things to get the same function, so for example entering a starting dynamic of piano used to be code 0007... But now you can input '0007', 'p', or 'piano' and they will all execute the function.
I see. I might do something like this:
code_map["0403"] = add_piano_dynamic()
code_map["p"] = add_piano_dynamic()
code_map["piano"] = add_piano_dynamic()
I was actually wondering about the need for a middleman function for a lot of these, which is the way CJ has it set up. In the example above, 0403 calls up barline_right_dashed() and 0404 calls up barline_right_thick(). BUT, if you were to look at those, I'd bet good money that those are essentially just small little launcher functions along the lines of
barline("right", "dashed")
. I've been wondering if that's really necessary, and just having0403 = barline("right", "dashed")
.
I would personally be in favor of this change. Though it's much closer to a stylistic change than a good vs. bad change.
My personal preference is to write as little code as possible without sacrificing readability. Every line of code you write is a liability that you need to maintain.
Occasionally there are a few functions launched by a single code... Nick, how would that situation be handled using the code_map table above? Perhaps in those instances the middleman function is truly necessary?
Yeah, for these I'd probably create a wrapper function. Have a single code point to the wrapper function, and have the wrapper function figure out what else to do.
For my reference, do you have an example off hand? From my understanding, the code is the only input the JetStream.lua gets to decide what to do. So how else might you figure out which functions to run if the code is the same?
Finally, I have now been properly schooled by Patterson and realize that I could do:
if compare_code({"0402", "||"}) then
and NOT compare explicitly totrue
!!!
Yep! I saw that thread in one of your other PRs so I didn't bring it up here. Would have been totally absurd of me to expect that you'd immediately go back and fix every single instance of that in this repo!
Is there a fast and efficient way to assign multiple variables (or keys) to the same value or function? I just tried this, so I know it doesn't work:
local x, y, z = 42
x is 42, but y and z come out asnil
... But is there a way to assign them all at once? So far Google hasn't turned anything up...
In Lua, you need to provide a value for each one:
local x, y, z = 42, 42, 42
https://www.lua.org/pil/4.1.html
Some other languages (for instance, JavaScript) allow you to chain assignments together as shown below, though this is generally considered bad practice as it hurts readability.
let a = b = c = 42
That compare() function is one I wrote, so now I'm wondering if there's some other 'compare' that you're referring to that I wasn't aware of.
I was referring to the compare function you wrote.
How do you add those pretty lines of code? When I do the 'code' button it's just this boring gray stuff...
You've got reds and blues going, plus the 'copy' button on the right!
```lua
-- Write your lua code here
-- the "lua" after the ``` tells GitHub that this is Lua code
-- the ``` tells GitHub that this is a code block
-- the code block ends at the second ```
-- try copy-pasting this code block into a new comment and viewing the "Preview" tab
Here are examples of what I feel are a bit superfluous middleman functions:
function tuplet_stem_beam_side()
tuplet_options({"Placement Stem"})
end
function tuplet_note_side()
tuplet_options({"Placement Note"})
end
function tuplet_above()
tuplet_options({"Placement Above"})
end
function tuplet_below()
tuplet_options({"Placement Below"})
end
However, there are other places where I can see the utility of the middleman:
function expressions_mallet_BD_soft()
if check_SMuFL(nil) then
findSpecialExpression({59288}, {nil, 0, 24, 0}, text_expression, "Bass Drum, soft", 5)
else
findSpecialExpression({97}, {"Finale Percussion", 8191, 24, 0}, text_expression, "Bass Drum, soft", 5)
end
getFirstNoteInRegionText("Region Start")
end
I agree with that assessment of helper functions
I can see how it could be way more efficient both in execution and in adding functions to do something like
local input = get_input()
code_map[input]()
Now... Currently there are two different types of functions within JetStream. Starting around line 11577 (!!) they transition from requiring a selection (and giving an error if they don't get one) to not.
I'm not entirely sure about how to do that in the above scenario... Maybe two separate tables?
code_map_requires_selection = {}
and code_map_no_selection = {}
, and try them both in succession?
if code_map_requires_selection[input]() and (there is in fact a selection) then
code_map_requires_selection[input]()
elseif code_map_no_selection[input]() then
code_map_no_selection[input]()
else
(error message)
end
This would be a good place for a helper function:
function do_something()
if environment_has_selection then
do_thing_a()
else
do_thing_b()
end
end
Fun fact which might explode your head at 10pm at night, you can actually pass around functions as arguments to other functions.
function greet(callback, name)
callback("Hello " .. name)
end
-- remember, `print` is a function
greet(print, "Jacob") -- prints "Hello Jacob"
So, you can create a helper function you can use for all items that can optionally use a selection:
function if_selection(use_selection, without_selection)
if environment_has_selection then
use_selection()
else
without_selection()
end
end
function something()
-- do something
end
function something_else()
-- do something else
end
code_map["1234"] = if_selection(something, something_else)
Using this very generic function may not be the best idea for this repository, but something to consider.
Here’s a question, though…
let’s say we’re entering that piano, code 0007. If we do it the existing way, it will start iteration through and find the code, and stop after reading through 6 lines.
If we first build the table up in order to access the function directly, then we need to read hundreds of lines of code to do that… How can that possibly be more efficient, unless we were going to be performing hundreds of separate functions rather than just one or two?
Not sure how I originally missed your question, but here you go.
In short, that's just not how compilers (or in Lua's case, and interpreter) works. It needs to read and parse the entire file before executing any of it. The exact reason behind this is a much longer issue that I don't fully understand (though could definitely go in a lot of detail if you wish).
However, yes it would need to execute hundreds of lines of code before it can choose the right function. But this is mitigated in a few ways.
First, if the code_map
is a global variable and finenv.RetainLuaState == true
, then code_map
only needs to be built for the very first JetStream script run. Then every time the script is called after that, only the single line code_map[code]()
needs to be run (excluding, of course, all lines of the applicable function).
Second, if
statements are actually slower than you might think. The actual reasoning behind this comes from how CPUs operate which is beyond the scope of this thread.
Though to be completely honest, speed isn't a huge concern for the JetStream. Both the if
statements or the code_map
are going to run so fast that no one will be able to notice the difference. The main reason I brought up speed originally was to bring up the concept of linear and constant time. And there are other programs in Lua that linear (or worse quadratic or exponential) time would make a noticeable impact to a script's execution time.
In my opinion, the more salient reason to adopt the code_map
is just code maintainability.
In the JetStream.lua file where the code is redirected to the relevant function, there's a lot of code that looks like this:
https://github.com/CJGarciaMusic/PowerUserWorkflow/blob/e1b3f86a0c0fc142313fa43dc84baaf916a5002b/JetStream.lua#L9627-L9632
This is actually both slow (because it runs in linear time) and verbose.
With this structure, you've now simplified adding each new function to a single line (instead of the 3 it previously took), and selecting the correct function is now a constant time operation instead of a linear time one.
If you're unfamiliar with constant time vs linear time, here's a brief introduction:
https://www.oreilly.com/library/view/a-common-sense-guide/9781680502794/f_0026.xhtml
Side note, I noticed some items use the
compare
function. I'm not sure why this is there or what the purpose is, but it should be noted using a table to index each item will not work with the compare function.https://github.com/CJGarciaMusic/PowerUserWorkflow/blob/e1b3f86a0c0fc142313fa43dc84baaf916a5002b/JetStream.lua#L9624-L9626