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

Inherit Both Scene and Script Option #17368

Closed
11clock opened this issue Mar 8, 2018 · 10 comments
Closed

Inherit Both Scene and Script Option #17368

11clock opened this issue Mar 8, 2018 · 10 comments

Comments

@11clock
Copy link

11clock commented Mar 8, 2018

I think that creating an inherited scene should include the option to automatically create a script that inherits the original scene’s root script. This would remove the step that involves clearing the new inherited scene’s root script and attaching a new script that inherits the other, as this would be a part of creating an inherited scene.

@PLyczkowski
Copy link

PLyczkowski commented Oct 5, 2018

I think this is a very good suggestion, and I suspect this would limit the 'multiscript'/'built-in script/linking scenes with classes' issues that often pop up (#8502 #21187 #17418 #10951 #17093 #6067).

I think the cause of these kind of problems is the inability to easily 'derive things from base things' - in Godot's terms meaning inherit both the scene contents and the scene script. Think creating a unique_effect that will have it's script built in (because will appear in only one place) and that is derivative of base effect.gd.

This can be done in Godot of course, but it's a bit non-intuitive and cumbersome and doesn't really feel like straight up deriving: one has to first add a base_effect subscene (or create an inherited scene), then remove the subscene's script, then create a new script, in the Inherits field click the folder icon, and then navigate to the base_effect's script (or write extends "res... by hand).

Adding one click deriving would alleviate those issues in some part I think. The workflow would look like this:

  • Press +
  • Choose Effect
  • Instead of Create Instance, choose Create Inherited
  • When the script effect is not saved to disk but built-in, a window popup appears that it can't be done unless you save script to disk, and offers a one-click option to do so
  • A node is added to the scene tree with the base type effect.gd has, with a built-in script "extends "res://effect.gd""

This can also be combined with #22181. The workflow would then look like this:

  • Press +
  • Choose BaseEnemy (base_enemy.tscn)
  • Instead of Create Instance, choose Create Inherited
  • When the script effect is not saved to disk but built-in, a window popup appears that it can't be done unless you save script to disk, and offers a one-click option to do so
  • A scene is added to the scene tree, with a built-in script "extends "res://base_enemy.gd""

If you don't want to add Inherited, but convert an existing instance to inherited:

  • RMB on node in scene tree that has script A attached
  • Choose Convert to Inherited
  • When the script effect is not saved to disk but built-in, a window popup appears that it can't be done unless you save script to disk, and offers a one-click option to do so
  • The script is converted to a new built-in script that extends script A
    (Similar functionality can be found in this add-on https://github.com/kubecz3k/KivanoEditorEnhancemens )

@willnationsdev
Copy link
Contributor

willnationsdev commented Oct 5, 2018

11clock: This would remove the step that involves clearing the new inherited scene’s root script and attaching a new script that inherits the other, as this would be a part of creating an inherited scene.

PLyczkowski: This can be done in Godot of course, but it's a bit non-intuitive and cumbersome and doesn't really feel like straight up deriving: one has to first add a base_effect subscene (or create an inherited scene), then remove the subscene's script, then create a new script, in the Inherits field click the folder icon, and then navigate to the base_effect's script (or write extends "res... by hand).

All of these features, are more or less already available via #22181. It's already impossible to extend from a built-in script sub-resource within a scene, so IF the root node of your scene had a built-in script, you'd have to save the script to a file first.

The recent script class features made changes to the editor such that the add-script button DOESN'T go away when you click on a node with a script. Instead, clicking the button will open the ScriptCreateDialog as before, but the dialog will automatically be deriving the existing script (either by path or, if it's a script class, by name).

What this means for the scene templates is...

  1. If you create a scene with a non-built-in script...
  2. and that scene is registered as a scene template...
  3. and if you then instance that scene template in your scene, the scene's root node will have that script.
  4. If you then want to DERIVE that script, you CAN by just clicking the instanced scene and clicking the add script button.
  5. The base script's path or name will be listed in the Inherits field. Everything will work as expected.

@PLyczkowski
Copy link

1. If you create a scene with a non-built-in script...

2. and that scene is registered as a scene template...
   3, and if you then instance that scene template in your scene, the scene's root node will have that script.

3. If you then want to DERIVE that script, you CAN by just clicking the instanced scene and clicking the add script button.

4. The base script's path or name will be listed in the Inherits field. Everything will work as expected.

That's sounds good enough, can't wait for merge then!

I guess the only thing that's left is a different icon for the add script button when there is a script present already?

@willnationsdev
Copy link
Contributor

willnationsdev commented Oct 5, 2018

@PLyczkowski Yeah, that's what people have requested. I'm not sure who actually does all that though (I'm no graphic artist really).

@PLyczkowski
Copy link

I am, in case one is needed for this.

@willnationsdev
Copy link
Contributor

willnationsdev commented Oct 5, 2018

@PLyczkowski Yeah, if you wanna figure out what the right workflow for that is and add an icon, go right ahead! I know it has to do with using the godot-design repository, making a .svg, optimizing it with a script they have, and then merging it into the repository. I created an icon there, but I'm not sure how that all fits into the actual editor. It's all very confusing.

You'd have to update editor to change the icon in response to the node having a script already or not though. Either that, or refactor it so that there are multiple buttons entirely and re-hook up the signal connection for the new button.

@PLyczkowski
Copy link

Here's mine:
shot_181006_010727

@willnationsdev
Copy link
Contributor

That looks good to me! :-)

@PLyczkowski
Copy link

@Calinou
Copy link
Member

Calinou commented May 26, 2020

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!

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

5 participants