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

Remove node deletion confirmation #30816

Closed
wants to merge 1 commit into from
Closed

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Jul 25, 2019

Proposal to close #30805

Node deletion is 100% always undo-able (nodes aren't even freed from memory upon removing), so it makes sense that it doesn't need confirmation (which is also a case in other software, like mentioned in the linked issue).

I could also change it into optional editor setting if that would be considered better for whatever reason.

@groud
Copy link
Member

groud commented Jul 25, 2019

I agree it should be removed completely.

@ghost
Copy link

ghost commented Jul 25, 2019

We already have shift delete. Why remove this?

@KoBeWi
Copy link
Member Author

KoBeWi commented Jul 25, 2019

We already have shift delete. Why remove this?

Why have a shortcut for something that should be a default behavior? Deleting nodes isn't unsafe (unless you don't notice it for some reason) and no confirmation means more seamless work.

@akien-mga
Copy link
Member

akien-mga commented Jul 25, 2019

Deleting nodes isn't unsafe (unless you don't notice it for some reason)

That's actually the main reason for this confirmation dialog AFAIK. Different editor panels can have different selections, and Godot is relatively bad at conveying which one has the focus and would be impacted by pressing Delete. It's not unlikely that you want to delete something in one editor plugin and end up deleting a node in the SceneTreeDock, without a clear visual feedback on what happened, so you don't notice it.

Until we're confident that these focus and contextual behavior issues are properly handled, I'd be a bit cautious with making node deletion easier.

@KoBeWi
Copy link
Member Author

KoBeWi commented Jul 25, 2019

So, editor option for now? (with checkbox in the dialog)
Or this PR will wait until then? Not that I mind >_>

@groud
Copy link
Member

groud commented Jul 25, 2019

So, editor option for now? (with checkbox in the dialog)

I'd rather not go for that. That's kind of settings bloat.

@distantforest1
Copy link

My 2 cents, I didnt know about the shift-delete hotkey till coming here. Prior to this, I checked if the editor had this setting when I first started godot. I checked a few more times and couldn't find it by entering in the editor setting search and such. It should be something that is easy to find imo, not many software has a shift-delete option so it's not common enough for people to search the hotkeys as well.

@TheSHEEEP
Copy link

TheSHEEEP commented Jul 26, 2019

Until we're confident that these focus and contextual behavior issues are properly handled, I'd be a bit cautious with making node deletion easier.

That's why it should be made an option. Make the default to ask for confirmation and add an option not to show that confirmation dialog.
Many people, like myself, are way more annoyed by constantly being pestered for confirmation than they are by accidentally doing things. And it isn't you who should determine what is more important to the users, it's the users themselves.
An option wouldn't affect anyone in a negative way. @KoBeWi Thus I think making this an option, and defaulting to actually showing the dialog, would make most sense.

That's kind of settings bloat.

There's no such thing as too many settings.
Only a matter of presenting them in a clean manner, which Godot does very well IMO. I mean, how many editiors let you text-search for options in the options menu? That's just awesome.

@LikeLakers2
Copy link
Contributor

LikeLakers2 commented Jul 26, 2019

[@KoBeWi] So, editor option for now? (with checkbox in the dialog)

I'd be cool with this. Having an option to skip the deletion dialog would be a welcome addition in my book (even if I won't use it).

[@groud] I'd rather not go for that. That's kind of settings bloat.

I disagree with this being "settings bloat." Unless we can somehow think of a way to satisfy the majority of people on both sides of the matter without adding this, I would recommend adding something such as that, since it could be considered beneficial, even if not essential to everyone. And I'd consider something that is beneficial to not be bloat.

@groud
Copy link
Member

groud commented Jul 26, 2019

There's no such thing as too many settings.

Yes there is, and that a big problem for a lot of open source software. When you search for important things it gets buried into a lot of other cryptic or pointless settings. The UX in such bloated settings is just horrible.

Something like "disabling confirmation for deletion" definitely is one of those settings that is bloat, hidding the other important settings, as it only serves as a patch of a badly designed UX. There are plenty of those useless settings, and I am against adding more.

So removing the confirmation is either a good thing or not. Settings for the sake of it does not make sense.

I disagree with this being "settings bloat." Unless we can somehow think of a way to satisfy the majority of people on both sides of the matter without adding this, I would recommend adding something such as that, since it could be considered beneficial, even if not essential to everyone.

The solution is simply to make the shift + del shortcut more discoverable. That's it.

@LikeLakers2
Copy link
Contributor

The solution is simply to make the shift + del shortcut more discoverable. That's it.

While that is indeed a solution (and regardless of this being an editor option, I would still recommend we make the shortcut more discoverable), I don't actually see how it causes a proposed editor option to be considered bloat until proven otherwise. In fact, I fail to see how you could even consider it bloat in the first place, considering how beneficial it could be to some users over having to reach over to the shift key every time.

(submitted this before I was finished last time, woops!)

@groud
Copy link
Member

groud commented Jul 26, 2019

I don't actually see how it causes a proposed editor option to be considered bloat until proven otherwise. See, I would argue that this "bloat" is little more than a few lines of code that adds next to nothing when you consider it against the rest of the editor settings.

Because, as I said, there are plenty other settings like this one. They are piling up because we do not manage to reach consensus on a lot of points, one after the other. We add settings for things that disturbs one person out of 200, while making the UX worse for all others.

@LikeLakers2
Copy link
Contributor

Because, as I said, there are plenty other settings like this one. They are piling up because we do not manage to reach consensus on a lot of points, one after the other. So for the 0.5% that will find interest in using this settings, we make the UX worse for the 99.5% others.

Right, well... having made a quick search through the issue section of this repo, I don't notice any issues related to editor settings bloat, only one related to engine bloat (and the only mention of settings on that page is Will explaining how ProjectSettings works). If we're now complaining about how many editor settings are bloat, I would highly recommend you open a new issue about this. This claim of many settings being bloat is news to me.

Anyways, I fail to see how UX could be made worse because an option is not used. Would you care to explain?

@groud
Copy link
Member

groud commented Jul 26, 2019

[I was editing my comment while you responded, so creating a new one]

I mean, have a look to this settings page:
2019-07-26-095511_3840x1600_scrot

  • Why would I want the main font to be anti-aliased and not the code font one ? If this is for performance, why not put that into a performance category.
  • Is hinting worth to be configured ?
  • What's to point of making the Dim amount configurable ? And the transition time ? Does it help people making games faster ? I understand making the editor colors configurable makes sense, but this does not.
  • What are those low processor mode ? I have no clues how useful are those numbers.
  • I don't even know what "distraction mode" means ^^
  • And finally, the famous setting for "quit confirmation"...

The only thing I believe useful here are the language, the editor scale, the main and code font choice and their respective sizes. And that's only what I expect to see in the editor settings, not surrounded by plenty of almost useless settings.
If we still want more granularity in the editor settings, for less important settings, we can implement and "advanced" mode or something like that, a little bit like the "about:settings" page of Firefox.

@LikeLakers2
Copy link
Contributor

LikeLakers2 commented Jul 26, 2019

Indeed, I can see how those might be considered bloat, but that's because they are confusing and don't explain their purpose in a simple manner, or perhaps offer too much customization to the point of effectively being redundant or overkill.

That said, I don't see how you could compare this to those, considering the preference whether to skip the confirmation dialog is purely subjective, and not something we could reasonably expect to figure out on our own (no matter how many percentages you want to guess and throw out there). Sure, we do have a shortcut to skip it, and we should make it more visible regardless, but I'm not sure that forcing the shortcut as the only option for this would be a reasonable solution for everyone who wants an option to skip it, because forcing the shortcut would now mean everyone has to spend a moment pushing the shift key before they hit the delete key.

@groud
Copy link
Member

groud commented Jul 26, 2019

I will open an issue to discuss what can be done. An "advanced settings" tab could do the job in solving all of this. :)

@TheSHEEEP
Copy link

The only thing I believe useful

See, that's the problem here.
You take your own opinion to determine what others might find useful or not. Just because you don't understand something, or don't find it useful, doesn't mean it's the truth or anyone has to agree with you.
There is no consensus that can be reached here, because everyone has different needs - and to exclude some people just because that would mean "another option in a list" is a rather weak argument. It's not like anyone would constantly be browsing the options menu to gaze at its magnificence ;)
And if an option is unclear, that just means it needs a better explanation, not that it should be removed.

That said, I'm all for having more fine-grained settings to prevent all "other"-category options to be lumped in the same menu.

@KoBeWi
Copy link
Member Author

KoBeWi commented Jul 26, 2019

everyone has to spend a moment pushing the shift key before they hit the delete key.

Well, the shortcut is configurable, so you can rebind it to only Delete.

@TheSHEEEP
Copy link

TheSHEEEP commented Jul 26, 2019

Well, the shortcut is configurable, so you can rebind it to only Delete.

I do feel kinda silly for not thinking of this. It kinda resolves the entire problem, doesn't it?
10+ years of working with various editors without rebindable shortcuts really messes with you...

@groud
Copy link
Member

groud commented Jul 26, 2019

See, that's the problem here. You take your own opinion to determine what others might find useful or not. Just because you don't understand something, or don't find it useful, doesn't mean it's the truth or anyone has to agree with you.

An option has to be proven useful for a significant amount of people. My opinion was indeed an example about what a lambda user (as I am), would feel when seeing this. I believe those options are not useful for a majority of people, and that's why they should be trimmed.

There is no consensus that can be reached here, because everyone has different needs - and to exclude some people just because that would mean "another option in a list" is a rather weak argument.

If you thing having too many settings does not hurt, I am sorry, you are wrong here. Having too many settings hurts discoverability and makes users think Godot is "too complex". As we have far more people that find Godot hard to understand than people finding the deletion confirmation popup annoying, this option has more drawback that advantages.

Godot is not designed to please everyone, it is designed to be a professional, easy to use and intuitive tool. And having too many overly specific settings goes against that.

@Xrayez
Copy link
Contributor

Xrayez commented Jul 26, 2019

I believe those options are not useful for a majority of people, and that's why they should be trimmed.

It would be really good to have some kind of a poll to determine whether certain options/features are useful or not, that would also save people time from working on features that are not welcomed in Godot. I'm looking forward to using godot-proposals for that matter. 😃

@groud
Copy link
Member

groud commented Jul 26, 2019

I just wanted to share this scientific paper to back my point. :p
See the "What is the “cost” of too many knobs?" paragraph.

@KoBeWi
Copy link
Member Author

KoBeWi commented Jul 26, 2019

I do feel kinda silly for not thinking of this. It kinda resolves the entire problem, doesn't it?

I see another problem here - you need to know that such shortcut exists.

How about this?
image

@groud
Copy link
Member

groud commented Jul 26, 2019

No, that's likely too much. I think it should be in the tooltip, not more.

@TheSHEEEP
Copy link

I just wanted to share this scientific paper to back my point. :p

From the very paper itself: "However, our study only focuses on system software; we do not intend to draw any conclusion about other types of software, such as desktop software and mobile applications"
And indeed that argument just doesn't hold any water when applied to Godot's Editor. The editor is not even close to system software, it is 100% end-user software (yes, developers can be end-users, too). End-users use the options if they require to change something - in which case the only thing that matters is if what they want to change can be done or not.

There is even a search field right in the options menu, go to that field, type "dele" and it would already show the relevant option. It doesn't matter in this case if that specific sub-menu would normally show 10, 100 or 1000000 options.

Though for this issue the option would indeed be too much, as this can already be resolved with another option - the shortcut. Having two options that do the same thing is undoubtedly bloat.

Having too many settings hurts discoverability and makes users think Godot is "too complex".

If anyone decides not to use any software after looking at the (low-impact, typical "advanced options") options and seeing a large list of options and default values, then I would certainly say: Good riddance!
Someone who is scared away by a list of 10+ options in a menu named "Editor Options" is probably the last person on Earth I'd want to cater to when I'd develop a complex program like a game engine editor. Seriously, what kind of low impatience threshold would that even be?
And I doubt that has ever happened, or that it will ever happen.

If anyone finds Godot too complex, it will be someone entirely new to software/game development, and it will be about the "main" editor interface, not some "other" kind of options menu.
All of that said, the Editor Options menu really is pretty messy (everything is kinda thrown in there and mixed together), looks a bit unprofessional and would be nicer to look at with some sub-sections.

Anyway, if something like an "advanced options" checkbox in the options would please everyone, I'm also for it. It's the first thing I always click when using any software that has it anyway.

@TheCire
Copy link

TheCire commented Jul 26, 2019

I hope im not reviving something in a bad way but...

why not have the mention of "Delete node without confirm" in the dialog box itself?
using wording something like this:

Do you really want to delete the selected node(s)?
(Press Shift-Del to avoid this confirmation dialog)

@KoBeWi
Copy link
Member Author

KoBeWi commented Jul 27, 2019

Though for this issue the option would indeed be too much, as this can already be resolved with another option - the shortcut. Having two options that do the same thing is undoubtedly bloat.

The idea is that if this option is introduced, the "no-confirm" shortcut will be removed.

Anyways, other than removing the confirm (which this PR does right now), here are other options:

  • Tooltip
    image
    (not sure if that's noticeable enough)

  • Confirm dialog hint
    image
    (doesn't look like something normally used)

  • Editor Setting with checkbox in the dialog:
    image
    (ignore the ugly mockup)

IMO the third one is the best, if we are going to keep it optional. It's discoverable and convenient. As for settings bloat, I agree that "Advanced Settings" option would be enough (though some cleanup could still be done so the settings are better organized). Avoiding new settings at all costs isn't always good, if at all.

@Bloodyaugust
Copy link

Stumbled across this while browsing issues, thought I'd drop by and give the perspective of a brand new Godot user (but experienced software engineer, so with a grain of salt or two):

I would have already accidentally deleted a few nodes while working with the Tileset editor, thinking I was deleting a selection there. Considering the scope of my project was rather small, it probably would've been immediately obvious that I deleted something I didn't intend to, but that's probably not always the case. I think it's a matter of familiarity with the editor; experienced users know the shortcomings and more about the configurations/shortcuts, while less experienced ones should probably experience a "safe by default" flow that saves headache while learning.

I'm not going to comment on "how many options is too many", but I would like to say that as I continue to explore the editor, the only thing that has prevented me from making use of configurability is a lack of explanation for options.

@girng
Copy link

girng commented Jul 30, 2019

You are deleting something! Something that is.. very important. Yes you can do ctrl+z, but it's part of the UX and value of the node(s). Confirmation dialogs appearing when deleting something important is not a bad thing. Only time it's a bad thing, is if it negatively affects your performance. I've never heard of this being an issue? 😕

Also, if you are highlighting a bunch of nodes, and in the moment are second guessing if you want to delete them or not, that second guess could be saved by that confirmation dialog. (part of the user experience). I like that.

@girng
Copy link

girng commented Jul 30, 2019

See, that's the problem here.
You take your own opinion to determine what others might find useful or not. Just because you don't understand something, or don't find it useful, doesn't mean it's the truth or anyone has to agree with you.

The thing is, groud has done an immense amount of work and tackles a lot of bugs for Godot's UI. I feel more than comfortable with him having the final say on UI stuff.

@TheSHEEEP
Copy link

Also, if you are highlighting a bunch of nodes, and in the moment are second guessing if you want to delete them or not, that second guess could be saved by that confirmation dialog. (part of the user experience). I like that.

When I work with an editor often and am comfortable with it, I'm no longer in the habit of second guessing myself. That really is over at some point, at least for me.
Anyway, since I have now internalized that I can just rebind deletion without confirmation to Del, all is good at least as far as I'm concerned.

@girng
Copy link

girng commented Jul 30, 2019

When I work with an editor often and am comfortable with it, I'm no longer in the habit of second guessing myself. That really is over at some point, at least for me.

Yeah, that's true. I think shift+delete for people who are versed in the Editor is a good trade off.

@golddotasksquestions
Copy link

golddotasksquestions commented Sep 11, 2019

Unfortunately Ctrl+Z is no replacement for this dialog, as it is currently not intuitive at all what actions in the Godot editor are undoable and which are not. #25919
godotengine/godot-proposals#59

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 11, 2019

Unfortunately Ctrl+Z is no replacement for this dialog, as it is currently not intuitive at all what actions in the Godot editor are undoable and which are not.

Node deletion, which is what this dialog is about, is always undoable.

btw, personally I don't need this PR anymore. I just bound "Delete (no confirm)" to my Delete key and and it works perfectly. I wouldn't mind if this was the "right" way to achieve that.

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 22, 2019

As mentioned in previous comment, rebinding "Delete (no confirm)" is enough in this case. Also the confirmation got some improvements (#31942) so it's not going anywhere soon.

Closing.

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.

Option to Remove delete node confirmation