Skip to content

Integrate general partial modifier editor - #149

Open
oheartone wants to merge 1 commit into
cmp-illinois:mainfrom
oheartone:integrate-partial-modifier
Open

Integrate general partial modifier editor#149
oheartone wants to merge 1 commit into
cmp-illinois:mainfrom
oheartone:integrate-partial-modifier

Conversation

@oheartone

Copy link
Copy Markdown
Collaborator

Summary

This PR re-integrates the previous general partial modifier work on top of the latest main branch without overwriting the new PHASE_MOD-specific editor.

Main changes:

  • Added GeneralPartialModifierDialog for non-PHASE_MOD partial modifier editing.
  • Kept the existing PartialModifierDialog as the specialized PHASE_MOD editor.
  • Routed PHASE_MOD + PARTIAL to the specialized editor, and other non-DETUNE PARTIAL modifiers to the general editor.
  • Preserved PHASE_MOD as backend type 7 while allowing it to appear earlier in the UI through combo-box item data/display-order mapping.
  • Updated modifier field enable/disable behavior so PARTIAL mode only exposes Partial Result String.
  • Made DETUNE SOUND-only by removing PARTIAL as an apply option for DETUNE.
  • Updated save/load behavior so PARTIAL mode does not overwrite top-level SOUND fields with visually cleared values.
  • Reduced modifier row spacing for a more compact layout.
  • Restricted modifier display/editing to Bottom events only.

Notes

I intentionally did not migrate the old maxPartialCountForCurrentBottom() helper from my earlier branch. The current main branch already has spectrum partial-count logic used by the PHASE_MOD editor, and the general partial editor now reuses that same path. Keeping one shared path avoids conflicting partial-count behavior.

@eightfifteenthspisquared

Copy link
Copy Markdown
Collaborator

Is there a general reason all of these need to be one big pull request? Could we separate out these changes into smaller ones (for example making modifier spacing its own PR), so that it's a bit easier to manage and keep track of?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants