godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.12k stars 69 forks source link

Automate delegation of exported properties to internal nodes to improve GDScript abstractions #361

Open willnationsdev opened 4 years ago

willnationsdev commented 4 years ago

Describe the project you are working on: A quiz game.

Describe the problem or limitation you are having in your project: Players move over an Area to "select" a quiz question's answer.

The Area has its own scene with several exported properties on the root node, including...

Now, I could just instantiate multiples of this scene and then use Editable Children to override the various values, but I don't want to have to do that since I only really need very specific overrides to be delegated to child nodes in the scene. I may also eventually want to convert these scenes into standalone scripts, so the user might not even be able to use Editable Children in the future.

As of right now, I have to do stuff like this:

onready var _sprite := $MySprite as Sprite3D
export onready var sprite_region := Rect2() setget set_sprite_region
func set_sprite_region(p_value: Rect2) -> void:
    _sprite.region_rect = p_value

However, as you can tell, this involves a lot of boilerplate code when all I really want to do is export values that my node reassigns, by default, to child nodes. It would be much cleaner if I could directly define a delegation relationship for the property and avoid all the boilerplate, especially since I can easily have anywhere from 3-12 of these properties (as I've encountered this sort of problem before).

Maybe do something like this instead:

export onready var sprite_region := Rect2() in @"MySprite:region_rect"

Describe how this feature / enhancement will help you overcome this problem or limitation:

Helps clean up abstractions in GDScript and makes the code flow better. Automates boilerplate code thereby removing clutter derived from the abstractions. Also, since the NodePaths are outlined at parse-time, you can get editor warnings on nodes that have a script whose stated node dependencies don't exist.

Show a mock up screenshots/video or a flow diagram explaining how your proposal will work:

A code usage example is in the description.

Describe implementation detail for your proposal (in code), if possible:

You would need to update the gdscript parser to support the new syntax. It would need to store information about which property gets redirected to which node and how it is being exported / what type it is, etc. all inside of the GDScript object. It would also need to, in the editor context, fetch the active scene and confirm that the nodes in question exist, displaying warnings in the editor if they don't. Then you would update the GDScriptInstance's set/get/get_property_list methods to check these particular things while iterating over properties, that way it can properly fetch the dependent node.

If this enhancement will not be used often, can it be worked around with a few lines of script?:

Yes, it can be worked around with a few lines of script, but the proposal itself is about the frequency of needing to do things like this when abstracting multiple nodes under a single node and how cluttered it makes the code. The workaround is what we already have and it is cumbersome.

Is there a reason why this should be core and not an add-on in the asset library?:

It involves the GDScript parser and therefore cannot be implemented as an addon.

Zireael07 commented 4 years ago

I tend to use get_parent().whatever from the child nodes in such a case (yeah, I am doing similar things where the parent node governs/exports a ton of stuff). But the point still stands.

girng commented 4 years ago

Now, I could just instantiate multiples of this scene and then use Editable Children to override the various values, but I don't want to have to do that since I only really need very specific overrides to be delegated to child nodes in the scene. I may also eventually want to convert these scenes into standalone scripts, so the user might not even be able to use Editable Children in the future.

You can adjust the variables after the scene is instanced to any specific value you want (if you don't want to use editable children).

However, as you can tell, this involves a lot of boilerplate code when all I really want to do is export values that my node reassigns, by default, to child nodes. It would be much cleaner if I could directly define a delegation relationship for the property and avoid all the boilerplate, especially since I can easily have anywhere from 3-12 of these properties (as I've encountered this sort of problem before).

To me, that doesn't look like boilerplate. Looks like concise gdscript being used for your exact use-case

willnationsdev commented 4 years ago

You can adjust the variables after the scene is instanced to any specific value you want

Only if I export the property, cache the target node, and assign the value in a setter, all for a single delegation of a value.

To me, that doesn't look like boilerplate. Looks like concise gdscript being used for your exact use-case

It may be concise compared to other languages, but that's only because of GDScript's syntax already being concise overall. The number of things you have to do (4 different tasks) just to complete the single task of forwarding the property is verbose. And since this is something I do very often, with many properties (N) on many scenes/nodes (M), having to deal with 4 N M collections of truly boilerplate code is tiresome. Especially when improving it is entirely plausible and practical.