[lua] Bad code gen when referencing static functions from classes in other files.

[shakesoda]

In certain situations involving arrays of references to static functions with array parameters (or at least that's how I ran into it), bad code is generated in the Lua target.

local systems = _hx_tab_array({[0]=_hx_o({__fields__={update=true},update=function(_,...) return Works.update(...) end}), _hx_o({__fields__={update=true},update=Bork.update}) }

Note how Bork.update isn't wrapped in the function that throws out self - this only happens when Bork is defined in a different file (and even then, not always - I had several of these working before running into it...).

This causes the following error:

BadFunctionRefs.hx:16: { foo : 0 }
BadFunctionRefs.hx:16: { foo : -1 }
lua: test.lua:135: attempt to compare number with nil
stack traceback:
    test.lua:135: in function 'update'
    test.lua:128: in function 'main'
    test.lua:363: in main chunk
    [C]: ?

Works remains working so long as it is in the same file AFAICT.

Here's the test case I came up. As described, fails when class Bork is in Bork.hx, succeeds when it's in BadFunctionRefs.hx.

// BadFunctionRefs.hx
// Tested bad on Haxe 3.4.2 and Lua 5.1.5. Also tested on nightly (2017-06-15)
// $ haxe -main BadFunctionRefs -lua test.lua && lua test.lua

typedef Entity = {
    var foo: Float;
}

typedef System = {
    var update: Array<Entity>->Void;
}

class Works {
    public static function update(entities: Array<Entity>) {
        for (e in entities) {
            trace(e);
            e.foo += 0.5;
        }
    }
}

/*
// if this is in BadFunctionRefs.hx, it works.
// if you put it into Bork.hx, crash.
//import BadFunctionRefs.Entity;

class Bork {
    // note: only fails with an array here
    public static function update(entities: Array<Entity>) {
        for (e in entities) {
            trace(e);
            e.foo += 0.5;
        }
    }
}
*/

class BadFunctionRefs {
    static function main() {
        var entities = [
            { foo: 0.0 },
            { foo: -1.0 }
        ];
        var systems = [
            // this will work
            { update: Works.update },
            // this will fail
            { update: Bork.update },
        ];
        for (system in systems) {
            system.update(entities);
        }
    }
}

Hopefully enough here to get it fixed, I had to dig for quite a while to get that test to reliably fail.

[jdonaldson]

Thanks for the repro for this.

[karai17]

I am still running into this issue in latest nightlies. My static member function in a separate file has two arguments, the luagen code is directly assigning that function to my table and then executing it with a colon.

genfunc = function(a, b)
   return
end

x = {
   fn = genfunc
}

x:genfunc(1, 2)

This seems to happen in some cases, but not all cases. In other cases, it is generating:

x = {
   fn = function(_, ...) genfunc(...) end
}

The only difference I can see between these two sets of functions is that the ones that get wrapped do not return whereas the ones that don't get wrap have a return. Perhaps that might be the issue?

[jdonaldson]

Could I get another repro for your specific case?

[karai17]

Alright so after some investigation, here is what I have come up with:

// Main.hx
class Main {
    public static function main() {
        Tast.a(1, 2);
        Tast.b.a(3, 4);
    }   
}

// Test.hx
class Test {
    public static function a(a: Int, b: Int) {
        trace(a, b);
    }
}

// Tast.hx
class Tast {
    public static var a = Test.a;
    public static var b = {
        a: Test.a
    };
}

// main.lua
...

Main.new = {}
Main.main = function() 
  Tast.a(1,2);
  Tast.b:a(3,4);
end

...

local _hx_static_init = function()
  lua.Boot.hiddenFields = {__id__=true, hx__closures=true, super=true, prototype=true, __fields__=true, __ifields__=true, __class__=true, __properties__=true}
  Tast.a = Test.a
  Tast.b = _hx_o({__fields__={a=true},a=function(_,...) return Test.a(...) end})

end

...

You will note that if I assign a static function from another file, it wraps that function within a function as opposed to directly assigning it. In some cases that I can't quite figure out, this function is then executed without a colon so that we lose the first variable causing us to need a padding variable.. but only sometimes! We've found that some functions that are seemingly identical in all meaningful ways end up with a colon call and others with a period call, causing us to have to put the following ugly line in all of our functions to ensure that they work:

public static function a(_: Dynamic, a: Int, b: Int) {
    untyped __lua__("if not b then b = a; a = _ end"); // HACK

    ...
}

[jdonaldson]

The example you gave looks like it's working correctly in my latest build.

Static methods should be called with a standard dot accessor. Anything that looks like an "instance" method (such as the anonymous object you set up for Tast.b) will be called with a colon. The compiler should be inserting dummy "self" arguments for those.

I hope you can help pin this down a bit more, the ugly line hack looks terrible!

[karai17]

I can point you to our whole code base if that may help. I am having trouble really narrowing down the issue where it is using the incorrect self.

https://github.com/excessive/ludum-dare-39 files /src/Script.hx, /src/Pattern.hx, src/Emit.hx, and src/stages/Stage01.hx are the relevant files to look at.

[jdonaldson]

Are you guys still seeing this problem? I'm only seeing your hack now in the Patterns.hx file, with a note that it was fixed in Haxe 4.

[karai17]

This does still happen, yes. We have a couple files with hacks, one of which is fixed in haxe 4, the other is not. The really ugly one is still broken.

[jdonaldson]

I haven't been able to pin this down, butI fixed a related issue here recently : https://github.com/HaxeFoundation/haxe/issues/6722

[karai17]

I'll make another attempt at reducing this.

[karai17]

Haxe -> Lua Weirdness

Let's step through some code.

Pattern Defined

This is where we begin our adventure. In our Pattern class located in src/Pattern.hx we define functions that take in an Emitter object (/src/components/Emitter.hx) and an Int to do stuff. In the case of explode_pulse, it updates the ring and velocity properties of the Emitter. Note that this function is a public static function in a class in an external file.

class Pattern {
    public static function explode_pulse(p: Emitter, i: Int) {
        var ring: Int = Math.floor(i / p.spawn_rate);

        if (p.user_data.alt == true && p.user_data.ring < ring) {
            p.velocity = p.velocity.rotate(Math.PI, Vec3.up());
        }

        p.user_data.ring = ring;
        p.velocity = p.velocity.rotate(Math.PI * 2 / p.spawn_rate, Vec3.up());
    }

The generated Lua generally looks fine, I see no cause for concern here.

Pattern.new = {}
Pattern.__name__ = {"Pattern"}
Pattern.explode_pulse = function(p,i) 
  local ring = _G.math.floor(i / p.spawn_rate);
  if ((p.user_data.alt == true) and (p.user_data.ring < ring)) then 
    p.velocity = __math__Vec3_Vec3_Impl_.rotate(p.velocity, _G.math.pi, __math__Vec3_Vec3_Impl_._new(0, 0, 1));
  end;
  p.user_data.ring = ring;
  p.velocity = __math__Vec3_Vec3_Impl_.rotate(p.velocity, (_G.math.pi * 2) / p.spawn_rate, __math__Vec3_Vec3_Impl_._new(0, 0, 1));
end

Emitter is Created

In the Emit class (src/Emit.hx), we create Emitter object templates using the make helper function. Our example template here is Pulse which has an update property assigned the value of our Pattern from above. You will note that make is a public static function and Pulse is a public static variable.

class Emit {
    public static function make(params: Dynamic): Emitter {
        var limit = params.limit != null? params.limit : 1000;

        return {
            name: params.name,
            batch: Lg.newSpriteBatch(params.image, limit, SpriteBatchUsage.Stream),
            color: params.color != null? params.color : new Vec3(Lm.random(0, 100) / 100, Lm.random(0, 100) / 100, Lm.random(0, 100) / 100),
            data: new ParticleData(),
            limit: limit,
            lifetime: params.lifetime != null? { min: params.lifetime, max: params.lifetime } : { min: 15, max: 15 },
            pulse: params.pulse != null? params.pulse : 0.0,
            spawn_radius: params.spawn_radius != null? params.spawn_radius : 0,
            spawn_rate: params.spawn_rate != null? params.spawn_rate : 10,
            spread: params.spread != null? params.spread : 0.0,
            offset: params.offset != null? params.offset : new Vec3(),
            scale: 1,
            radius: params.radius != null? params.radius : 0.15,
            emitting: false,
            velocity: params.velocity != null? params.velocity : new Vec3(0, 0, 0),
            user_data: params.user_data,
            update: params.update
        };
    }

    public static var particles = {
        arrow:    Lg.newImage("assets/textures/arrow.png"),
        particle: Lg.newImage("assets/textures/particle.png"),
        bullet:   Lg.newImage("assets/textures/bullet.png"),
        orb:      Lg.newImage("assets/textures/orb.png"),
        petal:    Lg.newImage("assets/textures/petal.png"),
        feather:  Lg.newImage("assets/textures/feather.png"),
        card:     Lg.newImage("assets/textures/card.png"),
        thorn:    Lg.newImage("assets/textures/thorn.png")
    };

    public static var colors = {
        sakura_l:    new Vec3(1.00, 0.47, 0.80),
        sakura_r:    new Vec3(1.00, 0.80, 0.93),
        rose_l:      new Vec3(0.35, 0.00, 0.00),
        rose_r:      new Vec3(0.88, 0.00, 0.00),
        sky_blue:    new Vec3(0.00, 0.54, 1.00),
        pale_yellow: new Vec3(1.00, 1.00, 0.65),
        thorn:       new Vec3(0.10, 0.57, 0.00),
        red:         new Vec3(1.00, 0.00, 0.00),
        green:       new Vec3(0.00, 1.00, 0.00),
        blue:        new Vec3(0.00, 0.00, 1.00)
    };

    public static var pulse = {
        image:      particles.orb,
        pulse:      0.75,
        spawn_rate: 19,
        update:     Pattern.explode_pulse,
        user_data:  { alt: true, ring: 0 },
        velocity:   new Vec3(0.025, 2.0, 0)
    };

Here is the first area of concern for me. Emit.Pulse is being wrapped in a function and then executed to get our object data. This is weird to me, but not exactly wrong. What is notable here is that the update property is an anonymous function wrapping our Pattern, discarding the first argument. This basically means that the Lua should be executing self:update and self would not be passed into Pattern.

  Emit.pulse = (function() 
    local _hx_16

    local tmp = Emit.particles.orb;

    local tmp1 = __math__Vec3_Vec3_Impl_._new(0.025, 2.0, 0);

    _hx_16 = _hx_o({__fields__={image=true,pulse=true,spawn_rate=true,update=true,user_data=true,velocity=true},image=tmp,pulse=0.75,spawn_rate=19,update=function(_,...) return Pattern.explode_pulse(...) end,user_data=_hx_o({__fields__={alt=true,ring=true},alt=true,ring=0}),velocity=tmp1});
    return _hx_16
  end )();

And that is what we see in our update_emitter function in the ParticleUpdate system. particle:update is passing particle in twice and discarding the first one. So this is fine, if not a little redundant.

__systems_ParticleUpdate.prototype.update_emitter = function(self,transform,particle,dt) 
  local pd = particle.data;
  local spawn_delta = __systems_ParticleUpdate.time - pd.last_spawn_time;
  local count = pd.particles.length;
  if (particle.pulse > 0.0) then 
    if (((count + particle.spawn_rate) <= particle.limit) and (spawn_delta >= particle.pulse)) then 
      local _g1 = 0;
      local _g = particle.spawn_rate;
      while (_g1 < _g) do 
        _g1 = _g1 + 1;
        local i = _g1 - 1;
        self:spawn_particle(transform, particle);
        if (particle.update ~= nil) then 
          particle:update(particle, pd.index);
        end;
      end;
    end;
  else
    local rate = 1 / particle.spawn_rate;
    if ((count < particle.limit) and (spawn_delta >= rate)) then 
      local need = Std.int(_G.math.min(2, _G.math.floor(spawn_delta / rate)));
      local _g11 = 0;
      local _g2 = need;
      while (_g11 < _g2) do 
        _g11 = _g11 + 1;
        local i1 = _g11 - 1;
        self:spawn_particle(transform, particle);
        if (particle.update ~= nil) then 
          particle:update(particle, pd.index);
        end;
      end;
    end;
  end;

Define Scripts

This section of the game is used to define the AI. Timer.script is a wrapper for Lua coroutines. In this example, we have our Script class located at src/Script.hx. Each Script takes in an Entity and an array of Entitys that we classify as a wave. In the example Script, straight_down_slow, we are just setting the emitting flag on our Entity to true so that the Enemy's internal Emitter will activate, enabling the Pattern function from above. We also set its velocity, wait 4 seconds for it to scroll from the top to the bottom of the screen, then we clear then Enemy from the wave. Note that this is a public static function in a class in an external file.

class Script {
    public static function straight_down_slow(e: Entity, w: Array<Entity>) {
        Timer.script(function(wait) {
            for (em in e.emitter) { em.emitting = true; }

            e.transform.velocity.y = 5;
            wait(4);

            return Signal.emit("enemy clear", { wave:w, entity:e });
        });
    }

As for the generated Lua, it looks fine. Except there is actually an error here. When an enemy spawns, the game crashes and I get the following error:

Error: libs/timer.lua:93: main.lua:1644: attempt to index local '_g1' (a nil value)

Uh oh. So it looks like whatever we are passing in to e is not an Entity like we have written in the Haxe code. Let's keep going.

Script.new = {}
Script.__name__ = {"Script"}
Script.straight_down_slow = function(e,w) 
  __timer_Timer.script(function(wait) 
    local _g = 0;
    local _g1 = e.emitter;
    while (_g < _g1.length) do 
      local em = _g1[_g];
      _g = _g + 1;
      em.emitting = true;
    end;
    e.transform.velocity[2] = 5;
    wait(4);
    Signal.emit("enemy clear", _hx_o({__fields__={wave=true,entity=true},wave=w,entity=e}));
    do return end;
  end);
end

Particle Emitter is created

Here is where the flow of the game is defined. In src/stages/Stage01.hx we create the Stage01 class and have a really big public static init function that defines the waves in our game. Basically, a wave is just a list of Entitys that have delays for when they appear on screen and their Emitters are enabled. Once a wave is emptied, the next one is loaded, ad nauseum, until our boss wave is loaded and cleared, at which point the game is over. In this game, an Entity is defined by having several components, most of which are optional. In our case, we want Enemy Entitys which have the following components:

• enemy
• emitter
• drawable
• transform
• capsules

The two we are most interested in are enemy and emitter. enemy has a property called script that is assigned the value of a Script function, defining this enemy's AI. emitter is an array filled with 1 or more Emitter objects generated by our make helper function we discussed earlier.

class Stage01 {
    public static function init(): StageInfo {
        var waves: Array<Array<Entity>> = [
            [
                {
                    enemy: {
                        id:     Trash,
                        attach: [],
                        hp:     1,
                        max_hp: 1,
                        alive:  true,
                        delay:  3,
                        iframe: false,
                        speed:  10,
                        script: Script.straight_down_slow
                    },
                    emitter: [
                        Emit.make(Emit.pulse)
                    ],
                    drawable: new Drawable("assets/models/bear.iqm"),
                    transform: new Transform(new Vec3(0, -11, 0), new Vec3(), Quat.from_angle_axis(Math.PI, Vec3.up())),
                    capsules: {
                        push: [],
                        hit:  [],
                        hurt: [
                            new BoneCapsule(null, 1.5, 0.5)
                        ]
                    }
                }
            ],

Now here is something interesting. When we check the generated Lua, we find that script is being assigned directly, not wrapped in a function like our Emitter's update function that wrapped the Pattern function and dropped the first argument. This means that for this function to execute properly, it needs to behave differently. It needs to execute as self.script and not pass in an unwanted self argument.

__stages_Stage01.new = {}
__stages_Stage01.__name__ = {"stages","Stage01"}
__stages_Stage01.init = function() 
  local waves = _hx_tab_array({[0]=Emit.make(Emit.pulse)}, 1);
  local waves1 = __components_Drawable.new("assets/models/bear.iqm");
  local waves2 = __components_Transform.new(__math__Vec3_Vec3_Impl_._new(0, -11, 0), __math__Vec3_Vec3_Impl_._new(), __math__Quat_Quat_Impl_.from_angle_axis(_G.math.pi, __math__Vec3_Vec3_Impl_._new(0, 0, 1)));
  local waves3 = _hx_tab_array({[0]=_hx_o({__fields__={enemy=true,emitter=true,drawable=true,transform=true,capsules=true},enemy=_hx_o({__fields__={id=true,attach=true,hp=true,max_hp=true,alive=true,delay=true,iframe=true,speed=true,script=true},id=EnemyId.Trash,attach=_hx_tab_array({}, 0),hp=1,max_hp=1,alive=true,delay=3,iframe=false,speed=10,script=Script.straight_down_slow}),emitter=waves,drawable=waves1,transform=waves2,capsules=_hx_o({__fields__={push=true,hit=true,hurt=true},push=_hx_tab_array({}, 0),hit=_hx_tab_array({}, 0),hurt=_hx_tab_array({[0]=__components_BoneCapsule.new(nil, 1.5, 0.5)}, 1)})})}, 1);

Oh. Well that explains our error from above! It looks like our enemy component is passing itself in as the first argument to our function, which definitely explains why our first argument is not the Entity, it's being pushed down as the second, and our third argument is being dropped altogether.

Stage.new_wave = function(t) 
  local w = Stage.waves[Stage.wave];
  local _g = 0;
  while (_g < w.length) do 
    local e = _hx_tab_array({[0]=w[_g]}, 1);
    _g = _g + 1;
    __timer_Timer.after(e[0].enemy.delay, (function(e1) 
      do return function(t1) 
        World.entities:push(e1[0]);
        e1[0].enemy:script(e1[0], w);
      end end;
    end)(e));
  end;
end

Now, I have no sweet clue why this is happening. All of the Haxe files we've looked at are external files with public static functions, yet for some reason, some of these are being assigned directly and others are being wrapped. Hopefully this walk through sheds a little light on what is going on, because this is a pretty big bug.

[jdonaldson]

Wow, thanks. I'll take a look and see where this is going sideways.

[karai17]

It is worth noting that for this test I removed all my little hacks and used the latest commit from the haxe build site, so these results represent the current state of haxe 4.

On Nov 5, 2017 9:50 PM, "Justin Donaldson" notifications@github.com wrote:

Wow, thanks. I'll take a look and see where this is going sideways.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/HaxeFoundation/haxe/issues/6368#issuecomment-342026013, or mute the thread https://github.com/notifications/unsubscribe-auth/AAkB_JpjxP7Dj0MATzdsnQ0Tg1XXbDfeks5szmXkgaJpZM4N7_-R .

[karai17]

Any updates on this issue? It would be nice to see this fixed before haxe 4 is released.

[jdonaldson]

Nothing yet sadly, I'm still looking.

[jdonaldson]

Can I see the typedef you're using for Entity?

[karai17]

https://github.com/excessive/ludum-dare-39/blob/master/src/Entity.hx

[jdonaldson]

Try changing the type of the script field type to a function signature.

package components;

typedef Enemy = {
    var id:     EnemyList.EnemyId;
    var hp:     Float;
    var max_hp: Float;
    var alive:  Bool;
    var delay:  Float;
    var script: Entity->Array<Entity>->Void;  // <= updated line
    var iframe: Bool;
    var speed:  Float;
    var attach: Array<Entity>;
    @:optional var boss: Bool;

Lua has no notion of a static/instance method. Haxe has to keep track of this manually at compile time. When you use Dynamic to refer to a function type, you're preventing the compiler from remembering which invocation style to use (e.g. function(self,...) or function(...)). It defaults to calling the function as-is.

I'm pretty sure that will fix this particular issue. I realize in Lua it's common to plug together functions into loosely defined objects, but to do so in Haxe compiled Lua you have to be very specific with your typing, and also willing to incur a bit of overhead with function wrapping.

[jdonaldson]

I'm going to close this out, feel free to reopen if I'm not being clear.