WowAce

We've encountered a problem using this library with Skada, which is summarized in this ticket

In a nutshell, all of Skada's code uses libWindow in the "canonical" fashion (parented to UIParent) and when running in a stand-alone configuration everything works beautifully. However, some UI replacement mods (nUI and possibly ElvUI) like to reparent the Skada window (outside our control), in order to stick it onto their own UI panel. Then later when Skada calls LibWindow to manage position, the library silently assumes UIParent is in use and breaks in horrible ways, eg see comment from lib.SavePosition():

function lib.SavePosition(frame)
        local parent = frame:GetParent() or nilParent
        -- No, this won't work very well with frames that aren't parented to nil or UIParent
        local s = frame:GetScale()
        local left,top = frame:GetLeft()*s, frame:GetTop()*s
        local right,bottom = frame:GetRight()*s, frame:GetBottom()*s
        local pwidth, pheight = parent:GetWidth(), parent:GetHeight()

The comment only partially correct - in the case where the parent has been set to nil, :GetLeft(), :GetTop() and GetScale() all return nil (this might be a recent FrameXML change?) and the library subsequently generates a lua error (arithmetic on a nil value). When the parent has been set to a non-nil frame which is not UIParent, the library leads to the frame to jumping around in bizzare ways.

Ideally would like to see LibWindow made smart enough to detect when the parent window is NOT UIParent, and behave in an intelligent manner when this baked-in assumption is violated. At the very least it should avoid generating lua errors when the position/scale queries return nil. Better yet, the library would detect the reparenting and stub itself out entirely, under the assumption that some other mod has taken explicit control of position.

I would concur with the last statement.

"Better yet, the library would detect the reparenting and stub itself out entirely, under the assumption that some other mod has taken explicit control of position."

When nUI takes control of Skada (and thus generates this issue) - it doesn't want someone else trying to move it around. I reparented it so I had control over position, scale, size, visibility, etc. Having a library assume a window will never be reparented, or that no one else will ever seek to modify size/scale/position is probably a bad idea. jmo

Hrmph.. what I actually wanted to do originally was to make the library smarter about frames parented to something other than UIParent. I.e. handle relative positioning. But I never got around to it.

So.. what happens here? The parent is actually SET to nil? That could indeed be an indicator that LibWindow should just leave the window the hell alone.

Does it get changed to other things, too? What would correct behavior be, then? It seems a bit random to me that the library should just disable functionality without the original embedder being aware.

Let's step back a moment. Currently Skada's usage of libwindow amounts to:

1. one call to lib.RestorePosition() when a BarDisplay is first created, which usually happens once per window at startup (although there are other corner cases like on a profile change). I believe this usually or always happens BEFORE nUI or other addons would have the opportunity to re-parent the window

2. multiple calls to lib.SavePosition() whenever the user moves or re-scales the window, or other Skada settings change that might affect the window. This call can often occur AFTER re-parenting.

I think the immediate problem we're facing is that re-parented windows are generating lua errors inside the lib.SavePosition calls. This call semantically shouldn't perform any window movement at all, just query position and save internal state.

Regardless of what semantics we decide upon for re-parented windows, I suspect fixing the lib.SavePosition code to carefully avoid lua errors regardless of parent may solve 90% to 100% of our immediate problem for Skada+nUI (and hopefully we agree that defensive coding to avoid lua errors here is a Good Thing).
 
I notice the SavePosition code currently resets the window Points, and that may also need to be stubbed out to prevent unintended window movement (or it might just magically work with a reparented window).

A separate issue is where RestorePosition places a UIParent-parented window if the last call to SavePosition was made with a non-UIParent parent, but for the purposes of Skada I'm not too worried about the answer to that question (provided no lua errors are generated). For the case of Skada+nUI I think the answer is irrelevant, because nUI will subsequently move the window to the correct position anyhow. If the user suddenly stops using nUI his Skada window might appear in a strange place on his next reload, but I'm also not too worried about that rare situation. So we don't have to solve that question of library semantics in order to fix the immediate problem.

@mikk: Go

Micc --

nUI does not parent the window to nil. It creates a "container" window that is used to control the integrated addon (Skada in this case), reparents the integrated addon to the container window, and then manipulates the container. This is done so as to (theoretically) not have to mess with the other addon's windows (and potentially break it).

@oscarucb: Go

FYI - nUI creates two new Skada windows and does not alter the original (default) Skada window (other than to hide it). So, at least for the default window, if a user quits using nUI, then they would just delete the two Skada windows nUI created and be done with it.

EDIT: And, in fact, I think I'll alter nUI to re-show the default window and delete the two "new" windows when the user logs out - that will restore Skada to its default state if the user quits using nUI (it's the same thing I do for key bindings and a few other elements).

@spiel2001: Go

I'm a bit unsure what the problem actually is here, and frankly I'm finding it hard to get around to installing nUI so I can test my fixes.

Is there any way YOU could try and plug the problem and send the updated file/patch and/or just commit it to repo so I can review?

I'll have a look when I can.