Conversation
Benchmark for f153fdfClick to view benchmark
|
Benchmark for c8242c9Click to view benchmark
|
Benchmark for 77adad2Click to view benchmark
|
|
Looks great. I'll add a few nitpicking comments in the review and will do more testing over the weekend. Else this looks straight forward and like a really great addition! |
|
Hey, did you mean that you wanted to add those comments to this as-is, or are you waiting for me here to make the PR more complete? I'd appreciate any nitpicks here before continuing! |
|
There should be a new |
Benchmark for cf34af4Click to view benchmark
|
|
I've separated those and refactored the scripted cycle a bit, since the cycle either has a static table mapping or a callback (and a timeout hook always exist with a callback and never with a mapping), having these as an enum makes the code more self-explanatory. Added a new kind of context for the variables callback that gets the parameters and the current iteration of the cycle assigned, do you think anything else would make sense here? Updated the API docs and the guide with the var function example. |
Benchmark for 259018aClick to view benchmark
|
|
That looks great. Thanks. Give me some time to review and test this, as this is a larger change. Will do do ASAP beginning next week. |
Yes, all non mapping specific stuff that's also in the mapping callbacks: playback_state, time_base There are a few other small issues, like missing resets for the callback. This stuff unfortunately is really tricky in the details. I'll go though this in detail tomorrow and will fix that, okay? |
That would be great! |
…d cycles with variable callbacks
Benchmark for 21d5337Click to view benchmark
|
|
Fixed a few little things here and there and updated the callback context generation and docs: This is what the /// Sets the cycle context for the mapping callbacks.
pub fn set_cycle_map_context(
&mut self,
playback_state: ContextPlaybackState,
time_base: &BeatTimeBase,
channel: usize,
step: usize,
step_length: f64,
) -> LuaResult<()> {
self.set_context_playback_state(playback_state)?;
self.set_context_time_base(time_base)?;
self.set_context_cycle_step(channel, step, step_length)?;
Ok(())
}
/// Sets the cycle context for var callbacks.
pub fn set_cycle_var_context(
&mut self,
playback_state: ContextPlaybackState,
time_base: &BeatTimeBase,
parameters: &ParameterSet,
iteration: u32,
) -> LuaResult<()> {
self.set_context_playback_state(playback_state)?;
self.set_context_time_base(time_base)?;
self.set_context_parameters(¶meters)?;
self.set_context_cycle_iteration(iteration)?;
Ok(())
}Actually wouldn't hurt to also add the new Using an enum for the map callback or static map makes a lot of sense as they are mutually exclusive. I think we should do the same for the Unlikely to be a problem, but if both, I'll try to take care of this tomorrow. Else I think this is ready and works great as it is! |
|
Thanks! It's a small thing but this updated description of the
I've added one to the iteration (that starts from zero) so it would be an idiomatic index in lua. When the It's tangentially related but while experimenting with variables I found the way enum parameter's need to be defined to be fairly inergonomic in that you have to write one case two times (one for the default arg and one inside the list). The work around is to define the table outside but that's adding boilerplate. It would be nice if for example |
The old doc comment got removed by accident. Sorry. Restored it now...
Yes, definitely. I think it would be "safe" to allow strings or numbers here. When passing a string, it must be one of the enum values, when passing a number it's interpreted as index. So using |
Benchmark for fc659d9Click to view benchmark
|
2 participants
An initial draft for the discussed
varsfunction on the cycle object.Works similar to
:mapin that you can supply either a static table or a function returning a table.I guess we'll need a different context for
vars? Since variables are only being assigned once pergenerateinstead of for each event, the fields related to single steps are meaningless.Here, the table variant inserts the variables to the cycle directly instead of passing around the table (contrary to the case with
map) whereas the callback variant follows a similar pattern to themapping_function.Not sure what else should happen with the callback and its errors, but it seems to work fine so far.