Rapptz / sol

A C++11 Lua wrapper
MIT License
208 stars 32 forks source link

Bad access when calling a function on returned userdata #35

Closed notlion closed 10 years ago

notlion commented 10 years ago

Hi! I'm trying to bind a class that returns instances of it's own type. Here's a simplified example:

struct Vec {
  float x, y, z;
  Vec(float x, float y, float z) : x{x}, y{y}, z{z} {}
  float length() {
    return sqrtf(x*x + y*y + z*z);
  }
  Vec normalized() {
    float invS = 1 / length();
    return {x * invS, y * invS, z * invS};
  }
};

sol::state lua;
lua.open_libraries(sol::lib::base);

sol::constructors<sol::types<float, float, float>> ctor;
sol::userdata<Vec> udata("Vec", ctor,
  "normalized", &Vec::normalized,
  "length",     &Vec::length);

lua.set_userdata(udata);

This script works:

lua.script("v = Vec.new(1, 2, 3)\n"
           "print(v:length())");
// --> 3.7416574954987

But this one fails:

lua.script("v = Vec.new(1, 2, 3)\n"
           "print(v:normalized():length())");
// --> EXC_BAD_ACCESS

Is it possible to do this with sol, or am I missing something?

Rapptz commented 10 years ago

I believe at the moment the only things supported are return *this and return this. I'm not sure how to make returning a new instance work seamlessly.

notlion commented 10 years ago

Ok. Thanks for the reply. Is it also not possible to bind constructors that take previously defined userdata types? For example, this does not compile: (Sorry if these are dumb questions. I'm new to lua bindings)

struct Line {
  Vec p1, p2;
  Line(Vec p1, Vec p2) : p1{p1}, p2{p2} {}
};

sol::constructors<sol::types<Vec, Vec>> ctor;
sol::userdata<Line> udata("Line", ctor);
lua.set_userdata(udata);
ThePhD commented 10 years ago

I have both issues from this fixed up. Going to pull request after I add some tests that cover for this case (before we were only checking *this and this returns in our tests: now we check other cases).

Rapptz commented 10 years ago

I guess this issue is fixed now.

dgdev1024 commented 9 years ago

Main.cpp

#include <sol.hpp>
#include <cmath>

struct Vec {
    float x, y, z;
    Vec(float x, float y, float z) : x{x}, y{y}, z{z} {}
    float length() {
        return sqrtf(x*x + y*y + z*z);
    }
    Vec normalized() {
        float invS = 1 / length();
        return {x * invS, y * invS, z * invS};
    }
};

int main()
{
    sol::state lua;
    lua.open_libraries(sol::lib::base);

    sol::constructors<sol::types<float, float, float>> ctor;
    sol::userdata<Vec> udata("Vec", ctor,
      "normalized", &Vec::normalized,
      "length",     &Vec::length);

    lua.set_userdata(udata);

    lua.open_file("Test.lua");

    return 0;
}

Test.lua

v = Vec.new(1, 2, 3)
n = v:normalized()

print(n:length())

Running the code, 'print(n:length())' yields some weird results. Five different executions of the program yield the following: 0 inf 0 854380 1.2764733714477e-11

Is there anything I'm missing/doing wrong? Thank you.

Rapptz commented 9 years ago

I only get 0, which is still incorrect of course. It seems that the test case for this didn't actually verify the result but rather just tested if it didn't raise an error. I recommend opening a new issue.

Rapptz commented 9 years ago

This actually seems to be "fixed" in the develop branch sort of. It returns 0.99999994039536 instead of 1 but I suppose it's "close enough"? There must be some sort of issue going on with the accuracy but it's better than zero.

dgdev1024 commented 9 years ago

I guess the only thing I'm missing here is the concept of git branches. I had no idea there was a develop branch. Anyway, looks like my issue's been resolved. Thank you so!

As for the inaccuracy, I believe that is natural with Lua's C API. I had the same issue with a different non-c++11 Lua-wrapping API (namely, https://bitbucket.org/alexames/luawrapper/src).

A safe workaround would probably be to round it maybe to six decimal places...