Make NAM disablable - bypass dry signal #494
Conversation
There was a problem hiding this comment.
This is a very nice first attempt. Thanks for your work! But, there's a number of things that I think I've caught that would introduce bugs (see comments).
[This PR is also a nice reminder that I should work on a better PR template so that I can think through the sorts of changes that happen so that contributors can work through a checklist to make sure their contributions take into account all of the considerations.]
|
|
||
| if (strcmp(version.Get(), "0.7.10") == 0) | ||
| { | ||
| pos = _UnserializeStateLegacy_0_7_9(chunk, pos); |
There was a problem hiding this comment.
Actually, as I think of this, it might not be right--I think that one of the params' names was changed in v0.7.10:
ddd5ff8#diff-583ead9e08b2afc1f331a5073c88faea9cc55bdbfde16d357dee55438e63f19dL78
I think I thought that would make the unserialization behave differently. Let me check...
Yeah: ebd1352#diff-583ead9e08b2afc1f331a5073c88faea9cc55bdbfde16d357dee55438e63f19dR839
But the funny thing is that I managed to code it (I think) in a way that still works for v0.7.10--newNames.find(oldName) will miss on "Threshold" and will just use "Threshold"...which will work (I think)! Not sure if past me did that on purpose, but I'm glad he did 😉
If you could double-check this for me that it does in fact work as expected (after I bump the version) that'd be super-appreciated.
There was a problem hiding this comment.
It looks like, it works as you described.
After bumping the plugin version to 0.7.11 locally, I was able to properly open my DAW session (saved with 0.7.10) and restore correct params values.
|
|
||
| void NeuralAmpModeler::_SetDisabledForAllControl(const bool disabled) | ||
| { | ||
| if (const auto ui = GetUI(); ui != nullptr) |
There was a problem hiding this comment.
Is there something that makes this try again later if GetUI() returns null?
There was a problem hiding this comment.
No, but (although I am not very familiar with iPlug) I think that GetUI() should never return null when GUI was attached unless there's there's some problem, but please correct me if I am wrong.
This PR implements #431.
Changes:
"NEURAL AMP MODELER" logo in now clickable.
It changes color between white and black - depending if NAM is enabler or disabled.
UI controls are disabled when whole NAM was disabled - dry signal is passed to output.