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

[BUG] : cant handle multi return values #84

Open
ParadiseFallen opened this issue Aug 5, 2023 · 7 comments
Open

[BUG] : cant handle multi return values #84

ParadiseFallen opened this issue Aug 5, 2023 · 7 comments

Comments

@ParadiseFallen
Copy link

so basically code is

const factory = new LuaFactory()
const engine = await factory.createEngine();

const fn= await engine.doString(`
      return function ()
        return '22', '33', {};          
      end;
      `);
      
const result = fn(); // result is '22' while expected ['22','33', {}]
// same there
const result2 = await engine.doString(`
      return '11','22',{};
      `);
// result  is '11' while expected ['11','22',{}]
@ParadiseFallen
Copy link
Author

ParadiseFallen commented Aug 5, 2023

after some time i figure out that it always return ONLY one result for functions (which is wrong. bc .call() returns multiresult)

this is fix for functions

engine.global.typeExtensions[4].extension.__proto__.getValue = (thread: Thread, index: number) => {
        thread.lua.lua_pushvalue(thread.address, index)
        const func = thread.lua.luaL_ref(thread.address, LUA_REGISTRYINDEX)

        const jsFunc = (...args: any[]): any => {
          if (thread.isClosed()) {
            console.warn('Tried to call a function after closing lua state')
            return
          }

          const internalType = thread.lua.lua_rawgeti(thread.address, LUA_REGISTRYINDEX, func)
          if (internalType !== LuaType.Function) {
            const callMetafieldType = thread.lua.luaL_getmetafield(thread.address, -1, '__call')
            thread.pop()
            if (callMetafieldType !== LuaType.Function) {
              throw new Error(`A value of type '${internalType}' was pushed but it is not callable`)
            }
          }

          for (const arg of args) {
            thread.pushValue(arg)
          }
          
          const base = thread.getTop();
          const status = thread.lua.lua_pcallk(thread.address, args.length, LUA_MULTRET, 0, 0, null)
          if (status === LuaReturn.Yield) {
            throw new Error('cannot yield in callbacks from javascript')
          }
          thread.assertOk(status)
          const multiReturn = thread.getStackValues(thread.getTop() - base - args.length);

          // const result = thread.getValue(-1)

          thread.pop()
          return multiReturn;
        }

        ext.functionRegistry?.register(jsFunc, func)

        return jsFunc
      };
      
  And this is fix for just return
      engine.doString = async (code : string) => {
        const thread = engine.global.newThread()
        const threadIndex = engine.global.getTop()
        try {
            thread.loadString(code);
            const results = await thread.run(0)
            if (results.length > 0) {
              engine.cmodule.lua_xmove(thread.address, engine.global.address, results.length)
            }
            return results;
        } finally {
            // Pop the read on success or failure
            engine.global.remove(threadIndex)
        }
      };
      

@ParadiseFallen
Copy link
Author

const [result] = fn();

Is looks much predicteble when we always mean that lua function returns multiresult.
Or we can use kinda helper method that

const result = fn().singleResult();

where

array.prototype.singleResult = function()
{
  return this.at(0);
}

@ParadiseFallen
Copy link
Author

ParadiseFallen commented Aug 5, 2023

https://github.com/ParadiseFallen/wasmoon/blob/main/test/engine.test.js#L268

So this test is always failed. seems like top calculated not correctly

https://github.com/ParadiseFallen/wasmoon/blob/main/src/type-extensions/function.ts#L187

there was expeted 8 but i got always only 3. so where is 5 more missing?

@ParadiseFallen
Copy link
Author

#84

Fix is right there

@ParadiseFallen
Copy link
Author

@ceifa i can change apis to always return LuaReturn or just array. It will be breaking change. I want this feature

@ceifa
Copy link
Owner

ceifa commented Sep 2, 2023

@ceifa i can change apis to always return LuaReturn or just array. It will be breaking change. I want this feature

@ParadiseFallen this is not actually a bug, it was designed to work that way. We need to understand now which design would be better for readability/functionality/DX.

What is your use case?

@ParadiseFallen
Copy link
Author

@ceifa use function from lua with multireturn.
So for example if you want to call require in js side from lua it must return module & optionally path to module.
It will also good to use with other methods.

I suggest

Always return an array expecting mulret

It looks like

let [module,path] = luaRequire('abc');

And if it still

function ()
  return 123;
end

on js will looks like

let [result] = luaMethod();

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