Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve access to variables in set / get properties GDScript 2.0 #3096

Open
t3nk3y opened this issue Aug 7, 2021 · 4 comments · May be fixed by godotengine/godot#78991
Open

Improve access to variables in set / get properties GDScript 2.0 #3096

t3nk3y opened this issue Aug 7, 2021 · 4 comments · May be fixed by godotengine/godot#78991

Comments

@t3nk3y
Copy link

t3nk3y commented Aug 7, 2021

Describe the project you are working on

I'm building an out of game, and in game room editor. It has ships(like dungeons), decks (like floors), and rooms. I want to be able to rapidly develop different ships, decks, and rooms, in the Godot Editor, these act as base designs/templates that can be edited, in game.

Within the room and deck layers I have GridMap based classes that do things like auto-tiling walls, and setting spaces for rooms to go in. I've set this up so that in the editor, I can make a ship, add a deck to the ship, and a room to the deck. Rooms have a set of export vars that determine how their walls and floors look, and work, and what the initial and current visibility states of those walls are. However, many rooms will share the same settings, especially at the deck level(an engineering deck might have one look, while a bridge deck might have another). And decks, likewise have the same variables as their rooms, as well as some other variables of their own. These act as default values that the rooms can override, but that will initially not be set. I plan to do the same with ships, providing default ship level variables that can be set at design time.

Describe the problem or limitation you are having in your project

Whenever I set an exported variable on my room, the GridMap needs updated to display differently based on that new value. This means I need to call an update function anytime the variable is set by the editor. However, since I want to dynamically adjust these from the editor, at the deck(and eventually ship) level, I need to have each deck node, call an update to every room node, and each room node needs to then update itself.

In the end, I'll have at least 30 export vars at the room, deck, and ship level, each of which will basically look alike, here is an example with just 5:

@export var example_var:
	set(value):
		if is_nan(example_var) and is_nan(value):
			return
		if example_var == value:
			return
		example4_var = value
		update_child_params()
@export var example2_var:
	set(value):
		if is_nan(example2_var) and is_nan(value):
			return
		if example2_var == value:
			return
		example4_var = value
		update_child_params()
@export var example3_var:
	set(value):
		if is_nan(example3_var) and is_nan(value):
			return
		if example3_var == value:
			return
		example4_var = value
		update_child_params()
@export var example4_var:
	set(value):
		if is_nan(example4_var) and is_nan(value):
			return
		if example4_var == value:
			return
		example4_var = value
		update_child_params()
@export var example5_var:
	set(value):
		if is_nan(example5_var) and is_nan(value):
			return
		if example5_var == value:
			return
		example5_var = value
		update_child_params()

I have to manually(or regextually/macroedically) write the variable name 3 times, in what amounts to a simple 7 line function, for 30 variables. It's not just annoying, it looks terrible to me. Instead of a nice list of export vars, I've got this long list of code repeated over and over.

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

With just a little more access to the variable, itself, in the set property, setters would be so much more powerful, and allow a great deal of simplification, and reuse via functions.

Ideas for doing this(pick one):

  • Change the set property from set(value): to set(value, variable):
    Within the setter, variable would provide direct access to the variable, without re-calling the setter. Use of ,variable would be optional.
  • Make any references to the variable from functions called from the setter also have direct access to the variable.
    In this case, it's a sub-scope, and could still allow boiling the setters down to 1 or 2 lines.
  • Provide some way to set variables that skips the setter.
    Some proposals:
    • Making _variable mean to give direct access to the variable(everywhere, not just in the setter).
    • Providing an engine level dictionary that gives direct access to every variable in memory(Lua, PHP, and I think Python all do something like this)
    • Provide a special function that will set a variable without calling the setter, such as rawset(variable, value) (Lua does this)
  • Make anything that sets the variable, including the editor, call _set() so this can be handled in an override
  • Provide a super() function for the setter that would perform variable = value
    At some point in the setter, you pretty much have to do this function, anyway, unless setting it to something else, and you could just update value to something else, then call super().

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

With just a little more access to the variable, itself, in the set property, I could reduce this whole thing, potentially as small as the following:

@export var example_var1:
	set = handle_update_param
@export var example_var2:
	set = handle_update_param
@export var example_var3:
	set = handle_update_param
@export var example_var4:
	set = handle_update_param
@export var example_var5:
	set = handle_update_param

with the handle_update_param code looking like:

func handle_update_param(value, variable):
	if is_nan(variable) and is_nan(value):
		return
	if variable == value:
		return
		variable = value
	update_child_params()

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

It's fair to say that similar end goals could be achieved without any one of these enhancements. But for my scenario, even a couple of extra lines of script, turn in to 180 extra lines of very repetitive and readability impairing script. I think a dict may often be considered as a solution to something like this, but it doesn't work out here, because a major goal is to be able to dynamically view the changes that adjusting these variables make in the editor, via the inspector, and switching to a dict, breaks the ability to use all of those inspector hints, etc.

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

I think pretty much all of the solutions I propose dive in to the core of the GDScript code, and though I haven't yet taken a look at all of the core code involved in the solutions, I imagine several, including the top idea of just passing the variable to the set function, would easily be implemented in the core, do so without forcing users to adapt code to accommodate the changes, and come with fairly low risk.

@Calinou Calinou changed the title Improve access to variables in set / get properties GDScript 4.0 Improve access to variables in set / get properties GDScript 2.0 Aug 7, 2021
@dugramen
Copy link

dugramen commented Aug 9, 2021

I really like the first idea of having "variable" as a parameter, but wouldn't that mean Godot would need to add pointers?
If not, then I guess the "variable" parameter would be a string with its name instead. So you'd call set(variable, value).

@dalexeev
Copy link
Member

I really like the first idea of having "variable" as a parameter, but wouldn't that mean Godot would need to add pointers?

Fair remark. I am against pointers/references in GDScript.

If not, then I guess the "variable" parameter would be a string with its name instead. So you'd call set(variable, value).

Impossible, because set(variable, value) will result in an infinite recursive call to the setter.

In fact, avoiding code duplication here is very simple:

enum ID {
    POISON,
    CURSE,
    SHIELD,
    # ...
}

var poison: int:
    get: return get_effect(ID.POISON)
    set(value): set_effect(ID.POISON, value)

var curse: int:
    get: return get_effect(ID.CURSE)
    set(value): set_effect(ID.CURSE, value)

var shield: int:
    get: return get_effect(ID.SHIELD)
    set(value): set_effect(ID.SHIELD, value)

# ...

var _effects := {}

func get_effect(id: ID) -> int:
    return _effects.get(id, 0)

func set_effect(id: ID, value: int) -> int:
    # Some logic...
    _effects[id] = value
    # Some logic...

@cmd410
Copy link

cmd410 commented Nov 12, 2022

This is a feature I really need. My project also has a lot of repetitive setter code. I was considering that this could be solved with new gdscript Callable binding, so the hypothetical code would look like this

func set_prop(name, value):
   self.set(name, value)
   # ...do stuff...
   emit_changed()

@export var example_var1: int : set = set_prop.bind("example_var1")
@export var example_var2: int : set = set_prop.bind("example_var2")
@export var example_var3: int : set = set_prop.bind("example_var3")
# ...

Alas, now in godot 4 beta4 this does not work. The error is Expected end of statement after property declaration, found "." instead.. And also, as was pointed out above, set will result in infinite recursion here, so I guess this will require some kind of rawset that would bypass the setter.

@pineapplemachine
Copy link

pineapplemachine commented Jul 22, 2024

I think this could be better implemented with a setter a signature like set(new_value, old_value, value_setter_func). Instead of a variable that is treated very specially as a pointer to the var, you would refer to old_value for the value prior to setting, and you would call value_setter_func(new_value) to assign the var, without strictly needing to know in this setter which var it is.

In cases where a property name is needed I think I like @cmd410's suggestion of using bind over set providing a property name string as in godotengine/godot#78991, and just sticking with this more minimal list of params for set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants