godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Implement an "add_user_property" system for GDScript #8996

Open ZenithStar opened 9 months ago

ZenithStar commented 9 months ago

Describe the project you are working on

I'm working on two main games at the moment, with a few potential side projects that all use similar systems. Of the main projects, one is turn-based tactics and one is real-time adventure, but they share many overlapping features (both 3D with the same art style animation and many of the same control schemes). Therefore, I am trying to write my game's logic as modular and reusable as possible. In the future if there is enough interest, I would also like to release parts of my framework as addons.

Describe the problem or limitation you are having in your project

GDScript describes itself as a dynamic language that leans towards a duck-typing philosophy. For implementing functionality following this philosophy, I have found Object.add_user_signal to be an incredibly powerful tool. This is perfect for dispatching one-shot events, but isn't sufficient when state is involved. One way to handle this would be to trigger user_property_updated signals each time state is updated and have each listener component maintain its own memory of the state. However, what about the case that a new component attaches itself to the entity and needs to know the last known state for this user property? The last known state still needs to be recorded somewhere. In dynamic languages like python and JavaScript, you can easily achieve this by dynamically creating new properties attached to the instance of a object, rather than the class itself.

>>> class Thing: pass
>>> instance_of_thing = Thing()
>>> instance_of_thing.foo = "bar"
>>> instance_of_thing.foo
bar

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

Currently, I've implemented this feature in my own project as a child component Node that needs to be added to any entity parent that needs this functionality.

class_name UserProperties extends Node

var _user_properties: Dictionary = {}

func _get_property_list() -> Array[Dictionary]:
    return _user_properties.values()
func _get(property):
    return _user_properties[property]["getter"].call(property)
func _set(property, value):
    return _user_properties[property]["setter"].call(property, value)

func _default_get(property):
    return _user_properties[property].value
func _default_set(property, value):
    if _user_properties[property].value != value:
        _user_properties[property].value = value
        emit_signal(_user_properties[property].changed)

func add_user_property(_name: String, type: Variant.Type, default_value = null,
            getter: Callable = _default_get, setter: Callable = _default_set,
            usage: PropertyUsageFlags = PROPERTY_USAGE_NONE, hint: PropertyHint = PROPERTY_HINT_NONE,
            hint_string: String = ""):
    if _user_properties.has(_name):
        push_error("%s UserProperties: unable to add duplicate property %s"%[get_parent().name, _name])
        return
    var changed_signal:String = "%s_changed"%[_name]
    _user_properties[_name] = {
        "name": _name,
        "type": type,
        "usage": usage,
        "hint": hint,
        "hint_string": hint_string,
        "value": default_value,
        "changed": changed_signal,
        "getter": getter,
        "setter": setter
    }
    add_user_signal(changed_signal)
    notify_property_list_changed()

func get_user_property_definition(_name: String):
    return _user_properties.get(_name)

func has_property(property):
    return _user_properties.has(property)

func changed_signal(property) -> String:
    return _user_properties[property].changed

func connect_changed_signal(property, callable:Callable):
    connect(_user_properties[property].changed, callable)

func delete_property(_name: String):
    _user_properties.erase(_name)
    notify_property_list_changed()

To use this functionality, the very boilerplate syntax looks like:

var target_property = target.find_child("UserProperties")
if not target_property.has_property("heading"):
    target_property.add_user_property("heading", TYPE_VECTOR3, Vector3.ZERO)
target_property.set("heading", new_heading)

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

Here's a few idealized implementations in order of increasing boilerplate

var node = Node.new()
print("custom_property" in node)  # Prints false
node.custom_property = 10
print("custom_property" in node)  # Prints true
print(node.custom_property)       # Prints 10
var node = Node.new()
print("custom_property" in node)  # Prints false
node.add_user_property( "custom_property", TYPE_INT )
print("custom_property" in node)  # Prints true
node.custom_property = 10
print(node.custom_property)       # Prints 10
var node = Node.new()
print(node.has_user_property( "custom_property" ))   # Prints false
node.add_user_property( "custom_property", TYPE_INT )
print(node.has_user_property( "custom_property" ))   # Prints true
node.set_user_property( "custom_property", 10 )
print(node.get_user_property( "custom_property" ))   # Prints 10

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

For more complex projects, I believe this function will see frequent usage. I've posted my current workaround above, and I believe it exceeds "a few lines of script".

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

This requires adding functionality to the base Object class.

theraot commented 9 months ago

I think this could be built on top of metadata (get_meta, get_meta_list, has_meta, remove_meta, set_meta), you would to specify the type, have an updated callback, and syntactic sugar.

ZenithStar commented 9 months ago

I think this could be built on top of metadata (get_meta, get_meta_list, has_meta, remove_meta, set_meta), you would to specify the type, have an updated callback, and syntactic sugar.

Yes. Someone else also informed me that I should be using metadata for this purpose. I had glossed over it before because I was under the impression it was editor only, but I had misread that. I refactored some of my project using metadata and this weakly typed Property wrapper, and it turned out quite a bit cleaner.

class_name Property extends RefCounted

signal changed
var _value
var value:
    get:
        return _getter.call()
    set(value):
        return _setter.call(value)

var _getter: Callable
var _setter: Callable

func _default_get():
    return _value
func _default_set(new_value):
    if _value != new_value:
        _value = new_value
        changed.emit()

func _init(fget=_default_get, fset=_default_set):
    _getter = fget
    _setter = fset

An example from the usage side:

if not get_parent().has_meta("focus_target"):
    get_parent().set_meta("focus_target", Property.new())
_target_property = get_parent().get_meta("focus_target")
_target_property.changed.connect(_on_target_changed)
_target_property.value = anything

This honestly isn't much worse than my worst case boilerplate that I wrote in my first post and comes with the advantages of being a Variant container, making it nullable https://github.com/godotengine/godot-proposals/issues/162 and able to directly use a Node reference, rather than a NodePath, since Variant.Type only includes NodePath.