godotengine / godot-proposals

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

Adding @export_if(condition) to streamline conditioned properties exposing in inspector #9842

Open EviTRea opened 2 months ago

EviTRea commented 2 months ago

Describe the project you are working on

A RPG where entity has states that determines if some properties are required or not.

Describe the problem or limitation you are having in your project

I have a shop script, where the seller can either have one price for everything, or everything has its own price and currency. I wrote a bool use_unified_cost, and I have 2 sets of @export variables to account for each kind of seller; Considering only one property set would be useful for a time, I want the editor to only show some of them depending on the state of use_unified_cost. (You can ignore all these and just think about motion_mode in CharacterBody2D, the engine itself already showcased the use case.) notify_property_list_changed() and _get_property_list() method in Object documentation works, but it's requiring a lot of detail information about how to represent the property in the editor. I'm still stuck on it filling null into my custom TradeOptionCost resource array, which would work if I simply use @export. It's also currently only adding the properties to the button of the export list, which I believe is solvable, but...I just don't see why it should require these low level knowledge for the function.

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

Add a @export_if(condition) annotation, which only expose itself in inspector if condition is true.

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

Use the example from the official doc:

@export var holding_hammer := false

enum HammerType{WOODEN, IRON, GOLDEN, ENCHANTED}
@export_if(holding_hammer) var hammer_type: HammerType

(@tool and notify_property_list_changed() might still be required, but not using _get_property_list() saves many lines and effort.)

The condition can also be (A and B) or (state == State.A), so there will probably be very complex use case.

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

Somewhat, it would at least require calling a function and construct something and return, and one property would be appending a new dictionary. My current code look like this:

func _get_property_list():
    var properties := []
        ## Export custom resource doesn't work and I haven't figured out why.
    properties.append({
        "name": "trade_costs",
        "type": TYPE_ARRAY,
        "usage": PROPERTY_USAGE_DEFAULT if !use_unified_cost else PROPERTY_USAGE_NO_EDITOR,
        "hint": PROPERTY_HINT_RESOURCE_TYPE,
    })
    properties.append({
        "name": "default_cost",
        "type": TYPE_INT,
        "usage": PROPERTY_USAGE_DEFAULT if use_unified_cost else PROPERTY_USAGE_NO_EDITOR,
    })
    properties.append({
        "name": "cost_add_per_use",
        "type": TYPE_INT,
        "usage": PROPERTY_USAGE_DEFAULT if use_unified_cost else PROPERTY_USAGE_NO_EDITOR,
    })
    properties.append({
        "name": "currency",
        "type": TYPE_INT,
        "usage": PROPERTY_USAGE_DEFAULT if use_unified_cost else PROPERTY_USAGE_NO_EDITOR,
    })
    return properties

Would be 4 lines if it's implemented.

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

It's a core part of the GDScript.

dalexeev commented 2 months ago

Note that annotation arguments must be constant expressions. Annotations are resolved at compile time and are the same for all instances of the class. So the condition cannot be arbitrary and cannot be written as you would expect. Only something like @export_if("holding_hammer").

Also, you can use _validate_property() with @tool.

Related:

EviTRea commented 2 months ago

Note that annotation arguments must be constant expressions. Annotations are resolved at compile time and are the same for all instances of the class. So the condition cannot be arbitrary and cannot be written as you would expect. Only something like @export_if("holding_hammer").

Also, you can use _validate_property() with @tool.

Related:

* [Ability to dynamically hide exported variables in the inspector #1056](https://github.com/godotengine/godot-proposals/issues/1056)

* [Allow conditional exports (or sub-exports) #2582](https://github.com/godotengine/godot-proposals/issues/2582)

* [Add `@export_toggle` to GDScript #8717](https://github.com/godotengine/godot-proposals/issues/8717)

I standby that there should be an easierl way to do this but _validate_property() is nicer indeed and will do for me for now, thanks Not sure if I should close this though, doesn't seem to be duplicate, guess I'll just leave it here