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

Allow to use Callables as setters/getters #3316

Closed
KoBeWi opened this issue Sep 18, 2021 · 6 comments
Closed

Allow to use Callables as setters/getters #3316

KoBeWi opened this issue Sep 18, 2021 · 6 comments

Comments

@KoBeWi
Copy link
Member

KoBeWi commented Sep 18, 2021

Describe the project you are working on

A game using GDScript that I'm going to eventually port to Godot 4.0.

Describe the problem or limitation you are having in your project

There's this new setter syntax:

var a:
  set(v):
    v = something

The problem is that it gets messy with lots of variables with setters, especially if the setters are long.

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

We should be able to move the setter/getter body somewhere else. So instead of setter body, we could provide a Callable that gets called as the setter. This would also make auto-converting legacy code much easier.

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

var a:
  set(v): my_setter
  get: my_getter

func my_setter(v):
  etc.

Or a shorter version:

var a: set(v): my_setter, get: my_getter

(although that's reminiscent of the old setget, which wasn't very pretty)

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

No. Trying to call a custom method from withing setter/getter body results in infinite recursion and crash without message.

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

GDScript is core™

@dalexeev
Copy link
Member

var a:
  set(v): my_setter
  get: my_getter

What function does v serve in your example? You can already use this:

var a:
  set(v): my_setter(v)
  get: return my_getter()

IMO this is already short enough, but we can add the following syntax:

var a:
  set = my_setter
  get = my_getter

or

var a: set = my_setter, get = my_getter

Note that an equal sign is used instead of a colon. I think this is more logical and consistent with the rest of the syntax.

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 19, 2021

What function does v serve in your example? You can already use this:

my_setter(v) won't work (godotengine/godot#52757). The v is not needed, but I included it, so the syntax doesn't need to change.

Well, but if we were to introduce new syntax, your proposed one is better.

@dalexeev
Copy link
Member

my_setter(v) won't work (godotengine/godot#52757).

This is incorrect code, and it shouldn't work (but of course it shouldn't crash the engine). It should be like this:

var my_var:
    get: return get_my_var()
    set(value): set_my_var(value)

var _my_var

func get_my_var():
    return _my_var

func set_my_var(value):
    _my_var = value

By design, access to my_var should call get and set everywhere except within get and set. This is convenient for its consistency.

Do I understand correctly what you suggest that in the functions get_my_var and set_my_var, accessing my_var does not cause infinite recursion (if var my_var: get = get_my_var, set = set_my_var)? And so there was no need for _my_var? This can be convenient, but it creates additional non-obviousness.

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 19, 2021

Looking at it again, my proposal has the same problem as the code that causes infinite recursion .-.

So I guess

Do I understand correctly what you suggest that in the functions get_my_var and set_my_var, accessing my_var does not cause infinite recursion (if var my_var: get = get_my_var, set = set_my_var)?

This has to be true.

All these problems are caused by the fact that you can no longer change a property without triggering setter/getter. It's problematic.

@vnen
Copy link
Member

vnen commented Sep 21, 2021

This syntax already exists:

var my_prop:
    get = get_my_prop, set = set_my_prop

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 21, 2021

🙃

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

No branches or pull requests

3 participants