godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.22k stars 20.22k forks source link

Ability to encapsulate tool subscenes (New ways of using tool keyword) #8735

Closed kubecz3k closed 4 years ago

kubecz3k commented 7 years ago

Issue description: Quite often I have tool scene which is working inside editor. Sometimes I need for those scenes to be encapsulated inside other bigger scenes. To make them work in reaction to level designer actions I need to heavily modify the bigger scene, and this usually a lot of work and sometimes it's not even possible (when scenes are using/based on many other scenes/scripts which should not work in tool mode).

What I would want to have would be some kind of ability to notify child tool subscene about exported variable change without making owner scene a tool, maybe some kind of 'tool redirect' keyword for exported variables? Maybe even better would be to have more general ability of using 'tool' keyword not for whole scripts, but only for single members/functions instead.

Though for a second I will be able to overpass this issue with multi-script (different script for normal usage and different script for working inside the editor), but as many people pointed out there is little benefit from introducing multiscript and a lot of problems.

Zireael07 commented 7 years ago

I have a ton of tool scripts and child scenes and this would be an awesome quality of life improvement.

kubecz3k commented 7 years ago

I think it would be the best if we could use Curly braces with tool keyword to determine what parts of the script should be exacutable inside editor, example code could look like this:


tool{
export (NodePath) var sourceStateID setget setSourceStateID;
export (NodePath) var targetStateID;
}

onready var fsm = Global.findParentNodeWithType(self, FSM)

tool{
func _ready():
    setSourceStateID(sourceStateID);
        }
    fsm.init();

tool{
func setSourceStateID(inStateId):
    sourceStateID = inStateId;
    if(!is_inside_tree()): return;
}

(... rest of the class here...)
kubecz3k commented 7 years ago

I'm wondering if it would be possible to write editor plugin that would do just that. Let's assume for example that tool{ and } must be preceded by #(comment). Now I should be able to write tool plugin inside which you could select your 'tool' scripts, and this plugin should be able to load those scripts inside editor, remove all the parts that are not between 'tool{' and '}' and attach this new script (with set_script() (without saving it as a file) to the node).

kubecz3k commented 7 years ago

But I think the downside of this solution would be the fact that you would see just the tool part after opening the script in Godot :(

hubbyist commented 7 years ago

Magic code modification will just make things complicated I think. What about just, marking lines about tooling with a pipe | symbol and make parser treat them depending on a sort of environment parameter.

|export (NodePath) var sourceStateID setget setSourceStateID;
|export (NodePath) var targetStateID;

onready var fsm = Global.findParentNodeWithType(self, FSM)

func _ready():
|   setSourceStateID(sourceStateID);
    fsm.init();

|func setSourceStateID(inStateId):
|   sourceStateID = inStateId;
|   if(!is_inside_tree()): return;

(... rest of the class here...)
Zireael07 commented 7 years ago

A pipe symbol will only lead to unending questions 'what's that symbol do?'

hubbyist commented 7 years ago

But it seems more pythonic than curly braces (IMHO) and shorter than marking each line "tool". At first encounter this may seem different but when meaning is known this will be most easy on the eye and parser I think. Character for marking lines may be different. ^ can be used instead which have less usage than pipe while coding.

^export (NodePath) var sourceStateID setget setSourceStateID;
^export (NodePath) var targetStateID;

onready var fsm = Global.findParentNodeWithType(self, FSM)

func _ready():
^    setSourceStateID(sourceStateID);
    fsm.init();

|func setSourceStateID(inStateId):
^    sourceStateID = inStateId;
^    if(!is_inside_tree()): return;

(... rest of the class here...)
hubbyist commented 7 years ago

Only down side is this causes alignment for tool lines will be one character left of regular lines. This makes curly braces more acceptable and easy on the eye.

Listwon commented 7 years ago

How about:

tool:
    export (NodePath) var sourceStateID setget setSourceStateID;
    export (NodePath) var targetStateID;

or Java/Python-like annotations?

@tool 
    export (NodePath) var sourceStateID setget setSourceStateID;
    export (NodePath) var targetStateID;

@tool
func foo():
    pass

func bar():
    @tool
        setSourceStateID(sourceStateID);
hubbyist commented 7 years ago

Another syntax without additional indentation might be :

@tool@
export ....
@@
export ....

@tool@
func foo():
     pass
@@

or

%tool%
export ....
export ....
%%
bojidar-bg commented 7 years ago

Wouldn't having something like this be cleaner and more maintainable? :

element.gd:

tool "element_tool.gd"
# This script isn't tool by itself, but is replaced by the other in-editor
extends Node2D

export(int, "Fire", "Water", "Ice") var element_type = 0

func get_damage():
  return ConfigAutoload.elements[type].damage

element_tool.gd:

tool # Might be unnesesary, discuss
extends "element.gd"
# extends is not required, but better if you do this so you would get all the exported vars

# Discuss: do we really need to extend the other script for that?
# We might handle it like placeholder scripts, exporting/autocompleting the same, but
# having different in-editor logic.

export(bool) var debug = true setget set_debug

func _ready():
  set_debug(debug)

func _draw():
  if debug:
    pass # Draw circles, whatever..

# Needed since we don't have a cool way to observe property changes yet
func set_debug(new_debug):
  debug = new_debug
  update()
kubecz3k commented 7 years ago

@bojidar-bg interesting concept, I quite like it, like I said in first post I had hope it would be possible to use multi-script to achieve something similar. But I'm not sure how you want to handle exported variables in this scenario. Mean if for the purpose of editor we replace script with another one we will have in editor exported variables of another one

bojidar-bg commented 7 years ago

@kubecz3k That's what I mentioned in the # Discussion comment -- we are already replacing the non-tool scripts with the so-called "placeholder" scripts in the editor.

If we can get the tool GDScript instance to know that it should also export some variables of another script, it would have no problem doing so. Since we can set that other script at initialisation time, it would all work "just fine".

Thus, the second script would become:

tool # Might be unnecessary
extends Node2D
# ... ...
Zireael07 commented 7 years ago

I'm a bit confused by the two scripts idea...

kubecz3k commented 7 years ago

@bojidar-bg another thing to consider: I'm not sure how tool script would work when it's extending original script. Mean some part of the reason why we want to distinguish between tool and non tool part is because currently tool scripts sometimes cant even pass through the code validation inside the editor (like singletons are invisible in tool for example).
Besides that I think your proposition is good especially since you are the one of the few people who actually know how this stuff is working, and how to implement this in the most straightforward way.
When it comes to exposing exported vars of runtime script I think any solution would be fine for me personally, even if there would be some functions that we would need to use manually.

if we will have tool scripts to replace original one in editor, there should be ability to not extend original one. What I mean is currently it's hard to use tools also because some of Godot features are not present

bojidar-bg commented 7 years ago

@kubecz3k Well, you would be free to not extend the other script at all, yes.

@Zireael07 Why? You can just think of it as swapping the script at editor time, with some nifty details about exported vars.

kubecz3k commented 7 years ago

@bojidar-bg also I have yet another use case. In fsm plugin which I made, there is Transition class which is working in tool mode. It need to work in tool mode because visualisation part of the plugin need to know to which node states this transition is linked to (implemented by exported nodePath). The problem is there are generated classes which are extending this Transition class (users are doing this via UI), and those classes also need to be in tool mode because of the fact that their ancestor is sharing some data with plugin (this is a problem since if user make syntax error in this script, it will be gone for editor plugin until fix). Users are using this subclass because they need to implement some functions. conclusion: In ideal world ancestor could be a tool but subclass would not need to be tool in order for this ancestor to work in editor.

kubecz3k commented 7 years ago

@reduz could you take look at this issue? Specifically at @bojidar-bg proposition? Having this would be huge quality of life improvement and would be cool to have it in 3.0. Not sure if I'm right man to do the job but I think I might want to give it a try.

hubbyist commented 6 years ago

tool # Might be unnesesary, discuss

For the discussion part, I think marking tool scripts in a clear way is a handy requirement. So that they can be searched and processed in batch when necessary both by Godot and outside processes. Using a regex from terminal to find all tool scripts in a project folder may be handy.

buckle2000 commented 6 years ago

Use tool scriptname as import is awesome.

akien-mga commented 4 years ago

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!