MIDIFunc.noteOn does not work with floats #325

Closed
gesellkammer opened this Issue May 15, 2012 · 6 comments

Comments

Projects
None yet
3 participants
@gesellkammer

MIDIFunc.noteOn fails silently if noteNum is passed a float. If an Integer is needed, it should call .asInteger on the received argument or at least throw an Error .

@crucialfelix

This comment has been minimized.

Show comment
Hide comment
@crucialfelix

crucialfelix Jun 25, 2012

Member

this is also the case in CCResponder and has caught me a few times. It needs something like this in the constructor:

    if(chan.isNumber,{ chan = chan.asInteger });
    if(num.isNumber,{ num = num.asInteger });
    if(value.isNumber,{ value = value.asInteger });

It shouldn't be done during the actual event time as that would slow it down. Its technically a user error to have set it to a float, but we all make mistakes and its annoying.

Member

crucialfelix commented Jun 25, 2012

this is also the case in CCResponder and has caught me a few times. It needs something like this in the constructor:

    if(chan.isNumber,{ chan = chan.asInteger });
    if(num.isNumber,{ num = num.asInteger });
    if(value.isNumber,{ value = value.asInteger });

It shouldn't be done during the actual event time as that would slow it down. Its technically a user error to have set it to a float, but we all make mistakes and its annoying.

scztt added a commit that referenced this issue Apr 17, 2015

classlib: note nums / cc channels must be integers
Protect against non-integral values for note and cc number. Ordinarily inputting invalid values here would be user error - however, it's plausible that a note or cc num here might be the product of some calculation which could have caused an implicit conversion to float, which would cause a silent failure. Fixes #325
@crucialfelix

This comment has been minimized.

Show comment
Hide comment
@crucialfelix

crucialfelix Apr 17, 2015

Member

note: main reason that users supply floats is that we used a number input in a gui

Member

crucialfelix commented Apr 17, 2015

note: main reason that users supply floats is that we used a number input in a gui

@muellmusik

This comment has been minimized.

Show comment
Hide comment
@muellmusik

muellmusik Apr 17, 2015

Contributor

I think this issue may have been prematurely closed. See my comment in #1425. Reopening for now.

Contributor

muellmusik commented Apr 17, 2015

I think this issue may have been prematurely closed. See my comment in #1425. Reopening for now.

@muellmusik muellmusik reopened this Apr 17, 2015

@crucialfelix

This comment has been minimized.

Show comment
Hide comment
@crucialfelix

crucialfelix Apr 17, 2015

Member

the commit does fix the bug that this ticket is for: casting a supplied float to an integer

what you commented over there is about handling arrays. that is a separate issue.

Member

crucialfelix commented Apr 17, 2015

the commit does fix the bug that this ticket is for: casting a supplied float to an integer

what you commented over there is about handling arrays. that is a separate issue.

@crucialfelix

This comment has been minimized.

Show comment
Hide comment
@crucialfelix

crucialfelix Apr 17, 2015

Member

please file a separate issue for discussion if you feel its important. I'm reclosing this because the issue as reported has been fixed.

Member

crucialfelix commented Apr 17, 2015

please file a separate issue for discussion if you feel its important. I'm reclosing this because the issue as reported has been fixed.

@muellmusik

This comment has been minimized.

Show comment
Hide comment
@muellmusik

muellmusik Apr 17, 2015

Contributor

please file a separate issue for discussion if you feel its important. I'm reclosing this because the issue as reported has been fixed.

Sure. Done: #1426

Contributor

muellmusik commented Apr 17, 2015

please file a separate issue for discussion if you feel its important. I'm reclosing this because the issue as reported has been fixed.

Sure. Done: #1426

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment