Stack overflow

[Benalish]

May I ask why this code goes into stack overflow

local function SetWidgetScript(frame,method)
    if frame:IsShown() then
        for i = 1, #frame.buttons do
            frame.buttons[i]:HookScript("OnMouseDown", method)
        end
    else
        frame:SetScript(
            "OnShow", --when "OnShow" is fired, IsShown() become true
                SetWidgetScript(frame,method)
        )
    end
end

While it works fine by simply wrapping myfunc() inside the function environment after "OnShow"?

local function SetWidgetScript(frame,method)
    if frame:IsShown() then
        for i = 1, #frame.buttons do
            frame.buttons[i]:HookScript("OnMouseDown", method)
        end
    else
        frame:SetScript(
            "OnShow", --when "OnShow" is fired, IsShown() become true
            function()
                SetWidgetScript(frame,method)
            end
        )
    end
end

[SDPhantom]

The first example, you're calling yourself instead of registering a function to run as a script. What happens when you make a function call as an argument for another function is it runs the function call immediately and uses the return value(s) as the argument(s) for the second function. In this case, you're calling yourself (SetWidgetScript()) and :SetScript() is expecting to take any returns to use for the callback script. SetWidgetScript() doesn't actually return anything, so if it wasn't repeatedly calling itself, causing the stack overflow, it would end up sending nil to :SetScript().

The second example creates a dynamic function to call SetWidgetScript() with the arguments supplied as upvalues. Unlike the first example, you aren't calling it immediately as you are just giving it a function.



It's not exactly the way I'd design this function either way as trying to call it to register multiple callbacks on the same frame will cause the previous OnShow registrations to be lost. Also the OnShow script never unregisters itself, so it'll just keep calling itself whenever it gets fired, which will keep piling :HookScript() calls on top of each other.

Mouse scripts require a "visible" frame to fire, so there's no need to delay hooking until it's shown anyway.

[Benalish]

The original code was

local function SetWidgetScript(frame,method)
    if frame:IsShown() then
        for i = 1, #frame.buttons do
            frame.buttons[i]:HookScript("OnMouseDown", method)
        end
    else
        frame:SetScript(
            "OnShow", function()
                 for i = 1, #frame.buttons do
                     frame.buttons[i]:HookScript("OnMouseDown", method)
                 end
            end
        )
    end
end

I changed the method in the "OnShow" script just to not repeat the for i=1...end part twice.
There is probably a better way to write a DRY script.

[SDPhantom]

In all honestly, I rather use buttons than generic frames to receive clicks. As I said earlier, mouse events require the frame to be "visible" already to fire, they shouldn't if they're hidden. This is just over-engineered.

It should be this simple.

local function SetWidgetScript(frame,method)
    for _,button in ipairs(frame.buttons) do
        button:HookScript("OnMouseDown",method);
    end
end