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

Merge From Scene option invokes two file handling dialogs #36474

Closed
Jose-Moreno opened this issue Feb 23, 2020 · 4 comments
Closed

Merge From Scene option invokes two file handling dialogs #36474

Jose-Moreno opened this issue Feb 23, 2020 · 4 comments

Comments

@Jose-Moreno
Copy link

Jose-Moreno commented Feb 23, 2020

Godot version:
v3.2.Stable.Official

OS/device including version:
Windows 10 Pro Version 1809 OS Build 17763.973

Issue description:
Using the Merge From Scene option from any node, Root, Inherited or Instance will open two distinct file dialogs. One to open a file, and another one to import a file.

Video Demo

  1. This is a video demonstration of the issue
  2. This is a screenshot of the issue

To me it seems that it should only open "one" window to load a file instead of two. Someone on discord helped me triage and faced the same issue.

I'm not a c++ expert but could it be that the FILE_IMPORT_SUBSCENE case is falling through to the FILE_EXTERNAL_OPEN_SCENE case found here:

case FILE_IMPORT_SUBSCENE: {

Steps to reproduce:

  1. Create a new scene
  2. Create a root node (the node type doesn't matter I used Node2D, the other user used base Node)
  3. Right click on the node and click on the Merge From Scene context option

Minimal reproduction project:
With the reproduction steps you get a 100% reproduction rate.

Edit: Updated description to take less space and added video showcasing the problem

@timothyqiu
Copy link
Member

timothyqiu commented Feb 24, 2020

This is a feature introduced by #28292. If no scene is selected, the file dialog is automatically opened (as the only thing you can do in the dialog is to click the Browse button).

if (p_what == NOTIFICATION_VISIBILITY_CHANGED) {
if (is_visible() && scene == NULL)
_path_browse();
}

Maybe the title of the file dialog should be changed to something like "Choose a Scene".

@Jose-Moreno
Copy link
Author

@timothyqiu Hi thank you for your comment, however I don't think we're talking about the same issue?
Can you please read the description in detail? The problem is not about the name of the dialog, and just in case the scene is set as main scene, but the issue happens regardless of this.

If you follow the reproduction steps you should experience the problem i'm reporting, I know it's a simple thing but I don't see how automatically opening two different dialogs at the same time helps with the merge from scene functionality.

I'll update the screenshots as it seems there's a misunderstanding here, but what i'm reporting is clearly a UI / UX bug.

@Calinou Calinou changed the title [3.2 Stable][Bug] Merge From Scene option invokes two file handling dialogs Merge From Scene option invokes two file handling dialogs Feb 26, 2020
@Jose-Moreno
Copy link
Author

Jose-Moreno commented Feb 26, 2020

@timothyqiu Ok, now I have some more time. I was reading on the phone previously so apologies for any inconvenience. Once again thank you for taking the time to review this.

After reading the PR I think I understand what you meant now as well. So the file selector dialog a.k.a open a file dialog, was implemented on that PR to occur automatically the first time the Merge From Sceneis used and a file is chosen. It will not be sprung afterwards.

This dialog will then appear ON TOP of the node import without further notice, but after selecting a scene, the next time we use the merge from scene option, only the node import will appear and you will have to click on the browse button to prompt this dialog.

To me it was confusing when the window opened at first when I was expecting only the node import window, but I understand the reasoning of it happening the first time.

With this, I unfortunately think this should still be called out as a UI / UX bug, but perhaps it's something that could be "fixed" through documentation rather than source code regression. Although for the sake of consistency I think the open file dialog should not appear on it's own as it feels it does not respect user choice and could confuse a new user.

So should I close this and ask for documentation instead? or do you think this issue can be used to prompt any modification to this behavior? Thanks in advance for your time.

@KoBeWi
Copy link
Member

KoBeWi commented Dec 20, 2020

This doesn't look like a bug. The FileDialog is needed, the second dialog is not FileDialog, it's a separate dialog that displays scene hierarchy. It's impossible/not worth it to merge them.

Also the Merge from scene will be probably removed as obsolete after #34892 is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants