WTSE overview class refactor #7755
Conversation
nuoun
commented
Aug 14, 2024
nuoun
commented
- Changes WTSE overview class constructor to use similar style as FormulaModulatorEditor
- Renames WavetableEquationEditor class to WavetableScriptEditor
- Renames defaultWavetableFormula string to defaultWavetableScript
- Adds preliminary Prelude to WTSE
- Adds changes to WTSE UI
- Adds WavetableScriptEditState struct to DAWExtraStateStorage and SurgePatch load_xml()/save_xml()
- Increments XML file version to v25
- Changes WTSE overview class constructor to use similar style as FormulaModulatorEditor - Renames WavetableEquationEditor class to WavetableScriptEditor - Renames defaultWavetableFormula string to defaultWavetableScript - Adds preliminary Prelude to WTSE - Adds changes to WTSE UI - Adds WavetableScriptEditState struct to DAWExtraStateStorage and SurgePatch load_xml()/save_xml() - Increments XML file version to v25
|
Big PR with changes to SurgePatch that could use a thorough review when you can find the time, no rush so please take your time. |
There was a problem hiding this comment.
This all looks great. Really just some minor cosmetic changes
I think we should also create an issue with a checklist for what we want to do to get to minimal viable WTSE. I added a few items here which would go on that list.
But I'm definitely happy to merge this
| @@ -921,6 +922,11 @@ struct DAWExtraStateStorage | |||
| bool hasCustomEditor = false; | |||
| } oscExtraEditState[n_scenes][n_lfos]; | |||
|
|
|||
| struct WavetableScriptEditState | |||
| { | |||
| int codeOrPrelude{0}; | |||
There was a problem hiding this comment.
Maybe a comment here briefly about what 'codeOrPrelude' means would be useful?
| { | ||
| enum tags | ||
| { | ||
| tag_select_tab = 0x597500, |
There was a problem hiding this comment.
We probably want to use a different value here than other control areas just in case. Like just choose some other random starting number. In case messages do cross (which they should not)
There was a problem hiding this comment.
I did change this after I copied parts of the Formula overview classes and made sure 0x597500 is a unique value.
There was a problem hiding this comment.
ha sorry i should have checked.
There was a problem hiding this comment.
No worries, I'm just glad I did it right :)
| void onSkinChanged() override { rebuild(); } | ||
|
|
||
| JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR(WavetableScriptControlArea); | ||
| }; |
There was a problem hiding this comment.
I didn't read this all that carefully but I am assuming it is ok since it looks copied from other similar with appropriate changes. But we should add an item to your work list before we ship 1.4 to do an accessibility check here (which I can help with).
There was a problem hiding this comment.
I will add this to the WTSE issue.
| return std::nullopt; | ||
| } | ||
|
|
||
| /* |
There was a problem hiding this comment.
No reason to leave this commented out code around is there? We have git history if we need it.
|
|
||
| struct WavetableEquationEditor : public CodeEditorContainerWithApply, | ||
| /* | ||
| struct WavetableScriptEditor : public CodeEditorContainerWithApply, |
There was a problem hiding this comment.
Same on commented out code here
|
|
||
| bool shouldRepaintOnParamChange(const SurgePatch &patch, Parameter *p) override | ||
| { | ||
| return false; |
There was a problem hiding this comment.
This is fine for now but we should add an item to a list which is like "what happens if the wt editor is torn out and you change oscillator from wavetable to twist" or some such. We should make that a conscious decision at least.
There was a problem hiding this comment.
Did a little test and I think this works fine, changing oscillator type does not affect the torn out window and the WT generation still works, ie. the generated wavetable is ready to go after switching back to the WT oscillator.
|
Good to hear, I think the old WTSE issue is still open so we can add start referring to that issue and I shall create a little roadmap / list of issues there. And this last commit should take care of the unused code and add a description for the struct in SurgeStorage. |
|
Super exciting! Merging! |
