Tagging @Mudlet/core-cpp for review

@ahmedcharles I'm not sure if you get notified when I push commits, tagging you just in case.

The more I look into the code here, the more I realize it's more complex than it seems.

1. One problem is that the lua api closeUserWindow calls hideWindow, not closeWindow. This is a simple enough change that we should make it independently.
2. Another problem is that the maps that we use to track windows don't get updated when we close the window. There are two ways for a window to be closed, the user hitting the X or code calling close() on that window (granted, the user hitting X probably just causes close() to be called on that window). Anyways, that cases closeEvent to get called. So, calling closeWindow from closeEvent doesn't make sense, since that would result in a loop. Qt probably guards against this, though we really shouldn't abuse that. Ideally, we would have a closeWindowEvent function which removed the window from the maps, instead of doing it in the current, closeWindow function.
3. Another problem is that closeWindow closes the console window which is inside the QDockWidget rather than closing the QDockWidget directly. But since `closeWindow is never called, it was never tested.
4. Another problem is that hideUserWindow seems to apply to a completely different set of 'windows' than 'openUserWindow' and 'closeUserWindow'. Granted, closeUserWindow doesn't correctly hide the window currently, either. That's because hideWindow calls hide on the console, not the QDockWidget.
5. Another problem is that closing a Host (or profile, as it were) doesn't close the user windows which will no longer work, though this is more of a limitation of the current architecture, which makes doing this difficult.

I stopped looking after finding 5 things. The current approach in the PR seems to work, but I don't think it will fix all of the issues here. Let's take a step back and try and fix them one at a time, if that's ok?

Let me know if you'd like any help. 1 and 3 are pretty straight forward. 2 requires TDockWidget but it would call a different function.

Oh, apparently hideUserWindow and showUserWindow work on mini-consoles, which are created with createMiniConsole.

<sarcasm>I'm truly excited now.</sarcasm>

Okay a bit of history before we proceed - user windows were created first. Then we abandoned them, because building a out of them completely would have sucked due to all the window borders, so we left them be as-is and made miniconsoles instead - a much more nicely embeddable thing. User windows of course still have their purpose for example you can drag them to their own monitor.

The API has moved in a direction of re-using functions for similar stuff after that - so for example showWindow/hideWindow both operate on a label and a miniconsole. Given how closeUserWindow didn't work before - since we're reanimating userwindows, I'd prefer to bring them into alignment with miniconsoles and labels instead of having them by the side - so 1) could we have hideWindow work on them?

The other thing as you might notice is that there is no closeWindow for labels or miniconsoles. That's by design - efficiency reasons. Wouldn't be against adding it in for people who'd really want it, but making that entire mechanism work would be a pretty huge scope of work.

To solve the original problem of you not being able to re-open a userwindow after it has been closed, how about we 2) make the X on the widget redirect to hiding the window, so that creating/showing it will make it show up again?

I suppose that makes sense, sort of... I'll have a think on it for a bit.

... bit late to the party but are we talking about the dock-able windows that can be conveniently moved to a different monitor when you are on a multi-headed PC?✽ That being the case you can see the dockable widgets in the default context (right-click) menu on the Title-bar of any dock-widget or on any toolbar - note that we still haven't gotten around to putting up "names" (or ideally "type - profile - name" to handle multi-playing set-ups) for all items yet, including the main toolbars...! See how the "comms" window is handled below:

FTR The grey item is the (docked but float-able) toolbar with manual map movement direction controls - floating toolbars are not hidable/showable via the built-in UI so the checkbox is greyed out and we do not (currently) show a title on them which really breaks in a multi-playing setup as you do not know which user toolbar works which profile... 😒

✽ = {That reminds me I really must have another go at asking Matrox to update their Linux drivers, the latest kernel their binary blob driver with open source API supports is 3.10 😭 so is now unusable and I cannot yet get back to using a display card and a hydra system with a set of 4 monitors!}

I'm getting a crash in when I re-open a profile and try to call open openUserWindow() again:

1   QFontMetrics::QFontMetrics(QFont const&)                                   0x7ffff5cc6880 
2   TTextEdit::updateScreenView                             TTextEdit.cpp 250  0x73f7fb       
3   TTextEdit::showEvent                                    TTextEdit.cpp 1567 0x74758c       
4   QWidget::event(QEvent *)                                                   0x7ffff6747738 
5   QApplicationPrivate::notify_helper(QObject *, QEvent *)                    0x7ffff6701bdc 
6   QApplication::notify(QObject *, QEvent *)                                  0x7ffff67088d0 
7   QCoreApplication::notifyInternal2(QObject *, QEvent *)                     0x7ffff530b0b0 
8   QWidgetPrivate::show_helper()                                              0x7ffff674457b 
9   QWidgetPrivate::showChildren(bool)                                         0x7ffff6744480 
10  QWidgetPrivate::show_helper()                                              0x7ffff6744552 
11  QWidgetPrivate::showChildren(bool)                                         0x7ffff6744480 
12  QWidgetPrivate::show_helper()                                              0x7ffff6744552 
13  QWidgetPrivate::showChildren(bool)                                         0x7ffff6744480 
14  QWidgetPrivate::show_helper()                                              0x7ffff6744552 
15  QWidgetPrivate::showChildren(bool)                                         0x7ffff6744480 
16  QWidgetPrivate::show_helper()                                              0x7ffff6744552 
17  QWidgetPrivate::showChildren(bool)                                         0x7ffff6744480 
18  QWidgetPrivate::show_helper()                                              0x7ffff6744552 
19  QWidgetPrivate::showChildren(bool)                                         0x7ffff6744480 
20  QWidgetPrivate::show_helper()                                              0x7ffff6744552 
... <More>                                                                                    

To reproduce:

1. make a new script, put the following into it:

function userwindowTest()
  openUserWindow("dockedwindow")
  print("created docked window")
  tempTimer(1, function() closeUserWindow("dockedwindow") print("hid docked window") end)
  tempTimer(3, function() showWindow("dockedwindow") print("showed docked window") end)
end

userwindowTest()

2. let it open/close a window
3. close profile
4. open profile and run test again -> crash

Let us know if you'd like help in figuring out the problem!

This probably needs closer testing and review. I've poured over the miniconsole and I'm still not sure how it works, or if it works. I tried using it in 2.1 and got the same results so I'm almost sure I'm doing it wrong. I'm hopeful this code wont break anything in those regards. I tested the lua functions and have not generated a crash.

Edit: DockWidgets aren't properly host-mapped. heh. I'll be patching that shortly.

Hey we broke your stuff with some formatting changes in mudlet.h but I've fixed it up for you - pull again.

Sorry I should have done a merge locally but spaced. Thanks for fixing it.

Not sure I am clear on whether the Host mapping will do what I think it might. If I have two profiles loaded and running and I create a mini-console called "info" in one profile, can I create the same named one in the other profile? Maybe that is a side-issue and not entirely within scope of this PR, but I am aware that I cannot do that in the existing code (or as it was the last time I looked at it) i.e. #842 .

Yes, you should be able to create a window of the same name in two profiles.

There is an issue still with creating both a Miniconsole and a DockWidget console of the same name, since they both occupy the same console map. I had a fix in mind for that (some 'simple' name prefixing within the console map could do it.) but the issue is almost a non-issue and could be patched later if needed.

Vadi has found a crash with this code, related to closing a profile with docked consoles open. The dockwidgets don't get closed with the profile. I'll have a fix for that shortly but may not add it to this PR unless you all think I should.

Done! Thanks so much for this fix 💯