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

Make lazy loading of nodes easier with a dedicated function or real callables as getters #9935

Open
Kobberholm opened this issue Jun 9, 2024 · 4 comments

Comments

@Kobberholm
Copy link

Kobberholm commented Jun 9, 2024

Describe the project you are working on

An open source educational real time strategy game for students (aged 10-13), about energy production and consumption with lots of functionality available from inside the editor via tool scripts.

Describe the problem or limitation you are having in your project

First off, my apologies for the long post. I love what you all do here and don't want to waste your time. I fear that reading this post would take longer than implementing what I am asking for.

I modularize my code and keep it in separate files, that make heavy use of dependency injection, to keep the logic separate from the implementation.
To avoid the "pollution" of the game code, all references to nodes are done in the variable section of the root node of each scene, making it easy to update if anything gets reordered or renamed. I.e. I have a statically typed variable for each node that I wish to access.

This is commonly done with @onready var my_node = get_node("MyNode").
The get_node() function call is often replaced with the % or $ syntax, but the result is the same.

For tool scripts, this doesn't work well, since:

  • @onready doesn't run in the editor I was misinformed on this. See note below.
  • All of these methods have the side effect of throwing errors, if the node does not exist, which is highly likely when run before the scene is ready.

_enter_tree() works as a replacement for _ready() in many cases, but not when it comes to referencing other nodes, so you need to check if the node is available every time you try to access it.

NOTE
ydeltastar informed me that the 'ready' notification also happens in the editor, which means the onready vars CAN be used for tool scripts, if you don't mind manually reloading the scene occasionally, however in my opinion, lazy loading is still better in nearly every single situation and deserves an equally simple syntax.

For this, getters are ideal, so all my node references use getters and in order to avoid errors, I use get_node_or_null() instead of get_node().
I've read somewhere, that searching the scene tree comes with some overhead (and certainly there is a bit of overhead in parsing the string), so instead of simply returning the result of get_node_or_null("MyNode") in the getter, I store the resulting node reference in the variable and directly return that if it is not null.

This leads to a ton of code blocks like this:

var buildings : Node2D:
	get:
		if not buildings:
			buildings = get_node_or_null("%Buildings")
		return buildings

Initially I only converted my @onready loaders to getters as I needed them in my toolscripts, but over time I have come to the conclusion that there is hardly ever a reason to load node references before they are accessed, so I exclusively use lazy loading getters like the above, unless a rare and specific reason to load on ready exists.

Having to use 5 lines (can be shortened to 4, at the expense of readability) for something that would normally require just 1, means my variables section is unnecessarily long. Many scenes have 10 or 20 variables like these, which results in as much as 100 lines, just for boiler plate code at the top of the file.

One solution is to make dedicated getter functions, but this makes the script even larger and harder to read and it seems like the get = getter_function only accepts function names, not functions with parameters or real Callables (that could have arguments bound to them), so you can't even implement a flexible lazyload function that could be used for all variables.

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

Introducing a get_node keyword to variable assignment

The most direct solution would be to implement a feature specific to this use-case.
This could be the introduction of a new keyword (e.g. get_node) in the variable definition, that strictly lazily loads a node and stores the result in the variable:

var buildings : Node2D: get_node = "%Buildings"

This would only try to resolve the NodePath when the variable is accessed and the variable is null.

Introducing a lazy_load_node annotation

If, for some reason, there is a desire to not expand the variable definition syntax, it could be implemented with a new annotation:

@lazy_load_node("%Buildings")
var buildings : Node2D

Both of the above mentioned dedicated lazyloaders should require very little code to implement and as far as I can tell. would not conflict with any existing functionality.

Real Callables as getters

A more generic solution, that could be used for a lot of other purposes, would be to allow the get = getter_function syntax to accept any callable, instead of just a function name. This would lead to a syntax like so:

var buildings : Node2D: get = lazyload.bind("%Buildings")

Since you cannot assign the output of a function to a a getter, there would be no ambiguity by shortening the get = Callable.bind(args) syntax to be just get = Callable(args) for an even cleaner syntax.

The Callable would of course need to not trigger the get() method itself.

The issue here is that the lazyloader cannot know which variable it is acting as a getter for, so it cannot store the result, only return it.

Forced pass-by-reference

If it was possible to force a pass-by-reference, the following syntax could tell the getter where to store the result before returning it:

var buildings : Node2D: get = lazyload(buildings, "%Buildings")

I have tried to work around this lack of control over pass-by-reference, but it is non-trivial and would likely require the variable to be initialized as an object that Godot will pass by reference, but this will clash with the variable type declaration (and therefor the auto completion in the editor) and would be confusing and a (smallish) waste of resources.

So for a generic solution with a clean syntax, the following would need to be done:
A: Allow Callables as getters.
B: Force the function to be called with variables as references (or add a syntax to mark a pass-by-reference).
C: (optional) Remove the need for bind() when using a callable, since it is already unambiguous (purely a cosmetic feature).

Variable initializer

Similar to a getter, but the return value is automatically stored to the variable and it only runs if the variable is null.
This could be used in a lot of different scenarios to lazy load variables without having to check if they were previously loaded.

var buildings : Node2D:
	init: return get_node_or_null("%Buildings")

Variable initializer with real Callable

Combining the two above, this could be a one-liner without pass-by-reference, if real Callables were implemented as well as the variable initializer that stores the output to the variable before returning the result:

var buildings : Node2D: init = get_node_or_null.bind("%Buildings")

A: Allow Callables as getters.
B: Introduce a keyword to both initialize a value AND store the result in the variable if the variable is null
C: (optional) Remove the need for bind() when using a callable, since it is already unambiguous (purely a cosmetic feature).

Other ways to lower the line count

There are various ways the number of lines could be lowered with new language features, such as by implementing #1321, and allowing return values from variable assignment, but it would still require custom getter code for something I believe ought to be the recommended way of accessing nodes inside your scripts.

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

If implemented with a dedicated keyword, such as get_node, this would simply make a getter that does the equivalent of the following code, except in C++:

var node_ref : Node:
	get:
		if not node_ref:
			node_ref = get_node_or_null("%NodePath")
		return node_ref

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

It is relatively simple to do the lazy loading via script, but it is the repetitiveness of implementing this workaround that leads me to make this proposal.
This enhancement is specifically intended to avoid large sections of boiler plate code and to promote good coding habits, by not scattering NodePaths around your game logic code.

Most scripts separating NodePaths from game logic already use the @onready method of assigning nodes to variables, which can be done in a single line of code, but the even better (and for tool scripts necessary) way of doing this, by lazy loading, requires 5 lines (strictly speaking 4) for each variable.

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

This requires a (small) addition to GDScript itself.

@ydeltastar
Copy link

allow the get = getter_function syntax to accept any callable, instead of just a function name.

I agree that get/set assignments should accept callables. They still work like the time functions weren't objects.

  • @onready doesn't run in the editor

The editor works like a running game. You have to Scene > Reload Save Scene for the scene to be deleted and added to the editor again, which triggers 'ready' notifications. I assigned it a shortcut Ctrl+Shift+R, it's needed every time I edit initialization code in tool scripts.

@Kobberholm
Copy link
Author

  • @onready doesn't run in the editor

The editor works like a running game. You have to Scene > Reload Save Scene for the scene to be deleted and added to the editor again, which triggers 'ready' notifications. I assigned it a shortcut Ctrl+Shift+R, it's needed every time I edit initialization code in tool scripts.

Oh, I see. I wasn't aware of that.
I am sure that where ever I learned GDScript several years ago, said that the scenes don't emit the ready signal/run the ready function inside the editor and I have been working under that assumption ever since, but I just tested it and you are right!
This will come in handy in several places where I previously would reload the project :)

I'm not sure it changes my opinion on lazy loading vs onready loading though.
Lazy loading still seems like the superior way of accessing nodes or initializing variables, except for some rare scenarios where the initialization is particularly slow or initialization needs to happen in a specific order that cannot be guaranteed otherwise.

I try to build my programs around the data, with signals for when the data changes and then having the frontend update just the necessary bits to reflect the data changes, through listening for those signals.
Bulk-loading everything during onready feels like it is the opposite strategy.

And manually reloading scenes to trigger the ready signal also seems like a step back, though it is certainly handy for some things.

@huwpascoe
Copy link

I end up using this pattern everywhere when it comes to tool scripts:

@tool
extends Node

@export var my_var := 123:
  get: return my_var
  set(value): my_var = value; _update()

func _ready():
  _update()

func _update():
  if not is_node_ready(): return

  # do stuff

@Kobberholm
Copy link
Author

I end up using this pattern everywhere when it comes to tool scripts:

@tool
extends Node

@export var my_var := 123:
  get: return my_var
  set(value): my_var = value; _update()

func _ready():
  _update()

func _update():
  if not is_node_ready(): return

  # do stuff

That definitely looks familiar 😄

There is nothing wrong with that pattern, but one possible redesign, is to use a signal to connect the variable change to the function call.
Instead of calling the _update function from the setter, you just emit the signal.
Inside your _ready function, you connect the signal to the _update function.
Only after the node is in ready state, will the _update() function run when the variable changes.

@tool
extends Node

signal data_changed

@export var my_var := 123:
	set(value): my_var = value; data_changed.emit()

func _update() -> void:
	pass

func _ready() -> void:
	data_changed.connect(_update)
	data_changed.emit()

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

4 participants