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

[Feature Request / Lua] Type for handling Lua var args "..." #7306

Closed
henrikvik opened this issue Jul 27, 2018 · 12 comments · Fixed by #9961
Closed

[Feature Request / Lua] Type for handling Lua var args "..." #7306

henrikvik opened this issue Jul 27, 2018 · 12 comments · Fixed by #9961
Assignees
Labels
enhancement platform-lua Everything related to Lua
Milestone

Comments

@henrikvik
Copy link

henrikvik commented Jul 27, 2018

I am using Haxe to make World Of Warcraft addons and something that is used frequently is the Lua var args as a parameter "..." my proposal is to add a specific Type for this

In Haxe I would write

eventManager.RegisterEvent("MESSAGE", (self:Frame, event:String, args:VArgs) -> {
    trace(args.length);
    if (event == "MESSAGE") {
        var msg:String = args[0];
    }
});

and that would compile to something like this

eventManager:RegisterEvent("MESSAGE", function(self, event, ...)
    print(select("#", ...));
    if (event == "MESSAGE") then
        local msg = select(0, ...);
    end
end);

Limitations would be that you can only place it in a parameter list and only as the last parameter.

@jdonaldson jdonaldson self-assigned this Jul 27, 2018
@jdonaldson jdonaldson added platform-lua Everything related to Lua enhancement labels Jul 27, 2018
@jdonaldson
Copy link
Member

I will look into a method for supporting this. It will require a lot of checks and restrictions as you note.

@Gama11
Copy link
Member

Gama11 commented Jul 27, 2018

I wonder if it would make sense to reuse haxe.extern.Rest for this instead of adding a new type? Might be confusing though, given that it's documented to only work for externs.

@jdonaldson
Copy link
Member

Yeah, I don't think overloading the Rest extern type is a good idea. However, I do think this feature intersects with the stack based tuple proposal. I think the best way forward in the short term is to have a special lua.Args type, and have genlua check for that and emit an ellipses whenever it shows up in the AST.

@YellowAfterlife
Copy link
Contributor

YellowAfterlife commented Jul 28, 2018

Rest had been lightly misuseable for a long time now. you can do

class Some {
public static var log:(tag:String, rest:haxe.extern.Rest<Dynamic>)->Void = Reflect.makeVarArgs(...);
}

and that will work.

Personal opinion is that a simple Rest (covering just array read[] and .length) should have been made accessible in non-extern types instead of adding Reflect.makeVarArgs to begin with, as all (?) targets support overloading and/or varargs natively, so the worst case scenario is having to account for rest-argument offset for targets with JS-style schema.

@vendethiel
Copy link

vendethiel commented Aug 7, 2018

Hey,

I'm also interested in this. My current solution involves this macro: https://github.com/vendethiel/haxewow/blob/master/Test.hx

@jdonaldson
Copy link
Member

jdonaldson commented Aug 20, 2018

Another workaround would involve using metadata on a function:

@:luaVarArgs
static function foo(x:Int) {
  trace(x);
  trace(lua.Lib.varargs);
}
foo(1,2,3); 
// 1
// {2,3}

This would generate a function signature with varags, and remove the possibility that the user screws the order up (varargs must be at end - currently the extern does not throw an appropriate error if that is not the case). This wouldn't really work with closures, and the varargs would be untyped.

@jdonaldson
Copy link
Member

On second thought, that would still require Reflect.makeVarArgs. I think we can do better.

@jdonaldson
Copy link
Member

I now have a test branch here : https://github.com/HaxeFoundation/haxe/tree/lua_varargs

I'm going with the "abuse haxe.extern.Rest<T>" approach. It looks like it works ok:

var f = function(x:Int, y : haxe.extern.Rest<String>) {
  trace(x);
  trace(lua.TableTools.pack(y));
}
[...]
f(1,'hi','ho');
//  1
// {1:'hi', 2 : 'ho'}

This still feels kind of off to me. I'll ask some of the other compiler folks about it and see if we can get proper support for it.

@henrikvik
Copy link
Author

here is my use case

work around

class EventManager {
    var frame : Frame;
    var callbacks : Map<String, Function>;

    public function new() {
        callbacks = new Map();
        frame = wow.G.CreateFrame(FRAME);
        frame.SetScript("OnEvent", untyped __lua__("function(_, e, ...) {0}.h[e](...); end", callbacks));
    }

    public function register(event: String, callback:Function) {
        frame.RegisterEvent(event);
        callbacks.set(event, callback);
    }

    public function unregister(event:String) {
        frame.UnregisterEvent(event);
        callbacks.remove(event);
    }
}

nicer usage

class EventManager {
    var frame : Frame;
    var callbacks : Map<String, Function>;

    public function new() {
        callbacks = new Map();
        frame = wow.G.CreateFrame(FRAME);
        frame.SetScript("OnEvent", on_event);
    }

    public function register(event: String, callback:Function) {
        frame.RegisterEvent(event);
        callbacks.set(event, callback);
    }

    public function unregister(event:String) {
        frame.UnregisterEvent(event);
        callbacks.remove(event);
    }

    function on_event(_:Void, e: Event, args: Rest<Dynamic>) {
        callbacks.get(e)(args...);
    }
}

@Gama11
Copy link
Member

Gama11 commented Mar 24, 2020

@jdonaldson Maybe it's time to merge that branch since it seems nobody found a better solution in the meantime? :) Or apply that approach so some new type in the lua package instead of haxe.extern.Rest.

@prnxdev
Copy link

prnxdev commented Mar 24, 2020

lua.VarArgs would be nice since there is cpp.VarArg, python.VargArgs etc.

@jdonaldson
Copy link
Member

It looks like the hack for haxe.extern.Rest doesn't work... the compiler won't let us use an extern type for haxe-generated code any longer.... most likely for good reasons.

I'll bring this up with the rest of the team, but it touches on a number of existing requested features. I don't want to do something half-baked for Lua if we can find another more general solution for other targets.

@jdonaldson jdonaldson changed the title [Feature Request / Lua] Type for handeling Lua var args "..." [Feature Request / Lua] Type for handling Lua var args "..." Mar 25, 2020
@RealyUniqueName RealyUniqueName mentioned this issue Mar 26, 2020
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement platform-lua Everything related to Lua
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants