Fix setting animation save paths on import breaking on Windows #90003
Conversation
AThousandShips
added
the
cherrypick:4.2
There was a problem hiding this comment.
Fixes an important bug on windows, so I'm approving this because 99% of the problem cases are covered by the change as written and I want to make sure it gets in.
Just a comment but should we check if material and mesh extraction has the same problem? perhaps we should validate_filename() in all cases
I think this problem stemmed because the Extract window doesn't offer a way to rename the extracted animations. I don't think people ever want their animations to be named "Armature|" or "Armature_" or "Take 001", but it's hard to edit from the extract dialog.
Anyway, improving the extraction flow would require an updated design, and it's important to get this bug fixed, so I think this is a good change.
| @@ -1448,7 +1448,8 @@ void SceneImportSettingsDialog::_save_dir_callback(const String &p_path) { | |||
| item->set_metadata(0, E.key); | |||
| item->set_editable(0, true); | |||
| item->set_checked(0, true); | |||
| String path = p_path.path_join(name); | |||
| // Replacing | (pipe) with _ (underscore) for compatibility with the Windows file system. | |||
| String path = p_path.path_join(name.replace("|", "_")); | |||
There was a problem hiding this comment.
I would consider using String::validate_filename() instead of specifically replacing | with _. Basically, there are many illegal filename characters and we should replace all of them.
the validate_filename will replace characters with _ so it should work the same for the case you did.
There was a problem hiding this comment.
Thank you for your replies.
As far as I know, this only automatically happens in animation names in Blender, but I guess anyone could actually insert | in whatever they decide to name their material and meshes names.
I agree String::validate_filename() is better, I didn't know it existed and it's perfect for the use case. I guess I'll change that and wrap both the material and mesh names in the method as well.
A cool addition to this imo would be to display an error message or a pop-up whenever a user is trying to save their resources using invalid characters, as the previous error was a little cryptic. I don't know how much value there is in letting non-windows users use those characters in filenames either. What do you think would be the most appropriate solution?
For now, I'm gonna make the quick changes you mentioned and force-push them asap!
AlexOtsuka
force-pushed
the
fix-animation-save-paths
branch
from
f5eb9b0 to
4d3319e
Compare
akien-mga
added
the
cherrypick:4.1
|
Thanks! |
|
Cherry-picked for 4.2.2. |
akien-mga
removed
the
cherrypick:4.2
|
Cherry-picked for 4.1.4. |
akien-mga
removed
the
cherrypick:4.1
akien-mga
changed the title
Fixes #86279.
Blender uses the | (pipe or vertical bar) character in its animation names which is forbidden in Windows file names. My fix simply replaces those with a _ (underscore) in the automatic path suggestion code when using the "Set Animation Save Paths" option.
There are other possible approaches to this, such as:
I chose to go for this as it seems like a safe, convenient fix with no obvious downsides.