esq5505.cpp, esqpanel.cpp: make specifying the layout the driver's responsibility. #14379
Conversation
Move loading of those layouts into the esq5505 driver class. This simplifies the code: we no longer need to pass a panel type when constructing the esqpanel instance. It also allows for loading external override artwork.
|
You're forcing the panel device to be called "panel" and be at the top level. I don't really like that, honestly... |
|
If this were something intended to be reusable, then I agree it would be less than ideal. But here, these layouts are specifically for use by this specific driver class with the specific panel device. This placement would only change is someone reorganized things in esq5505.cpp - and in that case, I think that updating the inputtags in these three layout files is more than reasonable, no different from if they were to change the names of the ioports. Things are already tied very closely together; this doesn't change that. But it does make the code simpler, and it allows the layout to be loaded at the top level so as external artwork as well, so to me this is an improvement overall. I looked at writing something in the Lua script to find the correct device, but I can't change the inputtag from within the Lua script so I would effectively have to reimplement button-pressing functionality in there as well (catching pointer events, checking for bounds, updating the io ports etc). Doable of course, but to me, a much more cumbersome solution. |
…l make them call the panel when they change. This seems to give us something like the best of both worlds: the Layouts can be loaded by the machine driver (esq5505_device), and use relative paths in their inputtags to find the io ports. But the code to handle the changes to the input is in the esqpanel class.
|
I've found a different approach that may be better: I've moved the io port definitions into the esq5505_state class, but they will call directly into the panel device's members when there's a change in the value. The esq5505_device essentially connects its io ports directly into the esqpanel. This allows the esq5505_state instance to specify the layout, which can find the io ports with a relative path, as before. The esqpanel class doesn't need to know about the layout, it simply will receive calls to its changed() members when there's a change to handle. |
The esqpanel_vfx device adds a new static method that will add the IO ports it needs, to the specified owning device, and with change callbacks set to its own methods at the tag specified by the caller. esq5505_state calls the panel class's method when creating its input ports, passing itself asa the owner,and the tag under which it also declares the panel. This way, the panel is the only piece of C++ code that has to specify the io port names; and the esq5505_state device is the only place that has to know the tag where the panel lives. The layouts of course also need to know the names of the io ports, but trhey do not need to know their absolute paths. I've also added a 'panel_' prefix to the io port names, both for clarity (that these are panel-related io ports) and to reduce the likelihood of name collisions (should the esq5505_state class need to add other IO ports).
|
Is this approach better? It achieves the same goal: moves the responsibility for loading the layout file into the esq5505_state class, which allows external panels to be loaded instead of the built-in ones; but it no longer uses absolute paths, or requires the panel to be at any particular level. Each of the esqpanel2x40_vfx_device and the esq5505_state classes now tracks the names it cares about, separating their responsibilities: esq5505_state tracks the where the panel is loaded, esqpanel2x40_vfx_device tracks the names of the io ports. |
2 participants
Currently, the layout files for the Ensoniq VFX family of keyboards refer to their connected ioports by relative tags, which means they have to be loaded by an instance of the esqpanel2x40_vfx_device class; so the esq5505_state driver has to pass a parameter to specify the panel type when constructing the esqpanel2x40_vfx_device.
This PR instead makes it the esq5505_state driver's responsibility to set the layout to use, which removes the need to pass that parameter through, and allows us to remove it. But it also means the layout files can't look at relative tags to find their ioports, as they will now be loaded relative to the driver, not the panel. So we change the layout files to use absolute tags.
This also means that it's now more straightforward to use one of these layouts as a starting point for creating external artwork, which would also not be loaded relative to the esqpanel2x40_vfx_device.
The esqpanel2x40_vfx_device class still specifies a barebones fallback character display with no buttons or sliders, for those cases where it's currently being used by a non-VFX-family driver.