-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fixes a bug with the beat+bassline editor #2880
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
Conversation
Before, adding a new track would have the default size regardless of the width of other tracks. This change fixes this, where the newly added track will have the same width as existing ones.
I accidentally left in two "std::cout" debug prints in src/gui/editors/BBEditor.cpp . |
@Stephen-Seo just update your branch and the PR should update as well. I don't feel qualified to review the code, but nice job adding this! |
Correct, we don't modify code on merge, this will need to be fixed in your branch and as @Spekular stated, this PR will be updated to reflect your changes. |
Ok, I removed the accidental debug prints. Hopefully everything else is ok. |
Pattern* p = static_cast<Pattern *>( ( *it )->getTCO( m_bbtc->currentBB() ) ); | ||
steps = p->getSteps(); | ||
break; | ||
} | ||
} | ||
|
||
std::cout << m_bbtc->currentBB() << std::endl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
I will fix new tracks in the BB for all BBs in the next commit to my fork. As for why I'm using the signal/slot path, I need references to the other tracks (Patterns?) in the BB which is accessible from BBEditor to get their current size for the recently added track to have the same size. |
The bug was that newly added tracks to the BeatBasslineEditor would not have the same size as pre-existing tracks in the BBE. Previous fix only worked for the current BeatBassline, not others. This commit fixes this bug for all BBs.
Ok, the fix now works for all existing BBs. My understanding is that increasing the size of a track in the BB requires a call to Pattern::addSteps(). I think an alternate way of fixing this could be for "addSteps" and "removeSteps" in Pattern to actually call a "setSteps" which is called for each Pattern, ensuring they are the same size. This would also need to be called for newly added Patterns (they inherit from TCOs) which means the size also must be stored somewhere. Probably in the class that manages Patterns? |
After a lot of thought, I'm second guessing the validity of my pull request. It appears that the Patterns/TCOs are hooked up directly to the buttons that increase/decrease the size. A more involved fix could have something keep track of the size per BB in between the buttons and the Patterns/TCOs. Newly added Tracks would refer to this in-between object and would resize themselves, or the in-between object would keep track of newly added Tracks to resize them. This kind of fix may be better in terms of design. Do you think I should have another go at it? |
It should be simpler. I am confident that this can be solved in the constructor of |
As jasp00 suggested I made the fix in the constructor of Pattern. I'm closing this PR because I made the fix in another branch. Thus, I will make another PR with that branch. |
Before, adding a new track to the beat+bassline editor would have the default size regardless
of the width of other tracks.
This change fixes this, where the newly added track will have the
same width as existing ones.
This bug is mentioned in issue #2476 .