Fixes and alternates for #5737 #5764
Conversation
Fix broken support for multiple plugs - we can't call `getPlug()` because it throws if there is more than one.
We were making serialisable per-instance metadata when a static registration could do the same job.
And instead just use a NameLabel in RampPlugValueWidget.
I'm inclined to include it despite being a bit long. I didn't include it originally since the other metadata on
My vote is to go with the metadata solution. Reducing the functionality combined with what to me is a less elegant approach to the code steers me to the metadata option. But happy to discuss. |
Cool, it does seem worth keeping the existing functionality. @ericmehl could you please get this ready to merge minus the last commit? |
|
Closing as the fixes (without the last commit) have been merged in #5737 |
This contains fixups addressing my review comments on #5737. Usually I wouldn't be doing this myself, but given that we want to include this in today's release, I thought it was worth getting something concrete up. There are a few categories of things...
Fixups : 7719c2e and 755ea10 are fixups for comments I made on the original PR. I'm pretty sure these are legit, and should be included.
Metadata prefixing : bcb073a adds a prefix to the new metadata name, to make it clear it only applies to LabelPlugValueWidget. This matches our convention for most other widgets, but is very wordy. I think we should probably include it, but maybe that's open to debate.
Completely different approach : ad9a78b removes the metadata and just stops using LabelPlugValueWidget in the RampPlugValueWidget. It's simpler overall, but does result in a reduction in functionality for the user (no right-click menu on the labels). Probably isn't the way we want to go, but I wanted to provide it for consideration at least.