Add ability to drag files to and from external applications
Adds the ability to drag files in and out of LMMS. This required a bit more refactoring than I expected.
https://github.com/user-attachments/assets/3f4d81ce-5e42-4d7a-b74b-1a557c951faa
I should note that this doesn't just work to and from the desktop, I've tested with firefox, CLion, dolphin and discord. It also works with more than just sample files (projects, presets, midis, vsts)
To hell with CodeFactor.
once #7627 is merged, I'd like to add the ability to drag sample clips out of LMMS too. That could be for a different PR though.
I tested this briefly, and it seems to work! I'm using nautilus on GNOME to drag files in.
Things I noticed (these aren't that important though):
- Dragging in .xml files which are not presets causes a dummy instrument(?) to be added, and when you click on it, LMMS crashes. This is sort of also in master, but it gives you a warning first.
- You can't drag in .sf2 files (I think this is in master too)
- You can't drag in vst effects into an effect chain
Dragging in .xml files which are not presets causes a dummy instrument(?) to be added, and when you click on it, LMMS crashes. This is sort of also in master, but it gives you a warning first.
Okay, this should be a relatively simple fix.
You can't drag in .sf2 files (I think this is in master too)
Thats actually not in master, it's is a regression- Nice catch!
You can't drag in vst effects into an effect chain
This is out of scope as it isn't in master, but it's definitely a good idea for another PR.
I tested this again, and those two issues do appear to be fixed!
In Windows MSVC, dragging instruments to the Song-Editor doesn't work Edit: I used the right-click menu to insert Mallets, OBS didn't capture the context menu
https://github.com/user-attachments/assets/bd10b2d7-b436-43b6-b8da-29b5f7439f61
I would like a few things changed. Also I'm testing this and When I drag a sample clip into AudioFileProcessor, it doesn't load the audio for me. Firstly create a sample clip and load audio, then create an empty AudioFileProcessor and try to drag the clip into the processor. Dragging in files from outside works nicely. What else to test?
huh, I don't know why I'm just now seeing this comment, sorry.
Master doesn't let me drag sample clips into AFP either. In fact, I think master behaves worse, as it accepts the drop, but just has weird behaviour. So I think this is out of scope. However, I was planning to add that once your sample exporting PR is merged.
@szeli1 fixed. The issue was being caused by the hasFormat() check
@szeli1 any idea why the builds might be failing on windows?
@szeli1 any idea why the builds might be failing on windows?
Add LMMS_EXPORT to the problematic functions, the plugins can't access them without LMMS_EXPORT.
@szeli1 Thanks, that worked! This should be ready for merge now.
this is a really weird issue, but I see no reason why my PR would cause this. Are you using a custom build to test master and AppImage to test my PR? if that's the case, you should test master with the AppImage to see if you get the same issue.
I am unable to reproduce this on the AppImage or custom build
@szeli1 do the keyboard shortcuts work? (super + right)
@szeli1 do the keyboard shortcuts work? (super + right)
I believe it didn't work for me when I tested it.
@szeli1 do the keyboard shortcuts work? (super + right)
Wait this happened to me in an independent branch, it seems like this is an unrelated bug
I think this is ready for merge.
Tested on Windows 11 26100.4484, no issues here from the most common workflows (dragging samples in/out sidebar, dragging samples into Song-Editor track buttons, dragging samples into Song-Editor tracks itself).
Although I find the behaviour of overlapping samples in sample-track instead of overwriting them if there's zero space between the start of the track and the next sample a bit unexpected, because it's different from dragging a sample into the sample-track's track itself
Although I find the behaviour of overlapping samples in sample-track instead of overwriting them if there's zero space between the start of the track and the next sample a bit unexpected, because it's different from dragging a sample into the sample-track's track itself
Could you please provide a video? I'm unclear on what you're describing. Also, is this behavior different to master?
Do you also drag/drop a project file (.mmpz, .mmp) into the MainWindow > Song? Or, did the video demo just missed it?
Do you also drag/drop a project file (.mmpz, .mmp) into the MainWindow > Song? Or, did the video demo just missed it?
yes, anything thats currently draggable from the filebrowser will be draggable from external applications.
Could you please provide a video? I'm unclear on what you're describing. Also, is this behavior different to master?
It's the same as master, I just checked
Why are my commits in here?
Why are my commits in here?
huh... good question. it appears i attempted to merge your search revamping PR into one of my branches and accidentally merged it into this one.
I'm not going to have access to a computer until next week, so ill fix it then (although I have no idea how i would go about doing that)
@sakertooth I've cleaned up the commits and done some testing to ensure it works as usual.
@messmerd Since your last review, all mimetypes but presetfile midifile and projectfile are generated dynamically, by looping through every plugin. I don't think its possible for the remaining 3 to be dynamically generated.
Snippet of Clipboard.cpp:
static std::map<std::string, std::vector<std::string>> mimetypes =
{
{"presetfile", {"xpf", "xml", "xiz", "lv2"}},
{"midifile", {"mid", "midi", "rmi"}},
{"projectfile", {"mmp", "mpt", "mmpz"}},
};
Is this more in line with what you were thinking?
As I mentioned before, I'd like to see some of the mime type stuff refactored so that plugin-specific information is no longer present in the core LMMS application. Without that, our plugins are not truly plugins, are they?
You've approved this PR while plugin keys are still present in the code.
Maybe plugin descriptors should contain a mime type in addition to the file extensions. That could then be queried through PluginFactory like file extensions can.
This did happen. I think it is nice that plugins define the extensions, so this is a good change.
For example, Sf2Player would provide both "sf2,sf3" and "soundfontfile" in its descriptor. Or maybe use FileItem::FileType::SoundFont instead.
The existence of this code makes the changes not really "plugin-specific"
bool isAudioFile(const QString& ext) { return isType(ext, "samplefile"); }
bool isProjectFile(const QString& ext) { return isType(ext, "projectfile"); }
bool isPluginPresetFile(const QString& ext) { return isType(ext, "pluginpresetfile"); }
bool isTrackPresetFile(const QString& ext) { return isType(ext, "trackpresetfile"); }
bool isSoundFontFile(const QString& ext) { return isType(ext, "soundfontfile"); }
bool isPatchFile(const QString& ext) { return isType(ext, "patchfile"); }
bool isMidiFile(const QString& ext) { return isType(ext, "midifile"); }
bool isVstPluginFile(const QString& ext) { return isType(ext, "vstpluginfile"); }
FileItem::FileType and the mime type seem to have an almost identical job, so there's probably also a simplification waiting to be made there - maybe a QString("filetype_%1").arg(static_cast
(fileType)) mime type similar to what tracks use. Idk. Doing stuff like this is probably necessary to remove all the plugin-specific code from Clipboard, TrackContainerView::dragEnterEvent, TrackContainerView::dropEvent, and elsewhere.
Usually file types need human readable names, so I think marking them with a number complicates things.
I believe there is a much cleaner and more maintainable design waiting to be implemented, and rather than going along with the original poorly-made design by adding additional plugin-specific data hardcoded into the main application, I think this PR provides the perfect opportunity to truly fix the problem.
Keys are still hardcoded into the application, only extensions differ which is an improvement, but doesn't achieve the main goal, it doesn't truly fix the problem.
If it turns out to not be possible to move all the plugin-specific info into the plugins, then I think quarantining it to the FileItem::FileType enum within the main application would be an acceptable compromise.
This didn't happen.
My main issue is that I would like to replace string keys with enum keys. This means keys like "vstpluginfile" must be defined for example in clipboard, so the design mistakes are kept. You've approved this PR @messmerd while these design mistakes are still present.
If this was done, the caller of getFile would need to know every single supported extension. then we run into the same issues of hardcoding that this PR solved.
I've mocked up a system of completely dynamic file types.
There are no functions like isVST() and no string literals like "*.sf2".
What I've learned the past days is that the clipboard system builds on unspoken rules and is in need of refactoring. Maybe we could add drag and drop without having to refactor... Idk @AW1534 would know. But at the moment, adding a lot of isFiletype() functions feels like a refactor in the wrong way.