Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify WebMIDI detection #190

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Simplify WebMIDI detection #190

wants to merge 1 commit into from

Conversation

gagern
Copy link

@gagern gagern commented Jul 27, 2016

The previous code is broken: the result of indexOf is an index, and -1 if the searched string is not found. So the value will be non-zero (and therefore thuthy) in all realistic setups, making the plugin enumeration an unused code branch.

We could fix the intent of the original code, by comparing the indexOf result against -1. But doing so would break systems where WebMIDI is provided by some other plugin or polyfill, where plugins cannot be enumerated, where native functions stringify in some other way, where something monkey-patched the method in question, or something like that.

So instead this commit here relies on the ducktyping philosophy: if there is a navigator.requestMIDIAccess method, we assume it does the right thing and avoid further checks. This is what the current code is essentially doing right now anyway, even though not intentionally.

@gagern
Simplify WebMIDI detection
7e5d440 
The previous code is broken: the result of indexOf is an index, and -1 if
the searched string is not found.  So the value will be non-zero (and
therefore thuthy) in all realistic setups, making the plugin enumeration an
unused code branch.

We COULD fix the intent of the original code, by comparing the indexOf
result against -1.  But doing so would break systems where WebMIDI is
provided by some other plugin or polyfill, where plugins cannot be
enumerated, where native functions stringify in some other way, where
something monkey-patched the method in question, or something like that.

So instead this commit here relies on the ducktyping philosophy: if there is
a navigator.requestMIDIAccess method, we assume it does the right thing and
avoid further checks.  This is what the current code is essentially doing
right now anyway, even though not intentionally.
gagern added a commit to gagern/CindyJS that referenced this pull request Aug 1, 2016
@gagern
Support playtone via MIDI.js
86e65ea 
The MIDI.js verison here is one which integrates three of my PRs:
mudcube/MIDI.js#187 (percussion)
mudcube/MIDI.js#189 (MIDI.now)
mudcube/MIDI.js#190 (WebMIDI detection)

The playtone function automatically triggers loading of the plugin.  The
corresponding soundfont is loaded only when needed, which means there will
be some time between when the function calls start and when sound becomes
actually audible.

Samples are hosted on http://cindyjs.org/soundfont/ which might break
widgets delivered through HTTPS.  We need a certificate for cindyjs.org to
solve this problem, or allow configuring the soundfont through some API,
e.g. some CindyScript function.

The example is essentially taken from a Cinderella export, except for the
deletion of a duplicate play button.
gagern added a commit to gagern/CindyJS that referenced this pull request Aug 1, 2016
@gagern
Support playtone via MIDI.js
98b3376 
The MIDI.js verison here is one which integrates three of my PRs:
mudcube/MIDI.js#187 (percussion)
mudcube/MIDI.js#189 (MIDI.now)
mudcube/MIDI.js#190 (WebMIDI detection)

The playtone function automatically triggers loading of the plugin.  The
corresponding soundfont is loaded only when needed, which means there will
be some time between when the function calls start and when sound becomes
actually audible.

Samples are hosted on http://cindyjs.org/soundfont/ which might break
widgets delivered through HTTPS.  We need a certificate for cindyjs.org to
solve this problem, or allow configuring the soundfont through some API,
e.g. some CindyScript function.

The example is essentially taken from a Cinderella export, except for the
deletion of a duplicate play button.
gagern added a commit to gagern/CindyJS that referenced this pull request Aug 1, 2016
@gagern
Support playtone via MIDI.js
88c66b5 
The MIDI.js verison here is one which integrates three of my PRs:
mudcube/MIDI.js#187 (percussion)
mudcube/MIDI.js#189 (MIDI.now)
mudcube/MIDI.js#190 (WebMIDI detection)

The playtone function automatically triggers loading of the plugin.  The
corresponding soundfont is loaded only when needed, which means there will
be some time between when the function calls start and when sound becomes
actually audible.

Samples are hosted on http://cindyjs.org/soundfont/ which might break
widgets delivered through HTTPS.  We need a certificate for cindyjs.org to
solve this problem, or allow configuring the soundfont through some API,
e.g. some CindyScript function.

The example is essentially taken from a Cinderella export, except for the
deletion of a duplicate play button.
montaga pushed a commit to montaga/CindyJS that referenced this pull request May 25, 2018
@gagern @montaga
Support playtone via MIDI.js
930090f 
The MIDI.js verison here is one which integrates three of my PRs:
mudcube/MIDI.js#187 (percussion)
mudcube/MIDI.js#189 (MIDI.now)
mudcube/MIDI.js#190 (WebMIDI detection)

The playtone function automatically triggers loading of the plugin.  The
corresponding soundfont is loaded only when needed, which means there will
be some time between when the function calls start and when sound becomes
actually audible.

Samples are hosted on http://cindyjs.org/soundfont/ which might break
widgets delivered through HTTPS.  We need a certificate for cindyjs.org to
solve this problem, or allow configuring the soundfont through some API,
e.g. some CindyScript function.

The example is essentially taken from a Cinderella export, except for the
deletion of a duplicate play button.
montaga pushed a commit to montaga/CindyJS that referenced this pull request May 25, 2018
@gagern @montaga
Support playtone via MIDI.js
46fbdb7 
The MIDI.js verison here is one which integrates three of my PRs:
mudcube/MIDI.js#187 (percussion)
mudcube/MIDI.js#189 (MIDI.now)
mudcube/MIDI.js#190 (WebMIDI detection)

The playtone function automatically triggers loading of the plugin.  The
corresponding soundfont is loaded only when needed, which means there will
be some time between when the function calls start and when sound becomes
actually audible.

Samples are hosted on http://cindyjs.org/soundfont/ which might break
widgets delivered through HTTPS.  We need a certificate for cindyjs.org to
solve this problem, or allow configuring the soundfont through some API,
e.g. some CindyScript function.

The example is essentially taken from a Cinderella export, except for the
deletion of a duplicate play button.
montaga pushed a commit to montaga/CindyJS that referenced this pull request May 30, 2018
@gagern @montaga
Support playtone via MIDI.js
c01bb5e 
The MIDI.js verison here is one which integrates three of my PRs:
mudcube/MIDI.js#187 (percussion)
mudcube/MIDI.js#189 (MIDI.now)
mudcube/MIDI.js#190 (WebMIDI detection)

The playtone function automatically triggers loading of the plugin.  The
corresponding soundfont is loaded only when needed, which means there will
be some time between when the function calls start and when sound becomes
actually audible.

Samples are hosted on http://cindyjs.org/soundfont/ which might break
widgets delivered through HTTPS.  We need a certificate for cindyjs.org to
solve this problem, or allow configuring the soundfont through some API,
e.g. some CindyScript function.

The example is essentially taken from a Cinderella export, except for the
deletion of a duplicate play button.
montaga pushed a commit to montaga/CindyJS that referenced this pull request May 30, 2018
@gagern @montaga
Support playtone via MIDI.js
41623ef 
The MIDI.js verison here is one which integrates three of my PRs:
mudcube/MIDI.js#187 (percussion)
mudcube/MIDI.js#189 (MIDI.now)
mudcube/MIDI.js#190 (WebMIDI detection)

The playtone function automatically triggers loading of the plugin.  The
corresponding soundfont is loaded only when needed, which means there will
be some time between when the function calls start and when sound becomes
actually audible.

Samples are hosted on http://cindyjs.org/soundfont/ which might break
widgets delivered through HTTPS.  We need a certificate for cindyjs.org to
solve this problem, or allow configuring the soundfont through some API,
e.g. some CindyScript function.

The example is essentially taken from a Cinderella export, except for the
deletion of a duplicate play button.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@gagern