-
Notifications
You must be signed in to change notification settings - Fork 21
Mustang I V2 effects #37
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
Open
krskajo1
wants to merge
8
commits into
offa:master
Choose a base branch
from
krskajo1:mustang_i_v2_effects
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ackbox, Big Fuzz, Wah Modulation, Touch Wah Modulation, Diatonic Pitch Shifter).
include/com/IdLookup.h
Outdated
|
|
||
|
|
||
| constexpr effects lookupEffectById(std::uint8_t id) | ||
| constexpr effects lookupEffectById(std::uint8_t id, std::uint8_t idmsb=0) |
Check warning
Code scanning / CodeQL
Poorly documented large function Warning
Poorly documented function: fewer than 2% comments for a function of 101 lines.
Author
|
I realized I forgot to implement and run tests. I will implement them and update the PR. |
Author
|
Tests and formatting corrected. This PR is now ready for review. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello,
this PR adds support for the missing Mustang I V2 effects (Ranger Boost, Greenbox, Orangebox, Blackbox, Big fuzz) and modulations (Wah, Touch Wah, Diatonic Pitch Shifter). The support includes default effect and effect windows.
Main change of this commit is in the ID of individual effects. It seems that they are in fact 16-bit wide, instead of current 8-bit width. This mainly influenced the function
lookupEffectById, where I added second parameteruint8_t idmsbwith implicit value of0.The newly added effects are currently appended to the bottom of the combobox list in the effect window, as their position must correspond with their value in the
effectsenum.Additionally I've noticed there is a lot of duplicate code, especially in default effects and effects windows regarding knob labels. I know there might be a good reason for it, such as proper shortcut function, however, I think that by doing this line-efficiently would benefit legibility of the code and make editing it easier.
One solution I come up with and implemented for the new effects utilizes maps to store knob description for given knob like so
and sets for an effect
Alternatively, creating a class for each effect (deriving from effect base class), which would define its labels, is also an option.
Please, let me know if you do or don't agree with the map approach and or anything in this PR and what should be changed. I will gladly make the necessary edits.
Best regards
Joe