digego / extempore

A cyber-physical programming environment
1.41k stars 127 forks source link

Handle #f in pc:make-chord #353

Closed smoores-dev closed 5 years ago

smoores-dev commented 5 years ago

It's likely that this isn't the correct way to do this (I'm still quite new to Scheme), but currently pc:random can return either a number or #f, which results in pc:make-cord breaking often (some configurations break always, some break randomly, some always succeed).

This change just checks if pitch is a number before attempting to cons it with the chord, instead of checking that it's greater than 0.

benswift commented 5 years ago

Hmm. I'm not closed to this change, there might be some reason that -1 is used as the "choose from the full range" value.

If we were going to make the change, it'd probably be better to just do (not pitch), since #f is the only thing that's falsy in Scheme (whereas the current proposed change would allow e.g. an atom or string to trigger the branch.

@digego thoughts? Or should we just leave as-is?

smoores-dev commented 5 years ago

Agreed that (not pitch) is a better check; I wasn't sure what would happen with 0 in that case, but it looks like it has the expected behavior. It looks like this bug is due to this change: https://github.com/digego/extempore/commit/7bd3439ba2a59bd47a823d9b4e9c2ab75810b535 in which the error return value of pc:random was changed from -1 to #f, but the pc:make-chord function wasn't updated

benswift commented 5 years ago

Ah, good catch. Yep - whichever one we use (-1 or #f) it should be consistent.

@digego ok to merge?