Mapping/overriding note properties in cycle:map

[unlessgames]

I'd like to be able to somehow access the note properties when remapping events in cycle to make relative changes but I don't think there is a way (beside maybe writing a custom parser for value, but that doesn't sound right).

I feel like something similar to this should work

return cycle("a:#2"):map(function(context, value)
  local n = note(value)
  if n.notes[1] and n.notes[1].instrument then
    return n:instrument(n.notes[1].instrument + 1)
  else
    return n
  end
end)

even nicer would be to have mapping functions for properties that would unwrap the nil for you and pass in the instrument etc.

return cycle("a:#2"):map(function(context, value)
  -- supply a function to instrument to map it
  return note(value):instrument(function(i) return i + 1 end)
end)

[emuell]

value only contains the event's string value without any other properties right now. This makes sense if you want to remap identifiers such as bd to notes. All other properties will get applied after the mapping function is called.

Maybe it would make more sense if the mapping function overrides the values from the cycle string, and not the other way around?
But to do this we'd need to pass along the properties to the map as well, so one can override or apply them.

Something like this:

--- properties is a table with optional #,v,p,d keys
return cycle("a:#2"):map(function(context, value, properties)
  local instrument = properties["#"] -- or properties.instrument
  return note(value):instrument(instrument and  instrument + 1 or nil)
end)

Alternatively, pass them and return as second return value to override. Which would be backwards compatible. The proposal above is not.

--- properties is a table with optional #,v,p,d keys
return cycle("a:#2"):map(function(context, value, properties)
  properties["#"] = properties["#"] + 1 or nil
  return value, properties
end)

[unlessgames]

Personally I like the first one better as the properties being ovewritten after you explicitly said :instrument(something) is very confusing. I think this would be better to fix in these early stages than to solidify such a quirk for backwards compatibility reasons. I'm also not a fan of multiple returns but that's more of a question of taste.

While properties.instrument is more self-explanatory, I guess the table access is more versatile for future extensions with custom property names for example. Of course there could be some helper functions on the properties object for the default note props to access by instrument() or something.

[emuell]

I also prefer the first approach, but there's another issue apart from backwards compatibility that we'll need to address properly sooner or later anyway:

If you don't apply the properties in the map function, they will be wiped away.

--- this strips the #2 property, because it's not applied in the map function
return cycle("a:#2"):map(function(context, value, properties)
  return note(value)
end)

Instead something like this would be necessary in order to apply them:

return cycle("a:#2"):map(function(context, value, properties)
  return note(value):instrument(properties.instrument)
  -- or via a new `properties` or `map` function in note
  -- return note(value):properties(properties)
end)

This also is problematic when using a map table instead of a function:

return cycle("a:#2"):map({
  a = note("c4") -- what about the #2 here?
})

[unlessgames]

If you don't apply the properties in the map function, they will be wiped away.

This is not the case, the script you posted will actually apply the instrument specified in the cycle string when you render it. (would be nice to see the instrument in the preview though)

I think this is good but if you use :instrument() it should allow you to overwrite or - as discussed above - access the instrument and decide when to override.

[emuell]

The example assumed this:

Maybe it would make more sense if the mapping function overrides the values from the cycle string, and not the other way around? But to do this we'd need to pass along the properties to the map as well, so one can override or apply them.

Right now the map function is called before the note properties are applied. But if you want to override note properties in the map function as well, it must be evaluated after the cycle note properties have been applied.

Here's how things currently work:

For each emitted note/identifier event in a cycle:

• call map function or apply map table, if available
• check if the note value is valid
• apply note properties from the cycle

pattrns/src/emitter/scripted_cycle.rs

Line 86 in 7bd6f81

fn cycle_to_note_event(

[unlessgames]

Ah ok!

In that case the properties input sounds right. Maybe having an apply function would also be useful to help with default application?

cycle("a3:#4"):map(function(context, value, properties)
  -- apply all properties in one go
  return note(value):apply(properties)
end)

On a sidenote, going back to your first example, might be nice to alias # as i (even in the cycle parser) here, so you could do all without the bracket+string key access.

cycle("a3:#4:v.8 c4"):map(function(_, key, ps)
  return 
    note(key)
    :instrument(ps.i and ps.i + 1 or 3)
    :volume(ps.v and ps.v * 0.5 or 0.5)
end)

[emuell]

apply as new function in note and i as property value (alias) sounds good to me.

But for the sake of backwards compatibility, I think we do need to introduce a new map function here and keep the old one as it is.

What about transform? Sounds a bit as if this could change a note's timing as well, but i can't think of something better.

[emuell]

How about introducing this as a new function as apply? After all, it creates note events from note strings and optional properties.

Would make a bit more clear that one needs to consume all properties here, and actually could be chained with map:

-- map `kd` to a note value and apply `amplify` parameter to volume
cycle("kd:v=<0.5 1.0>")
  :map({kd = "c4"})
  :apply(function(context, value, properties)
   -- value already is mapped to 'c-4' here
    properties.v = (properties.v or 1.0) * context.parameter.amplify
    return note(value):apply(properties)
end)

[emuell]

Or as discussed/suggested in #94 we could also use a transform function, but instead of passing values and properties separately, it gets already mapped note events passed:

-- "event" is a Note userdata and already has all properties applied
cycle("c:v=steppy d e [g:v=steppy f]"):transform(function(context, event)
  if event.v == "steppy" then 
    local stepping_volume = context.step * 0.1
    event.v = stepping_volume - floor(stepping_volume)
  end
  return event
end)

I think this is easier to handle, because if you don't do anything in the transform function, the content is played back as it is. In the suggested apply function, however, a note(value):apply(properties) call would be necessary to create an event. If you skip this, no event would be created at all, which seems wrong to me.

@unlessgames, so are we're going for the the transform function with passed event? It can be combined with the existing map function without changing its behavior, so this also should not break backwards compatibility.

---

Implementation wise this would:

For each emitted note/identifier event in a cycle:

• call map function or apply map table, if available
• call transform function, if available
• check if all note values and all property values are valid

Since we want to allow arbitrary string values to be passed as properties for transform, we cannot actually create Rust note events before calling the transform functions. The Rust note event does not allow, and should not allow, string values as property values. In Lua this is okay.

This also means that when a transform function does not convert such string values, this will result into a runtime error. Just like it's now an error if you don't map custom note identifiers such as kick with map.

[unlessgames]

I'd vote for the second kind of transform with already applied properties, avoiding the boilerplate from the other version would be nice.

The only slight issue I see is that you can call transform before map, yet the application order is actually fixed.

[emuell]

The only slight issue I see is that you can call transform before map, yet the application order is actually fixed.

Could memorize the order here. Whatever get's assigned there first, will then be called first when evaluating the functions.

[emuell]

Do we still need a transform or apply? I guess after #107 all this can be elegantly done with the proposed var function.