search

gilzoide / godot-lua-pluginscript

Godot PluginScript for the Lua language, currently based on LuaJIT's FFI
https://gilzoide.github.io/godot-lua-pluginscript/topics/README.md.html
MIT License
300 stars 21 forks source link

Wrong y coord reported from Lua scripts #4

Closed bojjenclon closed 2 years ago

bojjenclon commented 2 years ago

I'm having a weird issue where self.position from a KinematicBody2D is always giving a y-coord of 0. I tested the exact same setup with GDscript and the position was reported as expected, so its definitely something on the Lua side.

gilzoide commented 2 years ago

Hi! Can you please send both Lua and GDScript scripts you're using, preferably within a Godot Project with a scene using them, so I can reproduce the bug?

bojjenclon commented 2 years ago

Sure, I've attached a super simple repro setup. This was made in Godot 3.4. You can just play the scene and look at the output. Notice that the Lua output is always (x, 0) whereas the GD output is (x, y) as expected.

position_test.zip

gilzoide commented 2 years ago

Well, that's weird, it seems to be working here =S Which platform are you using? I tested here in a x64 Linux machine and x32 + x64 Windows via wine. Also, do you get any errors in the output? Screenshot_20211129_190641

bojjenclon commented 2 years ago

Hmm this is very strange indeed. I'm on 64 bit Linux Mint. I even tried building the plugin from source, and the issue is still there. Since I pulled the repo down for testing anyway, I'll try digging in a bit and see if I can figure it out.

No errors btw, that's part of the oddness of it.

bojjenclon commented 2 years ago

So out of curiosity I tried running it in an older version of Godot (3.3.4) and I don't see the problem there.

gilzoide commented 2 years ago

Ok, I found how to reproduce. I've downloaded the Mono enabled editor, and now I see the zero every time =O

gilzoide commented 2 years ago

Running the Mono enabled Windows editor through wine doesn't show the problem =S I'll try building a debug version of the editor later to try running it through a debugger and see if I can find what's going on.

gilzoide commented 2 years ago

Ok, I found it! Here's the tale:

  1. First, I tried building my own Mono enabled editor and the problem was gone. So it only happens on this specific build and might be related to build configuration or compiler used or something like that.
  2. I tried some debugging using GDB and calls to godot_vector2_get_y and godot_variant_as_vector2. The first one was giving 0, but the later had the correct values.
  3. I tried reproducing the problem in C, still in the Lua PluginScript library, and the error persisted (I was testing OS.window_size because it was easy to get). While debugging, I saw that although godot_variant_as_vector2 had the right values, the godot_vector2 I was assigning the result to did not.
  4. Next, I tried creating an empty reproduction Godot Project with a pure C library with direct calls to GDNative (without HGDN) and the problem was gone! =O So, ok, something is off when using HGDN in this build of Godot.
  5. I copied the definition of godot_vector2 that both HGDN and Lua PluginScript use to this repro project, and the problem begun. So the problem only happens when using this definition.
  6. After messing around it for a little, it seems to be some sort of ABI mismatch. Godot thinks the data is uint8_t [8] but we say it's a float [2]. Although the sizes match, there must be some register allocation mismatch between my compiler and the one used for building the oficial Linux x64 + Mono build when this is returned from a function. Adding uint8_t data[8] as the first field in the union solves this and we can still directly reference x and y from C or Lua =D

Another interesting thing is that Vector3 also had this problem, probably more math types did, if not all of them. Also, I tried the x32 Mono build and it didn't present the error.

Thanks for reporting this, I'll push a fix soon!

gilzoide commented 2 years ago

@bojjenclon Should be working now =) Please tell me if you test and confirm the fix. Probably this weekend I'll release a new version with it.

bojjenclon commented 2 years ago

Ok, I found it! Here's the tale:

  1. First, I tried building my own Mono enabled editor and the problem was gone. So it only happens on this specific build and might be related to build configuration or compiler used or something like that.
  2. I tried some debugging using GDB and calls to godot_vector2_get_y and godot_variant_as_vector2. The first one was giving 0, but the later had the correct values.
  3. I tried reproducing the problem in C, still in the Lua PluginScript library, and the error persisted (I was testing OS.window_size because it was easy to get). While debugging, I saw that although godot_variant_as_vector2 had the right values, the godot_vector2 I was assigning the result to did not.
  4. Next, I tried creating an empty reproduction Godot Project with a pure C library with direct calls to GDNative (without HGDN) and the problem was gone! =O So, ok, something is off when using HGDN in this build of Godot.
  5. I copied the definition of godot_vector2 that both HGDN and Lua PluginScript use to this repro project, and the problem begun. So the problem only happens when using this definition.
  6. After messing around it for a little, it seems to be some sort of ABI mismatch. Godot thinks the data is uint8_t [8] but we say it's a float [2]. Although the sizes match, there must be some register allocation mismatch between my compiler and the one used for building the oficial Linux x64 + Mono build when this is returned from a function. Adding uint8_t data[8] as the first field in the union solves this and we can still directly reference x and y from C or Lua =D

Another interesting thing is that Vector3 also had this problem, probably more math types did, if not all of them. Also, I tried the x32 Mono build and it didn't present the error.

Thanks for reporting this, I'll push a fix soon!

Woah, awesome debugging! Thanks for the breakdown, I've never really tinkered with this side of Godot so its actually really cool to get a better understanding of the flow here.

Super weird about the type mismatch, but glad you managed to figure it out.

I'll give it a try ASAP and report back to you :)

bojjenclon commented 2 years ago

@gilzoide Just tested it and it works! Great job fixing the issue

gilzoide commented 2 years ago

The GDNative API is quite straightforward, although low-level, but it follows the same semantics as GDScript. The types are the same, there is godot_variant for holding any value, you can call methods by their name, things like that.

Super weird about the type mismatch

Yeah =/ I didn't stop to think about how the libraries are dynamically loaded and there could be mismatches it definitions are different. Living and learning, right? =]

Just tested it and it works!

Thanks for the double-check!

I want to have unit tests someday, they could be run in the several oficial Godot binaries and things like this bug would be caught earlier.