godotengine / godot-proposals

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

Allow scripts to override default property values #7593

Open Malcolmnixon opened 11 months ago

Malcolmnixon commented 11 months ago

Describe the project you are working on

Maintaining the Godot XR Tools library

Describe the problem or limitation you are having in your project

Godot XR Tools provides many types of nodes for users to create XR experiences: image

Godot XR Tools has many scenes which exist purely to provide working default values for properties provided by the C++ scene objects such as collision layers and masks. Our users are continuously trying to use the Add Child Node dialog in the scene-tree editor which results in broken objects due to these properties not being configured correctly.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

We would like a way for the scripts to override the creation values for properties in the base C++ nodes.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

A script can already change the default revert value for C++ implemented properties by implementing the _property_can_revert() and _property_get_revert() methods.

Changing the initial value after "Add New Node" could be achieved via a new notification (NOTIFICATION_CREATED) which is invoked only when a new node is created (either via Add New Node, or dynamically in code) - not when it is instantiated from a PackedScene or duplicated.

The script would then be able to contain the following code:

const DEFAULT_COLLISION_LAYER = 0x0103

# Override base properties when the node is constructed in the editor
func _notification(what : int):
    if what == NOTIFICATION_EDITOR_CREATED:
        collision_layer = DEFAULT_COLLISION_LAYER

# Report revert support for base properties
func _property_can_revert(property : String) -> bool:
    return property == "collision_layer"

# Provide report values for base properties
func _property_get_revert(property : String):
    if property == "collision_layer":
        return DEFAULT_COLLISION_LAYER

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

The current approach of providing pre-configured scenes and extensive user documentation appears to be the only workaround at this time; however it creates significant user issues.

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

No means of implementing it outside of the core has been found.

BastiaanOlij commented 11 months ago

Something I'd like to add in here is that historically we would use scenes here, when you create a scene with a certain Godot type as the root node, you can set the default values in the properties in that scene, and they would become the overriding defaults when that scene was added as a child scene. Perfect.

But ever since we've added class_name and allowed for people to add nodes based purely on the script, we've ran into the problem that it confuses users of our plugin. It's easier to press + and add a node, than to press the link button and add a child scene, but the result is a broken system. We need to define a class_name on these scripts for them to work properly with other parts of the plugin, but we're missing out of functionality when just the script is used.

Ergo, we've started to rebuild the plugin to not rely on the scenes, but have the scripts handle all logic including creating further child nodes. This is the one missing feature to make that approach complete.

edit Note that my solution to this problem would be to simply allow defining the default by either redefining the variable like so:

@export var collision_layer = 0x0103

and Godot seeing this is an existing property and simply overriding whatever is changed and leaving the rest of the property in tact, Or add a new keyword for this:

@override var collision_layer = 0x0103
dalexeev commented 11 months ago
ultrasuperpingu commented 4 months ago

As a complement, it would be nice also to be able to send this notification again on demand, because this notification could be also use to create new nodes for example.

AThousandShips commented 4 months ago

How would creating another node mean you should receive this notification? A nice is only created once

ultrasuperpingu commented 4 months ago

How would creating another node mean you should receive this notification? A nice is only created once

Not sure to understand. Sorry, my english level is probably the cause :(

My purpose is:

As an example, here is what I'd like to be able to do on a new node creation:

I hope with clarify my purpose (and I hope my purpose is not stupid :))

AThousandShips commented 4 months ago

here is what I'd like to be able to do on a new node creation

A node is only created once, you can't "recreate" it, this will be called when a node is created, once

ultrasuperpingu commented 4 months ago

A node is only created once, you can't "recreate" it, this will be called when a node is created, once

You're right, so my needs are: I'd like a setup notification that would be automaticly called on node and to be called on demand.

But clearly, the main need for me is a notification when the node is created. For the possibility to run the setup again, I can do it with other mecanism, you're right. My "As a complement" in my precedent post should be ignore in this topics.

ultrasuperpingu commented 4 months ago

Trying to be clear on this: