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

Register doesn't work #178

Open
skejeton opened this issue Mar 24, 2020 · 5 comments
Open

Register doesn't work #178

skejeton opened this issue Mar 24, 2020 · 5 comments

Comments

@skejeton
Copy link

let fengari = require('fengari');
const luaconf  = fengari.luaconf;
const lua      = fengari.lua;
const lauxlib  = fengari.lauxlib;
const lualib   = fengari.lualib;


let L = lauxlib.luaL_newstate();

lualib.luaL_openlibs(L)

lua.lua_register(L, "test", (state) => {
    console.log("got here")
    return 0;
})

lauxlib.luaL_dostring(L, "test()")

doesn't output anything

@skejeton
Copy link
Author

lauxlib.luaL_dostring(L, fengari.to_luastring("test()"))

@daurnimator daurnimator reopened this Mar 26, 2020
@daurnimator
Copy link
Member

luaZ_fill inside of lzio.js should use from_userstring

@ricksterhd123
Copy link

luaZ_fill inside of lzio.js should use from_userstring

My naive approach:

const luaZ_fill = function(z) {
    let buff = from_userstring(z.reader(z.L, z.data));
    if (buff === null)
        return EOZ;
    lua_assert(buff instanceof Uint8Array, "Should only load binary of array of bytes");
    let size = buff.length;
    if (size === 0)
        return EOZ;
    z.buffer = buff;
    z.off = 0;
    z.n = size - 1;
    return z.buffer[z.off++];
};

Broke a lot of things, i assume that
from_userstring(to_luastring(x)) = to_luastring(x) because a lot of similar errors appeared:

FAIL test/lcorolib.test.js
   coroutine.create, coroutine.yield, coroutine.resume

    expect(received).toBe(expected) // Object.is equality

    Expected: 0
    Received: -1

      24 |     {
      25 |         lualib.luaL_openlibs(L);
    > 26 |         expect(lauxlib.luaL_loadstring(L, to_luastring(luaCode))).toBe(lua.LUA_OK);
         |                                                                   ^
      27 |         lua.lua_call(L, 0, -1);
      28 |     }
      29 |

      at Object.<anonymous> (test/lcorolib.test.js:26:67)

Here's what I got:

Test Suites: 38 failed, 2 passed, 40 total
Tests:       447 failed, 8 skipped, 43 passed, 498 total
Snapshots:   0 total
Time:        20.781s

It seems that a large number of tests convert strings to lua_strings before passing into loadstring, or dostring, etc. Maybe that was the way the creators intended? I was looking for something to fix, I don't think this is broken.

@daurnimator
Copy link
Member

daurnimator commented May 31, 2020

I had something more like this in mind:

const luaZ_fill = function(z) {
    let buff = z.reader(z.L, z.data);
    if (buff === null)
        return EOZ;
    buff = from_userstring(buff);
    let size = buff.length;
    if (size === 0)
        return EOZ;
    z.buffer = buff;
    z.off = 0;
    z.n = size - 1;
    return z.buffer[z.off++];
};

Test Suites: 38 failed, 2 passed, 40 total

Likely you forgot to import from_userstring?

const { from_userstring } = require('./defs.js');

@ricksterhd123
Copy link

I had something more like this in mind:

const luaZ_fill = function(z) {
    let buff = z.reader(z.L, z.data);
    if (buff === null)
        return EOZ;
    buff = from_userstring(buff);
    let size = buff.length;
    if (size === 0)
        return EOZ;
    z.buffer = buff;
    z.off = 0;
    z.n = size - 1;
    return z.buffer[z.off++];
};

Test Suites: 38 failed, 2 passed, 40 total

Likely you forgot to import from_userstring?

const { from_userstring } = require('./defs.js');

No, I'm not quite sure why my approach did not work, maybe the assert failed? Your solution works, I PR'd it. I will mention you, thanks.

ricksterhd123 added a commit to ricksterhd123/fengari that referenced this issue Jun 1, 2020
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

Successfully merging a pull request may close this issue.

3 participants