Remember window positions. #214
Conversation
|
Thank you!!
|
mel-odious
force-pushed
the
remember-window-positions
branch
2 times, most recently
from
ffbc30b
to
f23541c
Compare
Vezzra
added
the
category:refactoring
|
OK, I'm considering splitting out the "keep other panels open when using the production/research/design windows" commits into a new branch and making a forum thread to discuss it, since it's likely to clutter up the changes here + it would benefit from wider discussion, I think. |
|
Works like a charm, and I'd like this very much to go in for 0.4.5. One issue, however (at least on OSX), which is actually an issue that already existed before, and is made worse by this new feature: When you resize the app window, the side panel on the map, the production screen, the design screen etc. keep the layout as it has been initialized for the orginal app window size. This is an age old minor flaw that usually had been solved by just closing and restarting FO after resizing the app window. However, with this new feature, that workaround doesn't work anymore. Because the position and size of all those windows is persistent now, it doesn't get reset when relaunching FO, so you can't force them to be reset to the new app window size anymore, short of manually removing the "windows" section from config.xml, which is more annoying and tedious than just deleting the file (he latter does not work, because this also resets the app window size, and here we go again... 😏) So, as a follow up to this PR we need to do something about that problem in order to include this new feature into 0.4.5. Any ideas what we can do? Can this be fixed easily? |
|
That would be resolved by docking windows to / within other windows, as has been discussed. In the case of windows at the edge of the screen, they would be docked to that edge and move with it when resized. Beyond the scope of this pull request, though. |
|
There's a command line switch '-w' or '--window-reset' and a toggle on the UI page of the options window for that. |
No question, but will nonetheless be needed for this PR to be accepted for 0.4.5. Having to start FO, maximize the app window, close, open up config.xml in a text editor, deleting the "windows" section, saving, restarting FO just to get a useable standard layout, or to have to rearrange all those windows manually isn't really acceptable... So, either we can do that quickly enough for 0.4.5, or we can come up with a stop-gap solution to the problem that can be implemented quickly enough. Or this feature, as nice as it would be to have it, will have to wait. |
That should be sufficient... I'll have to take a look at that. |
|
Bear in mind that it currently resets to the default positions as calculated based on the app size when FO was last launched (because those calculations are in the constructors of various container Wnds) so one would still need to restart FO to calculate the 'standard' layout for the new app size. I could allow it to recalculate those positions and sizes at any time but that would involve moving those calculations out of various constructors into helper functions & I don't want to go ahead and do that unless people think it's a good idea. |
|
Yep, that toggle on the UI page of the options dialog does the trick. Still not ideal, because players need to be aware that they need to do that after a app window resize instead of restarting the app. So I foresee a lot of complaints about that. Nevertheless, good enough to accept that PR for 0.4.5. As it works very well otherwise (as far as my brief tests have shown), I'd say ripe for merge... opinions? |
|
I have a question to resolve and then possibly another commit to push, writing that comment now... |
Ok... didn't notice that, but have to test again. No more time right now although, so I'll report back later. |
|
The reset window position toggle should probably be a button. |
|
There needs to be some persistent state for dynamically-created modal windows to know that they need to fall back on their defaults instead of using stored values. I could disable this feature for modal windows altogether but that would mean that the object list filter window would remain a pain to use until it's made movable and not-modal. |
|
You can't just reset the stored values to their defaults when the button is pressed? |
|
For non-modal windows, yes, because they're all constructed when the app starts so the option change signal (or button pressed signal) can be connected to them then. Modal windows are constructed dynamically when they are opened so we can't connect the signal to their LoadOptions() function if they're not open at the time. |
|
What if you set the currently stored values in the relevant options? Then the next time a modal window is opened, it would use those values, no? If there are any (other) modal wnds open, presumably the optionswnd wouldn't be visible for the button to be clicked... |
|
I could do that if, say, the CUIWnd class had a static set of all window names that have been registered but I wanted to avoid adding members to classes where possible (which is why I added the ".exists" option to prevent name clashes). Otherwise, there's no way of getting a list of currently-registered options out of the DB to find out which windows have been registered. Could add another OptionsDB function to get iterators into a list of options that match a substring ("UI.windows." in this case) but that would be overkill, I imagine. (eta: or just a const ref to OptinsDB::m_options I guess) |
I'd rather not make assumptions like that and have it break later... |
|
Would it actually matter if another modal wnd is open? If it was, wouldn't it exist and be able to respond to a signal to reset its position...? |
|
It would work for any modal windows that are open at the time, yes. I just don't want other programmers to add dynamically constructed windows and then wonder why the "reset defaults" button doesn't work for their window if it's not open at the time... |
|
I suppose this can all wait, as the ability to in-gui reset isn't a top priority. But I think adding iterators or other ways to access the already-registered options is reasonable. |
|
I'd like a better solution for it, but yes, it can wait until the prerequisite work is done (e.g. functions to recalculate default positions, finding arbitrary options in the OptionsDB) |
|
I've added an options window toggle to enable/disable hiding other windows when the production window is open, but it currently doesn't hide the messages and empires windows. Previously, these would stay open so I was wondering if there was some previous decision to keep it that way or if I should hide those windows along with all the others. |
|
Hiding them all seems fine... |
|
I'll clean up this branch tomorrow, it's far too late to do so right now. |
Add functions SaveOptions(), LoadOptions() and AddWindowOptions() to
CUIWnd. Implement the feature for several CUIWnd subclasses (SitRepPanel,
EncyclopediaDetailPanel, ObjectListWnd, CombatReportWnd,
ModeratorActionsWnd) which involves supplying the constructor with the
desired name for the window in the config ("UI.windows.[name].left" etc).
The window will then save & load its position, visibility etc. under that
name.
Make GG::Wnd::Hide() virtual so that CUIWnd can override it and call
SaveOptions().
Rename width/height parameters in ctors to reflect that they will be
overridden by any values present in the config.
...from triggering ::SaveOptions(). Add bool CUIWnd::m_config_save. When false, prevents SaveOptions() from writing to the OptionsDB, so LoadOptions() can temporarily call SizeMove() etc. without triggering SaveOptions(). Add bool GG::GUI::DragWnd(GG::Wnd*, unsigned int) const; to check if a window is being dragged by a certain mouse button. Use this function to disable CUIWnd::SaveOptions() when the CUIWnd is being moved/resized by dragging it, instead of using CUIWnd::m_config_save because m_config_save will be needed to disable position validation when calling ::SizeMove() from ::LoadOptions()...
...when activating them from the toolbar. Prevents the map scale line overlapping those windows now that they aren't always in the same position on game load, plus it makes those functions more consistent.
...like other top-level CUIWnds owned by MapWnd, i.e. don't repeatedly Register() and Remove() them but just rely on Show() and Hide(), calling MoveUp() on them when they are shown.
Change the ctors for the CUIWnd subclasses which have had position-saving implemented so far to take default x,y,w,h as parameters so they can be set as defaults in ctor without calling MoveTo() etc. on them afterwards (which would override the saved position!)
...and allow several top-level CUIWnds to enforce that effectively without being overridden by now-redundant calls to Show()/Hide(). Add "bool visible" parameter to CUIWnd ctor so that certain windows can specify that they default to being hidden. CombatReportWnd, EncyclopediaDetailPanel, ModeratorActionsWnd and ObjectListWnd now do this, and they are not visible by default when entering a game (but the SitRepPanel is). Remove calls to GG::GUI::Register() and CUIWnd::Hide() for some floating map windows in the MapWnd ctor. Ensure that MapWnd::m_sitrep_panel is updated properly in MapWnd::InitTurn() because the sitrep panel can now be open when a game is started (without the update, the panel wouldn't update itself when loading a game on turn 1). Add MapWnd::RegisterWindows() and ::RemoveWindows() Which are called from the FSM to enable/disable the rendering of top-level CUIWnds rendering without affecting their visibility flag. Would be more elegant to parent them to a container GG::Wnd but that causes some issues with mouse events being intercepted... Remove Hide[...]() calls from MapWnd::Cleanup(), the windows can determine their own visibility. Replace Show/Hide calls for message and player list wnds in the HumanClientFSM with calls to Regiter()/Remove(), again in order to avoid overwriting their stored state. In MapWnd ctor, ensure that the correct images are displayed on the toolbar buttons based on visibility of their respective windows, since those windows remember their own properties and apply them before game start/load. Delete MapWnd::m_moderator_wnd in MapWnd dtor.
Update many ctors for CUIWnd-derived classes to be more consistent and support passing a config_name so that they can save their properties to the config file. Add config_names to many CUIWnd-derived wnds, including BuildDesignatorWnd::BuildSelector, SidePanel (in production window), SidePanel (in map view), MessageWnd, PlayerListWnd, DesignWnd::PartPalette, DesignWnd::BaseSelector, DesignWnd::MainPanel, FleetWnd (disabled when multiple fleet window mode is enabled!), MapWndPopup, FilterDialog, OptionsWnd, TechTreeWnd::TechTreeControls... Remove redundant static FleetWnd members. Remove redundant position checking code in FleetWnd::Init() and MapWnd::SelectFleet(). Remove SidePanel cleanup from MapWnd::Sanitize() since the sidepanel now remembers its own properties. Remove OptionsDB options "UI.sidepanel-width", "UI.chat-wnd-width" and "UI.chat-wnd-height" because they are now redundant.
...and connect to it in CUIWnd ctor.
Add a signal that is sent from HumanClientApp::Reinitialize() when it
detects a fullscreen/windowed transition. Can't rely on
OptionsDB::OptionChangedSignal("fullscreen") because that can change
without actually triggering a video mode change (e.g. user still has to
click the apply button in the OptionsWnd) and
HumanClientApp::HandleWindowResize() doesn't provide video-mode-change
info.
Connect this signal to CUIWnd::LoadOptions() in the CUIWnd ctor if
config_name is specified so that it automatically loads the new options
when switching between fullscreen and windowed modes.
...WindowOptions() to be static, also take and return a string. CUIWnd::AddWindowOptions() now uses a new OptionsDB option to check if a new CUIWnd should use the config_name it has been provided, and if not, returns an empty string (which means to not save window properties). This allows it to be used to initialize the const m_config_name member using its return value without letting two windows have the same m_config_name. It no longer refers to any non-static members/functions so it has also been made static.
Fixes issues with translated text being too long and getting cut off, user can now make the window wider to show all text. TODO: add automatic text wrapping...
...to their defaults. While checked, the option will cause all current CUIWnds and any new ones to reset their saved values to the defaults (the defaults are calculated when the window is constructed). Change MapWnd ctor to use the default dimensions of the SitRepPanel to calculate defaults for EncyclopediaDetailPanel, instead of the current position of the SitRepPanel.
...temporarily disable m_config_save while it calculates its own height.
...while the production window is open. Show/HideProduction() now calls RemoveWindows() and RegisterWindows() instead of calling Show()/Hide() on the map view panels (SitRepPanel, ObjectListWnd etc.) if the relevant option is enabled in the OptionsWnd (in the Galaxy Map tab), which allows those panels to remember their state. Change MapWnd::Toggle[window]() functions to call their Show[window]() counterparts if the production/research/design windows are open, instead of simply hiding the latter windows. This ensures that if e.g. a turn is advanced with both the production window and sitrep panel open and the sitrep toolbar icon is clicked, the production window is hidden and the sitrep panel is updated with the latest turn's sitreps, instead of only hiding the production window and leaving the sitrep panel un-updated. When any of the panels' toolbar icons are clicked, the production/research/design windows are closed and all of the panels are restored. Add calls to ShowAllPopups() and RemoveWindows() to MapWnd::Cleanup() to ensure that popups don't save ".visible = false" to the config and that windows are properly hidden when entering/leaving a game. Bind panel close buttons to MapWnd::Hide[wnd]() rather than ::Toggle[wnd](). Clicking the close button on a panel while the production window is open now closes the panel instead of closing the production window.
CUIWnd subclasses. They can reposition themselves when they load their properties from the config and need to call DoLayout() even if they're not visible, otherwise their children won't be resized and won't display correctly when they are eventually made visible.
mel-odious
force-pushed
the
remember-window-positions
branch
from
911b5ac
to
7c36bf3
Compare
|
OK, the branch has been rebased and cleaned up, the overall diff is the same as it was beforehand and is ready for merge, as far as I can tell. It "works" in that I've dealt with any unexpected behaviour that's been reported and it's possible (but awkward) to recalculate the default window positions after changing the app size. I'd like to make that second part much less awkward (and I think I know how to do so, see below), but I'm thinking of putting that in a seperate PR so that this feature can be merged in time for 0.4.5. The follow-up PR might also be ready in time, but I can't guarantee that... What I'd like to do from here is: split the calculation of default window positions from the MapWnd, BuildDesignatorWnd, TechTreeWnd, DesignWnd and ClientUI constructors so that those defaults can be recalculated at any time based on the current app size. This would address @Vezzra's concerns about being able to reset to the default layout after changing the app size. The second thing would be to add a way to find OptionsDB options that match some substring ("UI.windows.") and a) set and apply the defaults for the ones that have been registered using the calculations mentioned above, b) remove the ones that haven't been registered (currently, this includes modal windows that haven't been opened yet) so that there are no stored values to use and they fall back on the defaults when they are eventually opened. This would allow the 'reset window positions' option to be a button in the options window instead of the current awkward toggle. Thoughts? |
Remember window positions.
I've extended the deadline for the creation of the release branch by a week, do you think that will be enough? Because...
...this seems rather important. Right now, you have to quit, restart and set the toggle in the options dialog to reset the default window sizes/positions after an app window resize. Lets say, that's bearable, but far from ideal. If we can get in what you propose here for 0.4.5, that would go a long way to make this better. But if that's too much even for the delayed deadline, no problem. What you've accomplished here is already a really big step, this feature has been requested repeatedly since forever. Thumbs up! 😃
As far as I'm concerned, by all means, go ahead! |
|
Thanks! 😄 |
Ah, now that's a very good idea! 👍 |
Allows most non-self-positioning CUIWnd subclasses to save their position/size, visibility, pinned/minimized status to the OptionsDB, which is written to config.xml and saved between executions. To enable this for new windows, pass a config_name parameter to the CUIWnd constructor. Only one window can have any given name at a time (logs an error and doesn't let the second window save/load the options). No more rearranging the design window again and again!
Makes windows go to the top of the z-order when activated via toolbar button.
Keeps separate positions for windows in fullscreen and windowed modes and auto-repositions them in response to windowed/fullscreen changes, which helps deal with the fact that FO will only start in fullscreen mode if '-f' is specified on the command line (shouldn't it save this to the OptionsDB?)... not everyone will get around to setting that up, you might forget to start it in fullscreen mode one time, you might just want to play it in a window one time... your fullscreen layout won't be lost.
Adds a button to OptionsWnd to reset all window properties. As long as this option is checked (or if '-w' is specified) any existing or newly-created windows will use their default position and size, overwriting what's in the DB. Useful if the UI somehow breaks badly, could also delete the section from config.xml.
Commits f23541c, 77af188 and 1bf3b2b allow the top-level panels in the map view remember their state when the production/research/design windows are opened, so the sitrep panel, pedia panel, objects list etc. don't need to be re-opened every time you switch to those windows. Haven't discussed this change on the forums but it was simple to add and relies on changes in this branch, so I put it here for people to test.