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.

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

feat: Add MIDI constants #26

Merged
merged 4 commits into from
Jul 14, 2024
Merged

feat: Add MIDI constants #26

merged 4 commits into from
Jul 14, 2024

Conversation

MarquisdeGeek
Copy link

Using numbers like 144 for note on, or 38 to switch to a bass guitar, is a little opaque for my tastes so I've added all the names from the General MIDI spec to self-document my generative music.

@Julusian
Copy link
Owner

Julusian commented Jul 2, 2024

I'm open to merging this, but I think the exports of the values need better names.

Perhaps this could exported from midi.js as:

Constants: {
  Instruments,
  Drums,
  Notes,
  Messages,
}

@MarquisdeGeek
Copy link
Author

The instrument and drum lists are only valid for GM (General MIDI) instruments, so would need a designation. Would suggest GMDrums, instead of GeneralMIDIDrums to avoid making the names too long?

@Julusian
Copy link
Owner

Julusian commented Jul 2, 2024

I feel like 'GM' doesn't convey enough meaning to explain anything.
Perhaps this would be better added as a comment on the export/documentation, that should be visible to users when looking to use the values

@MarquisdeGeek
Copy link
Author

Bump for the above changes, as suggested.

@Julusian Julusian changed the title Add MIDI constants to remove the magic numbers from sendMessage calls feat: Add MIDI constants Jul 14, 2024
@Julusian Julusian merged commit 886ea29 into Julusian:master Jul 14, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants