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

[Native File Dialog] Add support for adding custom options to the dialogs. #83480

Closed
wants to merge 1 commit into from

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Oct 17, 2023

  • Add support for adding custom options (checkboxes and optionboxes) to the dialogs (both native and built-in).
  • Add support for using native dialogs in the editor.
Screenshots Screenshot Windows Screenshot Linux Screenshot macOS Screenshot built-in

Related proposal: godotengine/godot-proposals#1123
Fixes #86351

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally on Linux (KDE), it works as expected.

Load

Screenshot_20231207_022159

Save

Screenshot_20231207_022233

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Dec 7, 2023
@akien-mga
Copy link
Member

Will need a rebase, and filling the documentation.

@@ -413,6 +413,7 @@ void EditorSettings::_load_defaults(Ref<ConfigFile> p_extra_config) {
set_restart_if_changed("interface/editor/debug/enable_pseudolocalization", true);
// Use pseudolocalization in editor.
EDITOR_SETTING_USAGE(Variant::BOOL, PROPERTY_HINT_NONE, "interface/editor/use_embedded_menu", false, "", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_RESTART_IF_CHANGED)
EDITOR_SETTING_USAGE(Variant::BOOL, PROPERTY_HINT_NONE, "interface/editor/use_native_file_dialogs", false, "", PROPERTY_USAGE_DEFAULT)
Copy link
Member

@KoBeWi KoBeWi Jan 13, 2024

Choose a reason for hiding this comment

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

Any plans to enable it by default?
EDIT:
nvm, seems like it's not fully functional yet.

Copy link
Member

Choose a reason for hiding this comment

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

Seems like all editor problems were resolved, so this can be enabled by default to get more testing.
Though might be worth double-checking if it behaves correctly on unsupported platforms too.

Comment on lines 1067 to 1087
Node *child = grid_options->get_child(0);
grid_options->remove_child(child);
child->queue_free();
Copy link
Member

@KoBeWi KoBeWi Jan 13, 2024

Choose a reason for hiding this comment

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

Pretty sure you can just do memdelete().

Copy link
Member Author

Choose a reason for hiding this comment

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

Every similar instance seems to be using queue_free().

@KoBeWi

This comment was marked as resolved.

@bruvzg
Copy link
Member Author

bruvzg commented Jan 13, 2024

Adding, removing or editing options in the FileDialog do not update it immediately. You need to toggle visibility for it to have effect. I think it would be useful to update the values immediately, at least in the editor, so you can preview the dialog in realtime.

This can't be done with the native dialogs, there's no way to control it after showing (in case of macOS and Linux, native dialogs are show by completely different process to access file system outside sandbox).

Also native dialog for opening scenes does not show at all.

I can't reproduce it.

Seems like it's a problem with all file saving (event export).

It's a pre-existing issue with the native dialogs (#86351), should be fixed now.

@KoBeWi
Copy link
Member

KoBeWi commented Jan 13, 2024

This can't be done with the native dialogs, there's no way to control it after showing

I don't mean native dialogs. I mean a FileDialog placed in the editor. It doesn't update, you need to hide/show it.

I can't reproduce it.

I'm using Windows 10 if that matters.

@bruvzg bruvzg force-pushed the fd_options branch 2 times, most recently from 47292d5 to e57d34a Compare January 13, 2024 22:37
@bruvzg
Copy link
Member Author

bruvzg commented Jan 13, 2024

I don't mean native dialogs. I mean a FileDialog placed in the editor. It doesn't update, you need to hide/show it.

Ah, I see. It should update now.

I'm using Windows 10 if that matters.

I'll re-check it on Windows a bit later.

@KoBeWi
Copy link
Member

KoBeWi commented Jan 15, 2024

Restested. Scene dialogs are now working correctly, saving is still wrong. It does add extension, but a wrong one, e.g. saving tscn file will make scn.

@bruvzg bruvzg force-pushed the fd_options branch 2 times, most recently from 50dcf09 to b4dc13b Compare January 15, 2024 20:44
@bruvzg
Copy link
Member Author

bruvzg commented Jan 15, 2024

It does add extension, but a wrong one, e.g. saving tscn file will make scn.

Rechecked Windows API docs, seems like it's using one-based index for file filter return, should be fixed now.

@YuriSizov
Copy link
Contributor

  • Add support for using native dialogs in the editor.

So this conflicts with this feature: #79313 used by #79316.

I don't think we can expose it as a setting if we rely on these custom layouts for UX, and we definitely cannot enable it by default.

@bruvzg
Copy link
Member Author

bruvzg commented Jan 17, 2024

#79313 used by #79316.

That's definitely impossible to implement in the native dialogs (at least on Linux, on Windows and macOS we can add any standard OS controls).

@YuriSizov
Copy link
Contributor

YuriSizov commented Jan 17, 2024

That's definitely impossible to implement in the native dialogs.

Yeah, exactly my point. So if we want to offer this kind of user experience, I don't think we can provide native dialogs as a feature for the editor.

Edit: An option could be to redesign the flow so you configure these options before we show the file dialog. But I got to admit, based on my experience with Blender, having it on the same screen is very convenient.

@ssokolow
Copy link

As a data point in the other direction, I'd give up that proposed workflow in a heartbeat if it meant having my KDE Places (bookmarks) sidebar present and Just Working™... and that's before you count how I use Flatpak specifically so I can customize and lock down my sandboxing easily.

@bruvzg
Copy link
Member Author

bruvzg commented Jan 17, 2024

For the reference, I have opened separate PR with non-editor changes only - #87303

Personally, I would prefer native dialogs over this sidebar UI, but I'm not sure what would be the best replacement for it.

doc/classes/EditorFileDialog.xml Outdated Show resolved Hide resolved
doc/classes/EditorFileDialog.xml Outdated Show resolved Hide resolved
doc/classes/EditorFileDialog.xml Outdated Show resolved Hide resolved
doc/classes/EditorFileDialog.xml Outdated Show resolved Hide resolved
doc/classes/EditorFileDialog.xml Outdated Show resolved Hide resolved
doc/classes/FileDialog.xml Outdated Show resolved Hide resolved
doc/classes/FileDialog.xml Outdated Show resolved Hide resolved
doc/classes/FileDialog.xml Outdated Show resolved Hide resolved
doc/classes/FileDialog.xml Outdated Show resolved Hide resolved
doc/classes/FileDialog.xml Outdated Show resolved Hide resolved
OverloadedOrama added a commit to Orama-Interactive/Pixelorama that referenced this pull request Jan 24, 2024
Closes #274 and implements #568, at long last! Some issues remain:
- The native save pxo dialog doesn't have an "Include blended images" option. This will be fixed once godotengine/godot#83480 is merged.
- When a native file dialog closes, the interface still remains dimmed.
- In the export dialog, the "Browse" file dialog will also close the export dialog itself when it closes, when it's native.
@bruvzg bruvzg requested a review from a team as a code owner January 25, 2024 15:38
…logs.

Add support for adding custom options (checkboxes and optionboxes) to the dialogs (both native and built-in).
Add support for using native dialogs in the editor.
@bruvzg
Copy link
Member Author

bruvzg commented Mar 21, 2024

Superseded by #87303 and #89735.

@bruvzg bruvzg closed this Mar 21, 2024
@AThousandShips AThousandShips removed this from the 4.3 milestone Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FileDialog not return the extension when use_native_dialog
7 participants