Move MIDI's methods and constants into InputEventMIDI #86708
Conversation
| @@ -526,8 +549,8 @@ class InputEventMIDI : public InputEvent { | |||
| void set_channel(const int p_channel); | |||
| int get_channel() const; | |||
|
|
|||
| void set_message(const MIDIMessage p_message); | |||
| MIDIMessage get_message() const; | |||
| void set_message(const Message p_message); | |||
There was a problem hiding this comment.
Does this actually work? The enum values are the same but it's a different enum so it shouldn't match in GDScript and elsewhere, right?
There was a problem hiding this comment.
Not sure what you mean by "work". When I excluded static methods, the engine seems to compile just fine when replacing the MIDIMessage enum with Message all throughout.
There was a problem hiding this comment.
So doing event.message = MIDI_MESSAGE_NONE works?
There was a problem hiding this comment.
Just verified in my current build (without the static methods because I must reiterate, do not compile). Yes, you can receive MIDI input, get and set the event's message just fine.
There was a problem hiding this comment.
That's strange, because assigning other, matching valued, enums in GDScript like that doesn't work correctly, like doing:
event.messsage = TEXTURE_REPEAT_PARENT_NODEThrows an error as the result type is not the correct enum
There was a problem hiding this comment.
Not sure if it's directly related, but I forgot to change it in the class reference XML file (message's associated enum is still MIDIMessage). I will see.
There was a problem hiding this comment.
I'm not sure changing the enum here preserves compatibility, we'll see what the CI says
There was a problem hiding this comment.
Fixed up and built your code and can confirm that this does indeed not work:
event.message = MIDI_MESSAGE_NONEIt has to be:
event.message = InputEventMIDI.MESSAGE_NONEThis breaks compatibility unless you change the bound method to take the original type
Mickeon
force-pushed
the
all-midi-to-InputEventMIDI
branch
2 times, most recently
from
5e60696 to
775e18f
Compare
|
I apologize for the many, many incoming merge pushes that will ensue. |
|
Given the above issues with the property I'd suggest removing the enum related changes and keep just the moving of the MIDI driver management, that part I think makes a lot of sense to move out of |
Mickeon
force-pushed
the
all-midi-to-InputEventMIDI
branch
from
775e18f to
a79c7c4
Compare
Mickeon
force-pushed
the
all-midi-to-InputEventMIDI
branch
from
a79c7c4 to
773c608
Compare
|
I could split the PRs in two and keep this one for the Messages. They took me way longer than the methods to transfer over, and there's some discussion to be had for how useful some of them are. |
|
Sounds like a good idea, but unsure how to handle that one without breaking compat at all, so wouldn't be feasible for the short term or even the medium term, but worth saving it |
|
As of currently, this does not compile specifically because of the enum referencing the other enum. So indeed, they are being problematic. |
|
Oh yes that's because of the difference in class-ness, should be solved by just doing |
|
This was an accidental removal of a branch but it's no big deal, we are splitting it in two anyway. |
Superseded by #86713 and #86716.
This PR moves OS' methods about MIDI, and the global constants into the InputEventMIDI. The new methods are static.
The previous methods and constants still exist, but are marked as deprecated.
This means the following:
Edit: These methods are currently commented out, because they refuse to compile. Will ask for help later.
OS.open_midi_inputs()InputEventMIDI.open_inputs()OS.get_connected_midi_inputs()InputEventMIDI.get_connected_inputs()OS.close_midi_inputs()InputEventMIDI.close_inputs()