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

Move MIDI-related constants and methods away from the GlobalScope and into a singleton or class #8761

Open
Mickeon opened this issue Dec 31, 2023 · 0 comments
Labels
breaks compat Proposal will inevitably break compatibility topic:core

Comments

@Mickeon
Copy link

Mickeon commented Dec 31, 2023

Describe the project you are working on

Contributing to the documentation.

Describe the problem or limitation you are having in your project

Godot's MIDI's implementation has kind of been left out for years.

I mean, do not get me wrong. it's cool, neat, and it's actually functional, too. But it isn't exactly everyday-use. And that is fairly easy to tell, too. The documentation and related members haven't really been kept up to standard compared to the rest of the class reference. To make it brief, they're full of details that only the author may have been interested in, all the while ensuring that the user looks elsewhere to understand what it all means.
Furthermore, MIDI-related discussion is fairly rare.

I can only speculate that it's been like this because it sounded cool, then it got merged with little insight, and then got left out ever since.

But that would be okay, perhaps? I mean, it's still a shame, but it would be okay...

...If it weren't for the fact that, for some ungodly reason, all of the MIDI constants are present in the GlobalScope. Once again, I can speculate that back when this was implemented it would've not been that big of a deal at all. Maybe it was to be consistent with the Key and MouseButton enum? Either way, now they stand out.

There's many of them, and for how niche their purpose is, they "pollute" the global scope with information most users will never be interested in. The rest of the documentation about MIDI support is sporadically placed around OS and InputEventMIDI, too. Not that there is a lot of it, but it's not exactly a good impression.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Move all of the global scope MIDI constants and related OS's methods and/or into their own singleton or class. Of course, this would leave the currently existing ones as deprecated.
There are a few possible candidates:

  • New MIDI singleton

This is the simplest choice.

MIDI.open_input()
MIDI.get_connected_inputs()
MIDI.close_input()

func _input():
    if event is InputEventMIDI and event.message == MIDI.MESSAGE_NOTE_ON:
        print("Doot!")

One problem could be, that having a whole class with relatively few functions is quite silly. Godot is not exactly a stranger to this (Marshalls, IP...), but it needs to be kept in mind.

  • The AudioServer singleton

At a first glance, it would seem like the AudioServer would be the most logical.

AudioServer.open_midi_input()
AudioServer.get_connected_midi_inputs()
AudioServer.close_midi_input()

func _input():
    if event is InputEventMIDI and event.message == AudioServer.MIDI_MESSAGE_NOTE_ON:
        print("How verbose!")

However, its purpose strictly related to audio stream processing. MIDI is an input interface designed for musical instruments (it doesn't play the audio itself). From an implementation standpoint, this doesn't make sense.

  • The InputEventMIDI class

One last alternative, and this is kind of my favourite.

InputEventMIDI.open_input()
InputEventMIDI.get_connected_inputs()
InputEventMIDI.close_input()

func _input():
    if event is InputEventMIDI and event.message == InputEventMIDI.MESSAGE_NOTE_ON:
        print("How verbose!")

In Godot 4, static methods for built-in classes are supported without much hassle. We can take advantage of this to keep the scope of all MIDI properties in check, with the additional benefit of keeping absolutely all of the class reference about MIDI support in one page, since InputEventMIDI is the only one class, pretty much.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

See the above suggestions.

If this enhancement will not be used often, can it be worked around with a few lines of script?

The proposal is about the way these MIDI related methods and constants are exposed, so no.

Is there a reason why this should be core and not an add-on in the asset library?

Everything MIDI is built into core.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks compat Proposal will inevitably break compatibility topic:core
Projects
None yet
Development

No branches or pull requests

2 participants