Forward callbacks to all screens

[rm-code]

When I first wrote this library I just passed certain callbacks to all screens on the stack, but over time I realized that maybe for convenience it should forward all callbacks to all screens on the stack.

[pablomayobre]

I also thought about this, and a way to tell the screens down the stack if someone already consumed the event would be nice.

I also added some functions to my "fork" in Positive07/lovejam-training to call functions in the first stack or all the stacks

[rm-code]

In my current project the callback handling works fine the way it currently is, so I don't really have an idea how to improve it.

Of course we could simply break the event handling if one of the screens returned false, but I'm not sure how nicely that would work in more complicated setups.

for i = 1, #stack do
  if not stack[i]:xyz( ... ) then 
    break 
  end
end

On the other hand I don't want to over-complicate the library. I'm open for ideas :)

[pablomayobre]

I really like this idea by @s-ol

Basically you pass a first parameter to the callback which is a function that propagates the event down the stack

[pablomayobre]

I would implement it really simply like this:

local function propagator( callback, level, args )
   local state = stack[level]

   if not state then return end

   local propagate = function()
      return propagator( callback, level + 1, args )
   end

   return state[callback]( state, propagate, unpack( args ) )
end

Then in each callback you would do something like:

function ScreenManager.gamepadpressed( joystick, button )
    return propagator( "gamepadpressed", 1, { joystick, button } )
end

It creates a table per event which I couldn't avoid easily, but maybe you can do something like:

local function propagator (callback, level, a, b, c, d, e, f)
   local state = stack[level]

   if not state then return end

   local propagate = function()
      return propagator(callback, level + 1, a, b, c, d, e, f)
   end

   return state[callback](state, propagate, a, b, c, d, e, f)
end

function ScreenManager.gamepadpressed( joystick, button )
    return propagator("gamepadpressed", 1, joystick, button)
end

Since currently love.event.poll only returns 6 arguments per event

Note that the callbacks could also be:

function ScreenManager.gamepadpressed( ... )
    return propagator( "gamepadpressed", 1, ... )
end

[pablomayobre]

I have cleaned up the propagator function, now called delegate:

---
-- Delegates a callback to the stack, with the appropriate propagate function
-- callback is the callback name
-- a, b, c, d, e, f are the event arguments
-- level is the level of the stack to delegate to (defaults to the top)
--
local function delegate( callback, a, b, c, d, e, f, level )
    level = level or #stack
    local state = stack[level]

    if not state or type( state[callback] ) ~= "function" then return end

    local function propagate()
        return delegate( callback, a, b, c, d, e, f, level - 1 )
    end

    return state[callback]( state, propagate, a, b, c, d, e, f )
end

You could define all the callbacks by doing this:

for name in pairs( love.handlers ) do
    ScreenManager[name] = function ( ... )
        return delegate( name, ... )
    end
end

function ScreenManager.update( dt )
    return delegate( "update", dt )
end

function ScreenManager.draw()
    local values = { delegate( "draw" ) }

    ScreenManager.performChanges()

    return unpack( values )
end

Or alternatively:

function ScreenManager.draw()
    local values = { delegate( "draw" ) }

    ScreenManager.performChanges()

    return unpack( values )
end

return setmetatable( ScreenManager, { __call = function( self, callback, ... )
    return delegate(callback, ...)
end } )

And a usage example would be one like this:

local MainScreen = {}

function MainScreen.new()
    local self = {}

    local active = true

    function self:isActive()
        return active
    end

    function self:setActive( nactiv )
        active = nactiv
    end

    function self:draw( propagate )
        propagate() --We will draw after other screens in the stack
        --Here we draw our stuff
    end

    function self.mousepressed( propagate, x, y, button )
        if x > 100 and x < 700 and y > 100 and y < 500 then
            --We consume the event
        else
            --Since we didn't consume it we propagate it
            propagate()
        end
    end

    function self.keypressed( propagate, k )
        if k == "space" then
            --We consume the event
        end

        --We propagate anyway
        propagate()
    end
end

return MainScreen

Notes:

In the example I don't use Screen.lua, since the delegate function checks if the callback exists before trying to call it.

Also note that the metatable approach lets me define my own events so for example I could call ScreenManager.serverRequest( request ) and if the active state can handle it then it will be consumed otherwise nothing happens

delegate could propagate if the current state doesn't define the callback but I'm not sure if that is desirable

You can check the propagate branch in my fork

[rm-code]

I like the general approach, but I'm not a fan of the table allocation in the draw function local values = { delegate( "draw" )} because it produces a ton of garbage. I'm usually not too concerned with (over) optimisation until it becomes necessary, but I want to keep ScreenManager as lightweight as possible.

I understand why you are doing it, but I'd prefer something like this:

local a, b, c = delegate( 'draw' )

Of course the user would have to know which values his functions return, but that's something he should know anyway.

The other thing I'm concerned with (though maybe it isn't a problem ... I haven't been able to test it yet) is delegate( callback, a, b, c, d, e, f, level ). This would mean we could only pass six parameters to the functions. Why not use delegate( callback, level, ... ) instead? I'm thinking about situations where you want to pass custom values through the function.

[pablomayobre]

An ugly hack for the draw issue could be

local function vargsPerformChanges( ... )
    ScreenManager.performChanges()
    return ...
end

function ScreenManager.draw()
   return vargsPerformChanges( delegate( "draw" ) )
end

It could work hahaha

Of course the user would have to know which values his functions return, but that's something he should know anyway.

I'm not sure that I like the idea of ScreenManager assuming how many values I will return from my callback.

---

The a, b, c, d, e, f arguments are there to avoid the issue of varargs being undefined in propagate

local function delegate( callback, level, ... )
    level = level or #stack
    local state = stack[level]

    if not state or type( state[callback] ) ~= "function" then return end

    local function propagate()
        return delegate( callback, level - 1, ... ) --Errors! This function (propagate) doesn't take varargs
    end

    return state[callback]( state, propagate, ... )
end

The other alternatively is to allocate a table like I proposed first

local function delegate( callback, level, ... )
    local args = { ... }
    level = level or #stack
    local state = stack[level]

    if not state or type( state[callback] ) ~= "function" then return end

    local function propagate()
        return delegate( callback, level - 1, unpack( args ) )
    end

    return state[callback]( state, propagate, ... )
end

But I don't think you want that since you are trying to optimize the unneeded creation of tables.

So I proposed to use 6 arguments instead, which is the biggest quantity of arguments that love.event.poll can handle

Yet as I said I don't want ScreenManager to assume how many arguments a callback can take and that probably all callbacks should be defined like:

function ScreenManager.draw( ... )
    return delegate( "draw", ... )
    -- Of course there is a performChanges here
end

This way I can do stuff like:

love.draw = function ()
    ScreenManager.draw( "something", 123 )
end

[rm-code]

An ugly hack for the draw issue could be

Haha :D I think it's better than creating a ton of tables.

I'll try to sit down this weekend and give your version a try. Right now I'm basically discussing just from looking at the code and a quick test I wrote today.

So maybe this is a stupid question, but what are the benefits of your delegate / propagate approach compared to my original suggestion of simply using a return value. I just like simple solutions and by default LÖVE callbacks don't have return values IIRC. That's why I'd do this:

function ScreenManager.update( dt )
    local consumed;
    for i = #stack, 1, -1 do
        consumed = stack[i]:update( dt )
        if consumed then
            break;
        end
    end 
end

By default the user wouldn't have to change his current screens since nil would mean the event isn't consumed. So the user could do something like this:

function self:update( dt )
    if coolStuffIsHappening then
        -- Do stuff
        -- ...
        -- then consume the event
        return true
    end
end

[pablomayobre]

Your example has a fixed order, it traverses the stack in a for loop from the top of the stack to the bottom. You cannot change the order of the stack while in the loop or that may shield undefined behaviour.

Still your example can be implemented with delegate/propagate like this:

function self:update( propagate, dt )
    if coolStuffHappening then
        -- Do stuff
    else --We didn't consume the event so we propagate
        return propagate()
    end
end

But you could also do something like this:

function self:update( propagate, dt )
    local value = propagate()

    if value then
        -- Do something
    end
end

This basically means that the current state updates AFTER the state underneath it, and depending on the value returned by it, it performs an action or not.

There is no way to implement this with the for loop approach without changing ScreenManager's code.
And even then, this approach allows you to change the order per-screen so the order is up to you

[pablomayobre]

Any new thoughts on this?

[rm-code]

Any new thoughts on this?

I see the benefits of it, but I'm not willing to merge the current implementation into ScreenManager as it produces too much garbage for my taste.

[rm-code]

I'll keep this open, but for now I'll add the simple consume feature I talked about earlier:

for i = #stack, 1, -1 do
    local consumed = stack[i]:foo( xyz )
    if consumed then
        break;
    end
end

I know this isn't the perfect solution, but it's better than what we currently have.