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

Call setters / getters of setget variables when set / gotten from a child class #16788

Closed
raymoo opened this issue Feb 18, 2018 · 7 comments
Closed

Comments

@raymoo
Copy link
Contributor

raymoo commented Feb 18, 2018

I'm trying to expose an interface for subclasses of a class similar to the built-in properties (like position) that already exist, but it seems like the functions are not called by sets / gets from inheriting classes.

It's not clear in the docs whether this is intended or not, since it only says that local (in scope?) accesses do not call the setters / getters. If the current behavior is intended then this is a feature request.

@ghost ghost added this to the 3.1 milestone Feb 18, 2018
@vnen
Copy link
Member

vnen commented Jul 22, 2018

This is not a bug, it's the intended way. Calling getters/setters in this case would be a big performance hit. You need to manually force the setter/getter by indexing self: self.property.

Looks the same as #13331.

@raymoo
Copy link
Contributor Author

raymoo commented Jul 24, 2018

Yes, I think this is a duplicate. Though I think that it should be possible to know at some point before actually running a variable access whether it can do a direct access or if it needs to call getters / setters (already suggested by @letheed in the other issue).

@bojidar-bg
Copy link
Contributor

I think the current behaviour should be changed, since it hampers plugin-makers a bit.
For example:

# res://addons/testaddon/testnode.gd
export var image_rotation
export var image_rotation_degrees setget get_image_rotation_degrees, set_image_rotation_degrees

# res://TestNode.gd
extends "res://addons/testaddon/testnode.gd"
func _ready():
    image_rotation_degrees = 90

The creator of the TestNode script tried to make it similar to core classes by adding rotation in both radians and degrees. Unfortunately, a script for his custom node currently cannot specify angles in degrees, since it won't update the real rotation as the setter won't get called.

I'm not sure it is an exact duplicate since the other issue talks about all properties, while this one is about inherited only.

@letheed
Copy link
Contributor

letheed commented Jul 24, 2018

My issue was a bit poorly formulated looking back, but I think I'm not the only one who wishes we had something like properties without the need for self..

@bojidar-bg inheritance was indeed the biggest motivation for me, though I should probably have mentioned that in my first comment then.

@vnen
Copy link
Member

vnen commented Jul 24, 2018

Plugging in annotations (#20318), we could add one to force the setget to always be used. With the new parser changes it's possible to fix this at compile time, so there should not be any performance hits for the other variables.

@vnen vnen added feature proposal and removed bug labels Jul 24, 2018
@akien-mga akien-mga removed this from the 3.1 milestone Sep 15, 2018
@Calinou
Copy link
Member

Calinou commented May 26, 2020

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!

@vnen
Copy link
Member

vnen commented May 26, 2020

Note that my proposal for properties (godotengine/godot-proposals#844) would solve this.

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

6 participants