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

NodePath property is inaccessible in Lua #1

Closed
JohnDoneth opened this issue Sep 17, 2021 · 5 comments
Closed

NodePath property is inaccessible in Lua #1

JohnDoneth opened this issue Sep 17, 2021 · 5 comments

Comments

@JohnDoneth
Copy link
Contributor

JohnDoneth commented Sep 17, 2021

Plugin Version: r1
Godot Version: v3.3.3

I'm trying to use a NodePath property and access it from Lua.

The code appears to register the property with Godot as it shows up and works in the inspector but is inaccessible via self in Lua.
image

Properties are working for other primitive types I've used so far but not when the type is NodePath. When I inspect self inside of _ready with the following code, I get the subsequent outcome.

local inspect = require("inspect")

local function ready(self)
  print(inspect.inspect(self))
  print(self.player_path)
end

return {
	_ready = ready,
	player_path = property({type = NodePath})
}
{
  __owner = [Node2D:1622],
  __script = {
    __getter = {},
    __path = "res://enemy.lua",
    __setter = {},
    _ready = <function 1>
  },
  <metatable> = {
    __concat = <function 2>,
    __index = <function 3>,
    __tostring = <function 4>
  }
}
nil

As you can see, the property is not on self and printing the field directly shows that it doesn't exist, as it's nil.


I've been loving this project by the way. It's great! Thank you for working on it.

@JohnDoneth JohnDoneth changed the title Node NodePath property is inaccessible in Lua NodePath property is inaccessible in Lua Sep 17, 2021
@JohnDoneth
Copy link
Contributor Author

More Digging

Digging some more, I found that the property is in the property list that is returned by self:get_property_list().

print(self:get_property_list()[#self:get_property_list() - 1])

--> {class_name:, hint:0, hint_string:, name:player_path, type:15, usage:7}

Attempting to use self:get("player_path") directly instead of through the __index metamethod doesn't appear to work either, as it also returns nil.


Discovering a Solution

While looking through the code dealing with properties I noticed some mentions of the default value being kept for indexing purposes. The way I had the code did not have a default value. This lead to me wonder what would happen if I assigned the property player_path a new instance of NodePath directly instead of calling property with just a type specified. Doing so fixed my issue.

Code

local function ready(self)
	print(self.player_path)
	print(self:get_node(self:get("player_path")))
end

return {
	_extends = "Node2D",
	_ready = ready,
	player_path = NodePath()
}

Output

../Player
[Node2D:1604]

Conclusion

Maybe we should set default_value to a new instance of type, or always require a default value when calling property to avoid someone else getting stuck like me? I'll keep this issue open if you feel that's actionable, or we can create a new more focused issue. Feel free to close this otherwise.

Thanks again for the great project!

@gilzoide
Copy link
Owner

Hi there! I see you already found out how properties are working currently. Yeah, the property is registered in ClassDB, but without an initial value, getting it would return null on GDScript.
But you're right, there is a bug when the default value is nil, in a way that the "get property" callback thinks that the property doesn't exist!

I think the way to go is really adding a fallback default value for properties, when absent, thanks for pointing it out! Requiring a default value is reasonable too, but having fallback empty/zero/identity values seems nicer to me.
Tracking known properties in a separate __properties table is probably a good idea too.

Also, now that I'm thinking better about this, just falling back to __indexing the script will be very error-prone for Arrays, Pool Arrays, Dictionaries and Object values =X
When instancing the script, we can iterate over known properties and call a constructor function for each one. I'll make this happen =]

Either way, we can always stuff initializers in the _init method, although it is much nicer to just define initial values to be created.


Not related but more out of curiosity, what OS are you using? Also, did you use the prebuilt release .zip file?
Thanks again for the help, I'm really glad you like it ^^

@gilzoide
Copy link
Owner

gilzoide commented Sep 18, 2021

Just FYI, now that I'm remembering more stuff, the idea was that default value was sort of required, but I though that having null as default was not necessarily bad, as it might just be what people want, so just let it roll.
Now I see that a default default value is better and is probably what most people would want, and getter methods may be used for defaulting to null.

But the "get property" bug still applies, so in the end it was a really bad choice =P

@JohnDoneth
Copy link
Contributor Author

Thank you for the detailed explanation. I like your solution and I'm glad I could help.

Not related but more out of curiosity, what OS are you using? Also, did you use the prebuilt release .zip file?

I'm using Arch Linux and I did use the r1 release zip. It was plug-and-play. Very easy to get started.

@gilzoide
Copy link
Owner

gilzoide commented Sep 19, 2021

I'm using Arch Linux and I did use the r1 release zip.

I asked because I use mainly a Manjaro Linux, so was curious about how this plugin would behave on other systems.
I did test on Wine and on an OSX a couple of times, though, and it also worked on my attempts. But who knows, right?

It was plug-and-play. Very easy to get started.

Awesome!

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

No branches or pull requests

2 participants