WeaselGames / godot_luaAPI

Godot LuaAPI
https://luaapi.weaselgames.info
Other
348 stars 27 forks source link

Bug Report: Vector3 metatable not working when nested in an object #200

Open jbromberg opened 4 months ago

jbromberg commented 4 months ago

Describe the bug When pushing an object variant that contains a Vector3 property, it's not updated correctly. Only global level Vector3s are being updated.

To Reproduce

class LuaClass:
    var vec := Vector3.ZERO

var vec := Vector3.ZERO

var lua := LuaAPI.new()
lua.push_variant("vec", vec)
lua.push_variant("luaClass", LuaClass.new())
lua.do_string("
vec.x = 10
print(vec) -- (10, 0, 0)
luaClass.vec.x = 10
print(luaClass.vec) -- (0, 0, 0)
")
jbromberg commented 4 months ago

Seems to not work on other nested types such as godot arrays too.

jbromberg commented 4 months ago

Works with nested basic types like bool, float, and string

Trey2k commented 4 months ago

This is a side effect to how we handle metatables. Properties are not passed by reference unless they are a Object type. So types like Vector3 what ends up happening is obj.vec pushes a copy of the vector onto the stack. obj.vec.x = 10 modifies the copy of the vector and not the objects vector. And since the first obj.vec is only a index call and not a newindex call the copy is not reapplied to the object ever.

Realevent areas of the code base are the following. https://github.com/WeaselGames/godot_luaAPI/blob/edf4afe7665a03978e8a4a3a15b727a66c4dc03d/src/metatables.cpp#L176-L192 https://github.com/WeaselGames/godot_luaAPI/blob/edf4afe7665a03978e8a4a3a15b727a66c4dc03d/src/classes/luaObjectMetatable.cpp#L197-L213 https://github.com/WeaselGames/godot_luaAPI/blob/edf4afe7665a03978e8a4a3a15b727a66c4dc03d/src/classes/luaObjectMetatable.cpp#L214-L237

I will need to spend some time thinking on this to find a good solution. This is not the intended behavior.

jbromberg commented 4 months ago

Sounds good. Would it be possible to somehow recursively instantiate object properties the same way globals are created? I'm pretty new to Lua so not sure if it works like this but basically instead of just copying all the object's properties we could iterate over them and if they're a non basic type they get their own metatable like the globals do. If it encounters another object, it recurses and thus it could work for several layers of nesting & complex data types.

Trey2k commented 4 months ago

Sounds good. Would it be possible to somehow recursively instantiate object properties the same way globals are created? I'm pretty new to Lua so not sure if it works like this but basically instead of just copying all the object's properties we could iterate over them and if they're a non basic type they get their own metatable like the globals do. If it encounters another object, it recurses and thus it could work for several layers of nesting & complex data types.

So this is not actually the issue here. The data type has a metatable. The issue is the vector being modified is not the objects vector but a copy. If a property on the object is modified IE the call goes through newindex we update it via reference like with obj.prop=val. But if its not modified directly on the object IE the call goes through index like with obj.prob.x=val the property is pushed as a copy. Then that copy is what gets modified.

So in your example if you changed the lua code to

vec.x = 10
print(vec) -- (10, 0, 0)
luaClass.vec = Vector3(10, 0, 0)
print(luaClass.vec) -- (10, 0, 0)

It would then work as expected.

Trey2k commented 4 months ago

A fix would be to always push properties by ref. But I think that would create other issues. some_vec = obj.vec you wouldnt expect some_vec to be a ref to obj.vec in that case but it would end up being so.

jbromberg commented 4 months ago

Makes sense. So for globals it just modifies the vector directly on the global table instead of being a copy, but it's a copy for nested types?

Another idea could be to somehow pass __newindex calls up the owner chain. So if a child vector gets a new property assigned and it's being referenced from an object the object would get the new vector which got the new property.

Or could push by ref but create copies on assignment, so some_vec = obj.vec would copy