Godot: "Clear Script" doesn't show up anymore

Created on 30 Jul 2018 · 36Comments · Source: godotengine/godot

Godot version:
5a5614e

Issue description:
When selecting a node that has a script the "Attach Script" icon switched to "Clear Script", now it doesn't:
screenshot at 2018-07-30 00-05-21
How it's supposed to look:
screenshot at 2018-07-30 00-07-25

discussion enhancement editor usability

Source

YeldhamDev

👍5

Most helpful comment

I prefer the old behavior too. Inheriting a script is not common at all. The add script button here to "create a script that inherits the previous one and then replace it" has quite an unexpected behavior.

I would probably make this a RMB menu entry and either remove the button when the node has a script or bring back the delete script button.

groud on 19 Oct 2018

👍2

All 36 comments

Byproduct of #20233, now the add-script button would automatically populate the "extends" field to the path of the script.

bojidar-bg on 30 Jul 2018

CC @willnationsdev

akien-mga on 13 Aug 2018

Yes, I specifically brought this up with reduz, and he said that he would rather the clear script button go away entirely instead of showing both it AND the add script button. You still have access to clearing a script from the right-click context menu and from the Inspector's "script" property and it was an infrequent-enough operation that he thought that would be acceptable.

I guess the question remains: is there enough public outcry over this change that people want to show both of the icons at once?

willnationsdev on 13 Aug 2018

Why must the "Add script" be shown when there's already a script though?

akien-mga on 13 Aug 2018

Because clicking the add script button on a node that already has a script now auto-fills the inherits field of the ScriptCreateDialog, for easier creation of derived scripts. This is especially useful for deriving from script classes, which can be inherited from by name.

willnationsdev on 13 Aug 2018

So if a node has script A attached, you click the "Add script" again, it will propose to create a new script B to replace the attached A, which inherits from A?

That seems a bit weird usability wise to me, I'm not sure it's a good design. At least it's not the workflow I would expect when I want to attach a script to a node that should inherit a base script.

akien-mga on 13 Aug 2018

That's the functionality I would have expected. Would you expect there to be a separate button created with a new icon, that conditionally shows up instead of the add script button, but does the same thing? So basically just change the icon?

willnationsdev on 13 Aug 2018

I don't know, I just came back from holidays and maybe need to test the feature before criticizing ;)

As is I don't really see why the "common" use case for scripts inheriting another script would be to have the base script assigned first, and then replace that, instead of adding the new script directly without having to assign this base script before.

I understand the potential workflow if you have e.g. a base enemy.tscn with enemy.gd, and you create an inherited scene bat.tscn where you want to attach bat.gd that inherits enemy.gd - but is that really the main workflow for this feature?

akien-mga on 13 Aug 2018

@akien-mga

As is I don't really see why the "common" use case for scripts inheriting another script would be to have the base script assigned first, and then replace that, instead of adding the new script directly without having to assign this base script before.

It isn't meant to replace that workflow. It's about adding another workflow that just didn't exist before. If you have a script on a node already, and you want to extend it with another script, you can still do it the same way you did it before, but you have to add an extra step to clear the script beforehand which is cumbersome since you then have to go back and find the exact path of the old script (which could be buried deeply in a plugin - although script classes help with this).

Remove the existing script in order to make the add-script button appear on the node.
Create a script on the intended node.
Either in the create-script dialog or on the created script itself, change the inherits value to be the base script path/name.

Now, this common, but more complex pattern can be turned into...

Click the add-script button on a node with a script, with the inherited script already filled in because it exists, and create the new derived script already on the node.

It doesn't have any impact on the no-script case, so it's a huge workflow improvement for that case. However, I can definitely understand if people want to bring back the clear script button as well. People are used to it, and it may actually be more useful for beginner users who might be confused about a clear script button not existing.

Edit:

This also makes using plugins MUCH more intuitive when combined with script classes. I "add a node" to my scene (opens dialog), click on a script class defined by a plugin's script, and then immediately click on the "add script" button to derive it and create my own unique behavior, exactly as I would do with an in-engine class.

willnationsdev on 13 Aug 2018

Right, I'm starting to understand the workflow, especially when it comes to "custom nodes" where you'd add them to the scene tree with an existing "base" script that you'd want to inherit. Given this, I think it's fine to lose the "clear script" button, which was likely not that useful (convenient for testing stuff when debugging the engine, but maybe not so much when actually developing games).

akien-mga on 13 Aug 2018

As such it's not a bug as it's the expected behaviour. Now the question remains on whether the usability is "good enough" or if we want the clear script behaviour back. I see two options:

Reuse the "add script" button for this new "inherit existing script" feature, getting rid of the "clear script" button. This is what was proposed by @reduz and is implemented, and after thinking about it I think it may be good enough. The only question is whether it would be worth it to make the "inherit existing script" behaviour clearer to the user (e.g. with by changing the icon to one which should it would be inheriting the script instead of adding a new one).
Keep the "add script"/"clear script" toggle, and add an additional button when selecting a node with an existing script with this "inherit script" icon.

I'd tend towards keeping what we have for now and see what other feedback we get from users, and we can reevaluate then.

akien-mga on 13 Aug 2018

👍2

I think the icon and tooltip should change when in "inherit" mode.

Also, when I try to use it, it just says that the inherited parent name/path is invalid.

YeldhamDev on 13 Aug 2018

Oh, well that is definitely a bug. XD

willnationsdev on 14 Aug 2018

Inherit script icon could use something like this instead of a +
images

PLyczkowski on 11 Sep 2018

👍2

Yeah, I know akien proposed changing the icon when deriving. I'm not sure who "approves" visual designs like that though.

willnationsdev on 11 Sep 2018

CC @djrm

bojidar-bg on 12 Sep 2018

IMO it had more intuitive behavior in 3.0.6 than now in 3.1...

Chaosus on 13 Sep 2018

👍2

@Chaosus Yeah, I would've just kept them both personally, but it's not my call.

willnationsdev on 13 Sep 2018

I prefer the old behavior (and I remember cherry-picking this feature for the 2.1 branch), but the new workflow also looks interesting so I would opt for the second option:

Keep the "add script"/"clear script" toggle, and add an additional button when selecting a node with an existing script with this "inherit script" icon.

williamd1k0 on 13 Sep 2018

👍1

As such it's not a bug as it's the expected behaviour. Now the question remains on whether the usability is "good enough" or if we want the clear script behaviour back. I see two options:

Reuse the "add script" button for this new "inherit existing script" feature, getting rid of the "clear script" button. This is what was proposed by @reduz and is implemented, and after thinking about it I think it may be good enough. The only question is whether it would be worth it to make the "inherit existing script" behaviour clearer to the user (e.g. with by changing the icon to one which should it would be inheriting the script instead of adding a new one).

Keep the "add script"/"clear script" toggle, and add an additional button when selecting a node with an existing script with this "inherit script" icon.

I'd tend towards keeping what we have for now and see what other feedback we get from users, and we can reevaluate then.

whatever the new feature is. A new 'corner icon' should be design for that new feature, instead of "green plus sign" which already has other meaning. I think assigning a new behavior to a existed button without any appearance change is puzzling. A same 'file icon' with a different 'corner icon' will be much better than current behavior of Godot 3.1 alpha. The GUI explains itself.

JustusPan on 28 Sep 2018

@JustusPan I agree. I'm not sure what the workflow is to add new icons to the editor though. Has something to do with creating an SVG, optimizing it with a utility, and then adding it to /editor/icons. But then there's also the godot-design repository and idk how that's involved.

willnationsdev on 28 Sep 2018

@akien-mga Speaking of which, @PLyczkowski created an icon and forwarded it to me. I have added an optimized copy of the .svg file to a godot-design branch of mine, but I'm not sure how I'm supposed to integrate that with the editor. Am I supposed to just submit a separate PR to update the /editor/icons directory with the same .svg file?

willnationsdev on 16 Oct 2018

I would probably make this a RMB menu entry and either remove the button when the node has a script or bring back the delete script button.

groud on 19 Oct 2018

👍2

Perhaps some people use it more than others then, cause I know it's an important feature for my use-cases. But a lot of people have been confused by the change, so either having both buttons or switching to a RMB option sounds pretty good.