Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

128 channel support with user feedback #254

Merged
merged 34 commits into from
Nov 3, 2023
Merged

128 channel support with user feedback #254

merged 34 commits into from
Nov 3, 2023

Conversation

firthm01
Copy link
Contributor

Supercedes #253 - same fixes + additional commits for user feedback when channel limits are exceeded on import

existing getEntryById  method renamed to getEntryIndexById since that's more accurate of what it did.
Move out of state-polling timer method and in to onStateChanged
@firthm01 firthm01 mentioned this pull request Oct 12, 2023
@firthm01
Copy link
Contributor Author

Code review notes;

General approach was to make everything support 128 channels under the hood. Some areas needed to know the actual supported channels of the DAW (e.g, for routing during import and for UI), so there were some helper funcs for that.

Tasks

  • Add a global #define to set the channel count - change all hard-coded channel counts to use this. For routing, we need to know exactly how many channels the DAW supports so we don't use out-of-range values (e.g, REAPER older than v7). This is done with helper functions.
    main...aa10ba2

  • UI changes:

    • Decided to list channels up to the maximum, but to just disable unsupported ones in the dropdown - this required some changes to the EarComboBox* classes to support disabling entries
      aa10ba2...f15d61d
    • Show an icon and a tooltip if not all channels are available. For DS plugin, this required widening the left column and recentring the meters afterwards.
      2768bc0...935f83c
  • User feedback when problems occur during import;

    • ImportListener (and so ImportBroadcaster) supports warnings. A little crude using just a string, but didn't think it was worth over-engineering it at this stage. Keeps counts of warnings rather than storing dupes.
      935f83c...1623858
    • ImportStatus::COMPLETE state was handled in the import dialog timer method so was ran repeatedly. Moved it so it only runs once (onStateChanged)
      1623858...6fd222d
    • ImportBroadcaster ptr passed to the PluginSuite classes at the start of import so they can pass back warnings - warns about channels and also about unsupported PFs. Warnings displayed in import dialog.
      6fd222d...c3d70d0
  • (slightly unrelated) Show a more helpful error message when you try to import non-ADM - came across this during testing
    f15d61d...2768bc0

@firthm01 firthm01 self-assigned this Oct 18, 2023
@firthm01
Copy link
Contributor Author

firthm01 commented Nov 3, 2023

Suggestions implemented. Diff since last review; b2ec02f...a7d37bc

@firthm01 firthm01 merged commit e14ba77 into main Nov 3, 2023
6 checks passed
@firthm01 firthm01 deleted the 128ch-limit-warning branch November 3, 2023 12:24
@firthm01
Copy link
Contributor Author

Closes #244

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.

None yet

2 participants