fengari-lua / fengari

🌙 φεγγάρι - The Lua VM written in JS ES6 for Node and the browser
MIT License
1.81k stars 65 forks source link

Register doesn't work #178

Open skejeton opened 4 years ago

skejeton commented 4 years ago
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 commented 4 years ago
lauxlib.luaL_dostring(L, fengari.to_luastring("test()"))
daurnimator commented 4 years ago

luaZ_fill inside of lzio.js should use from_userstring

ricksterhd123 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.