pioneerspacesim
Feat/UI system view work
DRAFT PR
The code still needs some cleaning up, but preparing a PR for initial review/commenting on the implementation and direction.
Specifically:
- keep view-setting buttons on the bottom or move them into the left sidebar (the latter makes them more consistent with the Sector View)
- if moved into sidebar, should they be converted to checkboxes or drop-downs, again, for consistency?
NB: Apologies for the massive commit which refactors/modifies a large part of system-view-ui.lua; it could probably have been committed in stages to make it easier to review, but ultimately it was an iterative work which resulted in the current code.
Fixes #6170 (see issue for screenshots; before, during, and final)
After the trivial work of adding the missing values, some of the values were shown using icons instead of text labels due to the Object Info window being too narrow (constrained by the time control buttons).
So the Object Info window was converted to a Sidebar Module and added to the left sidebar, which displayed nicely, but prevented being able to click through system objects using the overview window while seeing the object information.
So the entire view was rewritten to be more in-line with the sector map:
- right-side button bar moved to bottom-center
- right-hand sidebar created
- sidebar Object View moved to right sidebar
- Orbit Planner converted to sidebar module and added to right sidebar
- a new settings sidebar module was created and the view settings buttons copied into it (Proof-of-Concept)
- some other minor tweaks, such as renaming the translate-button when in the System Overview mode
I like the orbit planner as a sidebar widget; I think that makes a lot more sense and removes a barrier to modernizing its functionality.
The "settings" buttons are tristate and could perhaps benefit from being made combo widgets (i.e. dropdowns) in the settings tab, but I'm unsure if that would hurt discoverability and ease of access as toggling ship display specifically is a somewhat high-frequency operation.
For consistency with the sector map, the object info and system overview widgets should swap sidebar sides. That more closely matches both the WorldView's placement of the system overview widget, as well as the general pattern from the SectorMap with object info being on the left and search/selection controls on the right. Additionally, it allows using the economy display widget with the overview open as well.
The "settings" buttons are tristate and could perhaps benefit from being made combo widgets (i.e. dropdowns) in the settings tab, but I'm unsure if that would hurt discoverability and ease of access as toggling ship display specifically is a somewhat high-frequency operation.
That's why I left this open as a question and copied the buttons for now instead of moving them / converting them to drop-downs until I got some feedback. I just felt it was more consistent with the Sector Map view to have them in the sidebar..
I didn't realise people would toggle the ship display constantly, I thought people would choose one and leave it. Perhaps it's more likely people would toggle ship lines on/off rather than ships themselves?
Also, these settings should probably be saved; it's a little bit annoying having to enable the grid every time I open the game, for example.
For consistency with the sector map, the object info and system overview widgets should swap sidebar sides.
Done (db24d60d1655ea2f987d269955f18548ab4e8058):
One issue is that object icons and labels are not being dimmed behind the sidebars. The same happens in every view..
Is there a way to raise the z-order of the sidebars to be on top?
I don't think we write out the full name of the setting, usually. Basically because then Germans come with their impossible long words, like: über-den-shipen-waffen-getrinken-displayung-jawhol-bitte-sehr
I don't think we write out the full name of the setting, usually. Basically because then Germans come with their impossible long words, like: über-den-shipen-waffen-getrinken-displayung-jawhol-bitte-sehr
:rofl:
As a german, I resemble this remark! And while the sentiment is not entirely inaccurate, Pioneer does have labels for settings everywhere else in the UI. Just open the main settings dialog, for example. Or, closer to home as it were, the Settings tab in the Sector Map.
Anyway, this is currently only a placeholder to determine how / where to have these settings. For consistency with the Sector Map, they should be in a sidebar panel.
That's why I left this open as a question and copied the buttons for now instead of moving them / converting them to drop-downs until I got some feedback. I just felt it was more consistent with the Sector Map view to have them in the sidebar..
I didn't realise people would toggle the ship display constantly, I thought people would choose one and leave it. Perhaps it's more likely people would toggle ship lines on/off rather than ships themselves?
Ship display is a common toggle (usually for mission purposes), and having it enabled it has performance implications. Full ship display with orbit lines is very inefficient right now and can cost multiple milliseconds of frame time even on high-end computers. Grids and L-points on the other hand have almost no gameplay purpose, and are rarely toggled in my experience.
Continuing discussion from IRC, my feedback/design decision is that we should retain a two-state button in the action bar (with a keyboard shortcut... eventually) which toggles display of ships (and hyperspace clouds) globally on/off.
Control of whether to render ship lines should be set with a combo or checkbox in the sidebar, defaulting to off (for performance and visual clutter reasons).
Control of whether to render arrival only, departure only, or all hyperspace clouds should be set in the sidebar as well. This ideally should be a dropdown (ui.combo(), uses zero-based indices) with three distinct options.
Each of these settings will only have an effect when the master toggles for ships and hyperspace clouds (respectively) are enabled.
Display of grids and lagrange points should also be combo boxes in the sidebar.
There is a premade UI style for themed combo boxes present in master; it's used in the sector map and can be accessed like so:
local comboStyle = require 'pigui.styles'.combo
comboStyle:withStyle(function()
ui.combo(...) -- or ui.comboBox if you want to manually submit contents without an array
end)
Finally got around to continuing this work.
- Rebased on current
master - Buttons are now toggle-buttons (on/off) for the various display items.
- Drop-downs in the settings set the specific modes
- Added hyperspace clouds
- Added hyperspace cloud details (only if hyperspace cloud analyzer is installed)
TODO:
- save/load the display settings
Buttons are now toggle-buttons (on/off) for the various display items.
I perhaps didn't specify it in my earlier comment: I'd prefer if the L-point and Grid display visibility is only controlled through the sidebar, i.e. drop the master on/off buttons in the action bar and add an "off" option to their respective combo boxes. Until such a time as either have a gameplay purpose, they're purely cosmetic options and are toggled at extremely low frequency.
Otherwise, I'm really liking the look of this, especially with that combo box style!
May I suggest a button/toggle row with icons instead of dropdowns, if it is not too late? Sorry, haven't been able to follow this PR. Dropdowns are a clunky solution here in my opinion, with three or so options at most. I can do a mockup after I get home today.
Otherwise, I'm really liking the look of this, especially with that combo box style!
Removed the buttons for Lagrange Points and the Grid, but left the Ship and Hyperspace Clouds buttons.
Code needs some refactoring/cleaning up if this is good?
May I suggest a button/toggle row with icons instead of dropdowns, if it is not too late? Sorry, haven't been able to follow this PR. Dropdowns are a clunky solution here in my opinion, with three or so options at most. I can do a mockup after I get home today.
Sorry.. I think the time has passed for buttons in the side-panel. Especially buttons which require even more states. The drop-downs are more flexible for multiple options, and also look more unified with other similar side-bar panels which have options. Having multi-state buttons here would (IMHO) look a bit odd.
Feel free to try to convince me (us?) otherwise.
I think it is doable without multi-state buttons:
Each one is either on (blue) or off (gray), so all of them are toggles. Less cluttered and more compact this way, no need for translations (apart from tooltips).
First row is for ships and hyperclouds, second row is for map display features.
And it could even be expanded later on if so inclinded:
Additionally a sidenote: if we want to differentiate between toggles and buttons on the UI later on, toggles could have rounded corners, or a border.
Alternatively it could be done with text toggles, but that looks worse and icons are self-explanatory already.
And in my opinion drop-downs are more clunky: one more click to do a thing, especially when it is already hidden behind a settings panel. True, drop-downs are more flexible when it comes to lots of options, but in this case there are three at most, which are already combinations of two or so options. Admittedly the hypecloud icons need some designing, for now I just cropped the wormhole and added an arrow triangle.
@bszlrd
And in my opinion drop-downs are more clunky: one more click to do a thing, especially when it is already hidden behind a settings panel. True, drop-downs are more flexible when it comes to lots of options, but in this case there are three at most, which are already combinations of two or so options.
True, dropdowns can be more clunky, but they excel at clearly communicating what the options actually do. The icons are not very clear, and lean very heavily on the use of tooltips to provide explanation to the player. For new to intermediate users, a sea of icon buttons is very confusing, and provides extremely low discoverability.
Adding to the additional confusion, some of the icons in that matrix are independent state (hypercloud visibility) and some are dependent state (grid legs).
For the purposes of getting this PR merged, I'd much prefer the use of the dropdowns when combined with a globally-serialized state variable (i.e. not a per-save setting) to remember your preferred setting at startup. We can definitely revisit the idea later, as I'm very much not thrilled with the current system map UI as a whole.
For the purposes of getting this PR merged, I'd much prefer the use of the dropdowns when combined with a globally-serialized state variable (i.e. not a per-save setting) to remember your preferred setting at startup. We can definitely revisit the idea later, as I'm very much not thrilled with the current system map UI as a whole.
@sturnclaw :
The Display Mode configuration is now loaded/saved from the global game configuration. It can possibly be improved, such as using a Section. I have also saved the "selected" and "displayOn" settings separately instead of just saving the display mode string as the latter would make loading and applying the configuration much more complicated. Again, it can be done, I just chose the easier way out for now.
Also refactored the code. Apart from cleaning up the commit history, I think this PR is ready to be merged, pending any review comment fixes to the actual implementation.
.. game settings for the System View are now saved into a separate section.
FWIW, the "GalaxyConfig()" constructor calls "Save()" on its settings many times when the main menu is entered. This is likely not good for SSD's..
This was added in 61ceb71b4172f3879c7289983c7de814a472e776
Fixed the unnecessary saves in GalaxyConfig - this is strictly not part of this PR so I can make it a separate PR if desired..
On the whole this looks good to me though, and I'm looking forward to getting it merged!
I'm very sorry, but is this PR still of interest? I appreciate this is a hobby project for everybody, and that real life happens, but it's been a little while since the last update.
I'm very sorry, but is this PR still of interest? I appreciate this is a hobby project for everybody, and that real life happens, but it's been a little while since the last update.
I think its very interesting, and hope to see it in master, alas, @sturnclaw has been very busy with real life, and I'm not comfortable reviewing it, (I'd rather you self-review).
As for who click's the "resolved" button in review conversation I'm not sure: is it reviewer or author?
As for who click's the "resolved" button in review conversation I'm not sure: is it reviewer or author?
Personally I think the person who opened a conversation should be the one to accept that it has been resolved. Someone can provide an explanation/fix, but it is up to the originator to accept that explanation/fix.
I've left a few nitpick changes which would be worthwhile to fix; please feel free to address those now or in a latter commit to master.
Thank you; I fixed the remaining nitpicks.
I will rebase on main and squash commits before merging, but it may take a few days as I'm about to pick my mum up from the airport..
Totally understood! Feel free to hit the merge button once rebased/squashed - thank you again for tackling this!
Totally understood! Feel free to hit the merge button once rebased/squashed - thank you again for tackling this!
Thank you, merged. I hope people like the new layout and additional functionality.
Thanks again for all the help and comments on this. Apologies if I sounded a bit snarky; just find it very difficult to pick things up again after several months. To be honest, I'd moved on to work on some KSP mods instead - only got reminded because I was reviewing some other stuff in GitHub. Would be nice to get back into Pioneer though.
No, close this one and just fix it in a new pr. I don't know what ~~resurrecting~~ restoring a closed pr means/does.
