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

Adds a button to clone the current BB track pattern inside the BB Editor #5531

Merged
merged 2 commits into from
Jun 15, 2020

Conversation

IanCaio
Copy link
Contributor

@IanCaio IanCaio commented Jun 11, 2020

Issue

I noticed a common situation during the music production workflow on LMMS. Sometimes I was working drumlines inside the BBEditor and for a small section I wanted to add a variation (i.e.: some drum fill). Currently there are two options:

  1. Cloning the track on the song editor to keep the current pattern, editing the pattern on the BBEditor to add the variation wanted and then clearing the track on the song editor, adding just the bars where this variation should play.
  2. Adding a new empty BB track, copying all the pattern by hand adding the variations wanted and then adding the bars on the song editor.

I realized it would make the process a little quicker if there was a button inside the BBEditor that just cloned the pattern, but not the song editor content (which is usually the desirable behavior on that case).

Proposed PR solution

This PR adds a small button on the BBEditor, next to the "Add Beat/Bassline", that clones the current BBTrack pattern, but also clears the content of the song editor. For now the icon being used is a copy of the edit_copy.png icon, but it can be changed to something more distinct.

Adds a button on the BBEditor that clones the current BB track pattern, but without also cloning the song editor TCOs. That can be useful when an user is editing drumlines and wants to make a section with a slight variation for example.
@LmmsBot
Copy link

LmmsBot commented Jun 11, 2020

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Linux

macOS

Windows

🤖
{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://6933-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.700-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/6933?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://6935-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.700-mac10.13.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/6935?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://6937-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.700-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/6937?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://6936-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.700-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/6936?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/3d9lhkbei2c3c1g0/artifacts/build/lmms-1.2.1-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/33501931"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/4bb2ll2yimnw9olp/artifacts/build/lmms-1.2.1-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/33501931"}]}, "commit_sha": "b1c1d146013ac2b47d3cec0c15ab99236bc97682"}

Copy link
Member

@Spekular Spekular left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of copying the innards of the clone and clear functions, why not just call the existing clone function, then the existing "clear track" function? That should be DRYer.

@IanCaio
Copy link
Contributor Author

IanCaio commented Jun 11, 2020

Instead of copying the innards of the clone and clear functions, why not just call the existing clone function, then the existing "clear track" function? That should be DRYer.

@Spekular The cloneTrack and clearTrack methods are from the TrackOperationsWidget class, but I haven't figured out yet how to access the TrackOperationsWidget from a particular Track object (even my failed attempts looked a little verbose). Is there an easy way I have missed?

@Spekular
Copy link
Member

Having taken a closer look, no there probably isn't without moving things around, since clone and clear are private slots. Even if you get the widget with

BBTrackContainer *bbtc = static_cast<BBTrackContainer*>(model());
BBTrack *bbt = BBTrack::findBBTrack(bbtc->currentBB());
auto *widget = bbt.getTrackView()->getTrackOperationsWidget();

you wouldn't be able to call the methods. I guess I'm okay with this version then, although I think it'd be nice if you added a call to bbt->addJournalCheckPoint(); before the clone so it can be undone.

include/BBEditor.h Outdated Show resolved Hide resolved
src/gui/editors/BBEditor.cpp Outdated Show resolved Hide resolved
@Spekular
Copy link
Member

From conversation on Discord, it seems the journal system doesn't handle tracks being added/deleted. With that in mind I don't have any big issues with this PR anymore, but I left a review with some nitpicks. I'm willing to merge once they're addressed if no one else disagrees.

- Changes method name from cloneBBTrackPattern to clonePattern
- Small fix on the comments
- Adds a TODO comment regarding reusing the code from TrackOperationsWidget as a reference, so we can later figure out a way to not repeat the code
@IanCaio
Copy link
Contributor Author

IanCaio commented Jun 13, 2020

From conversation on Discord, it seems the journal system doesn't handle tracks being added/deleted. With that in mind I don't have any big issues with this PR anymore, but I left a review with some nitpicks. I'm willing to merge once they're addressed if no one else disagrees.

Thanks for the review @Spekular , I made a new commit with the requested changes!

@Spekular
Copy link
Member

Awesome, thanks! I'll wait a day or two with merging so others can chime in if they want. Feel free to ping me on Monday if I forget :)

@Spekular Spekular merged commit 733a411 into LMMS:master Jun 15, 2020
@IanCaio IanCaio deleted the feature/cloneBBTrackPattern branch August 29, 2020 12:06
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
Adds a button to clone the current BB track pattern inside the BB Editor
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.

3 participants