-
-
Notifications
You must be signed in to change notification settings - Fork 106
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
Clip Names #2293
Clip Names #2293
Conversation
The code has a serious bug that I need to resolve :) Will dive into it. |
|
||
if (output->type == OutputType::AUDIO) { | ||
AudioClip* audioclip = (AudioClip*)clip; | ||
clip->clipName.set(&audioclip->sampleHolder.filePath); |
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.
this shouldn't be handled by the view, it should be set when the audio clip's file path is set or recording is finalized
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.
Going to change this part and look where the entry point is ( maybe on a few different places )
src/deluge/gui/ui/sound_editor.cpp
Outdated
} | ||
} | ||
// if else then just keep it like the old way to rename the kit row item | ||
goto doNextAfterName; |
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.
this is in-extensible (nobody can add another condition to the if else chain without breaking the goto)
Could you put all the conditions into the if statement? Split this block out to a function that returns true if the editing took place and false otherwise so it continues down the chain. Then just return dealt with if the condition is met
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 also don't see why audio clips are excluded
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.
Agreed, was a bit of a dirty patch here with the label. Will wrap it in a function.
706a1e5
Clip Names support for the Deluge
Tried to make the codebase as small as possible and tested with different scenarios. It's my first C++, please let me know if there are errors, so I can learn from it.
Many thanks!