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

Wrong y coord reported from Lua scripts #4

Closed
bojjenclon opened this issue Nov 29, 2021 · 12 comments
Closed

Wrong y coord reported from Lua scripts #4

bojjenclon opened this issue Nov 29, 2021 · 12 comments

Comments

@bojjenclon
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

gilzoide commented Nov 29, 2021

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
Copy link
Contributor Author

bojjenclon commented Nov 30, 2021

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
Copy link
Contributor Author

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
Copy link
Owner

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

@gilzoide
Copy link
Owner

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
Copy link
Owner

gilzoide commented Dec 3, 2021

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
Copy link
Owner

gilzoide commented Dec 3, 2021

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@gilzoide
Copy link
Owner

gilzoide commented Dec 4, 2021

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.

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