Prevent duplicate 3D model files #787
Conversation
Add Model allows multiple file selection
| @@ -55,7 +97,8 @@ std::string ImpPackage::ask_3d_model_filename(const std::string ¤t_filenam | |||
| filter->add_pattern("*.STP"); | |||
| chooser->add_filter(filter); | |||
| if (current_filename.size()) { | |||
| chooser->set_filename(Glib::build_filename(pool->get_base_path(), current_filename)); | |||
There was a problem hiding this comment.
https://docs.gtk.org/gtk3/method.FileChooser.set_filename.html
says this should only be used in a save chooser, I found it caused problems with the directory defaulting to the last used directory instead of the model directory
There was a problem hiding this comment.
Interesting, for me it does work.
EDIT: only once...
EDIT2: nvm EDIT, different issue
| @@ -83,16 +83,21 @@ ModelEditor::ModelEditor(ImpPackage &iimp, const UUID &iuu) | |||
|
|
|||
| auto browse_button = Gtk::manage(new Gtk::Button("Browse…")); | |||
| browse_button->signal_clicked().connect([this, entry] { | |||
| Package::Model *model2 = nullptr; | |||
| if (imp.core_package.models.count(uu)) { | |||
There was a problem hiding this comment.
I'm unable to a see a code path where imp.core_package.models[uu] goes away between the constructor and here, so I removed the check and directly use imp.core_package.models.at(uu). If the delete button is used then this button goes away too.
src/imp/3d/imp_package_3d.cpp
Outdated
| rel_names.push_back(rel_path); | ||
| } | ||
| if (std::none_of(rel_names.cbegin(), rel_names.cend(), | ||
| std::bind(std::string::empty, std::placeholders::_1))) { |
There was a problem hiding this comment.
This fails to build on all platforms and compiler versions in the CI. As far as i can tell what you're trying to do here is undefined behavior since you're trying to take the address of a standard library function: https://en.cppreference.com/w/cpp/language/extending_std
I suggest using a lambda function instead.
There was a problem hiding this comment.
@carrotIndustries looks like the CI is happy after switching to a lambda
|
LGTM, i'd like to keep |
|
Hmm, I suspect it's a GTK issue with |
|
To me at least the file picker defaulting to what's in the entry is part of a quality user experience. If it doesn't work on windows, I think the best way around this is an |
|
I think this a useful feature to have, but should be separated from the |
The model map doesn't check for duplicate values, so you can add the same file several times, which seems like a bug.
Also, for packages that use multiple 3D models, it seems convenient to allow selecting multiple files in the chooser. Any chosen files that are already present get ignored.