Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve and integrate the design and controls in the System Map and the Sector Map #4852

Merged

Conversation

Gliese852
Copy link
Contributor

@Gliese852 Gliese852 commented Apr 5, 2020

Screenshot from 2020-05-31 17-00-51
Screenshot from 2020-05-20 00-09-56

The ultimate task is to make a single control and some design elements for the SystemView and SectorView, and place this in the settings window and in the theme file.

Commits

Indent files for use in next commits

1f8081c

  • indent data/pigui/modules/map-sector-view.lua
  • indent data/pigui/themes/default.lua

Add new icons and rename several old ones

7a67a90

New icons (created by Nozmajner):

  • ships_with_orbits
  • lagrange_no_text
  • route
  • route_destination
  • route_dist

Move the sector map functions to a separate file

6fda132

  • Add src/lua/LuaSectorView.cpp
  • Move all the functions for the interaction between PiGUI and SectorView class from LuaEngine.cpp to LuaSectorView.cpp
  • Add pi_lua_generic_pull and _push overloads for the SystemPath class type
  • Add std::remove_reference and std::decay to the LuaPull function
  • Add LuaSectorView.cpp to the VS project

Redesign the sector map buttons and windows

1e2e471

  • redo the hyperjump planner from a separate module to a part of the sector map module, so that its position and size could be controlled by sector map module
  • add a button bar on the right
  • add handler functions for the zooming and rotation buttons to the SectorView class, and LuaSectorView.cpp
  • add the actual measurement of the size of the windows, or rather their simplified counterparts (dummies) to precisely align the windows relative to each other
  • add button for centering on the selected object
  • add function SwitchToPath, that only selects the system or also moves to it, depending on the settings
  • replace "SetHyperSpaceTarget" function to "SwitchToPath" function, when player click "Set As Navigation Target" in mission briefing
  • draw legs for all systems in the route
  • draw lines to the selected system from the current system and from the end of the route
  • set visibility params from button states every game start

Bold decisions to simplify the interface:

  • remove the description of the current system and the hyperjump target, now there is a description of only the selected system
  • hyperjump is done only to the next waypoint
  • not a system is remembered in the route, but a specific star, and the jump is carried out on it

Fixes/improvements to the system map

9d8be40

  • Separate grid color and grid leg color, adjust that colors.
  • Fix little problem with popup menu (sometimes is started showing the wrong object)

src/SystemView.cpp:

  • Refactor zooming in the SystemView: make zoom part of the camera space matrix. Now, multiplying by m_zoom is done only once, (there were 17).
  • Fix that smooth transitions did not affect the grid
  • Add clamping the camera pitch, like in the SectorView
  • Add grid legs for every "icon object" in the system map

Redesign the system map buttons and windows

8972f85

  • add string OBJECT_INFO to core/en.json
  • add the ability to change the background color and icon for buttons depending on conditions
  • decouple buttons window, orbit planner window, time control window
  • add the actual measurement of the size of the windows, or rather their simplified counterparts (dummies) to precisely align the windows relative to each other

Add input bindings for the maps to the settings

17e850e

  • Used the previously defined lines in /core/*.json, so I added an additional line search for this directory
  • Add some strings to input-core/en.json
  • Convert key pairs in axis
  • Remove unused bindings in KeyBindings.h
  • Completely remove several bindings that became irrelevant
  • Use some input bindings for the sector map in the system map

Move color settings to the theme file

b596d70

Final indentation of modified files

9a37b54

  • indent data/pigui/modules/map-sector-view.lua
  • indent data/pigui/modules/system-view-ui.lua
  • indent src/SectorView.cpp

@Gliese852
Copy link
Contributor Author

And I have a few questions:

  • Do we need a separate bindings to control the camera in the WorldView and in the SystemView and SectorView?
  • Do we need zoom in the SystemView and in the SectorView, if we can move the camera back and forth? (I believe that the zoom is still a change in the FOV, and not a physical increase in the objects in the model space.)

@bszlrd
Copy link
Contributor

bszlrd commented Apr 6, 2020

  • I think they could share the bindings. That would make it simpler, clearer.
  • Zoom isn't too useful for sure, and the FoV change could actually make the display more confusing I think. Actually going closer/further is the better mode of interaction in these cases I think.
    But I'd keep the name as zoom I think. Most people would grasp easily that it will get closer-further.

@Web-eWorks
Copy link
Member

Regarding System / SectorView bindings, I have a few recommendations:

  • The camera control bindings for ship control and Sector/SystemView should be different bindings and go in different binding pages. Among other reasons, you're not flying a ship in those modes, so the user should be free to bind ship-control inputs to those camera control bindings
  • I'd recommend a "View Control" page, with a "General" binding group that you're putting the shared bindings in. Further view-specific bindings (show ships, etc.) should go in view-specific groups, e.g. "System Map" or "Sector Map".
  • If you haven't noticed already, Binding Page names and Binding Group names are also translation keys, under the same rule as input binding names.

Zoom is completely different from FOV except maybe in an orthographic projection. If you keep the camera at the same distance but zoom by changing FOV, background objects will distort weirdly and it won't look good. Instead, "dolly" the camera physically closer or farther from the reference object.

@Gliese852 Gliese852 force-pushed the system-sector-views-controls branch 3 times, most recently from c935a63 to 6caa0c7 Compare April 15, 2020 17:35
@Gliese852 Gliese852 force-pushed the system-sector-views-controls branch 4 times, most recently from a5b0872 to c95d843 Compare April 26, 2020 09:46
@Gliese852 Gliese852 force-pushed the system-sector-views-controls branch 4 times, most recently from c5ee0ba to 5b96438 Compare May 9, 2020 07:18
@Gliese852
Copy link
Contributor Author

I have a proposal for redesigning the Sector Map and some related things. This is a rough illustration:
Screenshot from 2020-05-08 22-50-44

Key ideas:

  • instead of displaying on the left panel descriptions of the current system, the selected system and the hyperspace target system, leave only a description of the selected system
  • make checkboxes "Automatic system selection" "Remove jump when completed" always on and remove them
  • always make the next waypoint the hyperjump target
  • display in the route list the specific star of multisystem to which the jump will take place
  • if the selected star is a multisystem and is in a route, and in the left panel we click on a specific star of the system, this is also updated and fixed in the route list

Thoughts?

@Gliese852 Gliese852 force-pushed the system-sector-views-controls branch from 5b96438 to 4354dd5 Compare May 9, 2020 10:45
@bszlrd
Copy link
Contributor

bszlrd commented May 9, 2020

I'll include these mockups here as well, so they don't get lost:

01_sectormap
Sector map, everything's off, except the route planner. (Did not touch that part of the UI, but I think it should be toggleable as well)
I've added a toggle for the grid and label displays. The grid could cycle trough grid, grid+legs, nothing states.
The label could cycle trough label, in-range label, no label states. I'd move the uninhabited label toggle into the faction list.

02_sectormap_currentsys
Current system summary on. It shows the coordinates and short description of the system you are currently in.

03_sectormap_currentsys_targetsys
Current and target system summary on (If only one of them is active, then it shows on the top, if both, then the target is pushed down.
It shows the distance, fuel amount, ETA to the target system, and if it is in range (OK).

04_details_currentsys
Current system market details on

05_details_currentsys_targetsys
Current and target system market details on. If only one of them on, then it is displayed on the right side of the screen. If both are on, the target system is pushed to the left (or the other way, might make more sense)

06_faction_list
Faction list. If detail panels are on, they are hidden by it, but if you close the list, then they come back. Color coded, how it works now.
Added Uninhabited, Independent and Anarchy system filter (not sure whe have anarcy though), and Factions. Only those that are visible in the view show up in the list.
Uninhabited labeling van be moved here I think, so the label toggle button on the edge of the screen just toggles out-of-range labeling, not habited status labeling. I think it is more clear that way.

@Gliese852 Gliese852 force-pushed the system-sector-views-controls branch 4 times, most recently from 4e7dbc5 to 7e477ed Compare May 19, 2020 21:06
@Gliese852 Gliese852 force-pushed the system-sector-views-controls branch 8 times, most recently from d0142b9 to 5dfa633 Compare May 31, 2020 13:34
- add string OBJECT_INFO to core/en.json
- add the ability to change the background color and icon for buttons
depending on conditions
- decouple buttons window, orbit planner window, time control window
- add the actual measurement of the size of the windows, or rather their
simplified counterparts (dummies) to precisely align the windows
relative to each other
- Used the previously defined lines in /core/*.json, so I added an
additional line search for this directory
- Add some strings to input-core/en.json
- Convert key pairs in axis
- Remove unused bindings in KeyBindings.h
- Completely remove several bindings that became irrelevant
- Use some input bindings for the sector map in the system map
- indent data/pigui/modules/map-sector-view.lua
- indent data/pigui/modules/system-view-ui.lua
- indent src/SectorView.cpp
@Gliese852 Gliese852 force-pushed the system-sector-views-controls branch from 26ba445 to 9a37b54 Compare June 5, 2020 19:46
@Gliese852 Gliese852 mentioned this pull request Jun 18, 2020
6 tasks
- Change view movement mode to XY-constrained planar movement
- Rename map movement axes, invert key bindings for consistency
- Fix axis colorations in Drawables::Axes3D
src/SystemView.cpp Show resolved Hide resolved
src/SectorView.h Show resolved Hide resolved
@Web-eWorks
Copy link
Member

Pushed some improvements to the PR - I've changed the way movement in the SectorMap works, now it's constrained to the horizontal plane in the view direction, similar to how Elite: Dangerous' map mode works. I've also fixed a small inconsistency with the default key bindings for the sector map.

data/pigui/modules/hyperjump-planner.lua Show resolved Hide resolved
data/pigui/modules/map-sector-view.lua Show resolved Hide resolved
data/pigui/modules/settings-window.lua Show resolved Hide resolved
src/SectorView.cpp Outdated Show resolved Hide resolved
@The-EG
Copy link
Contributor

The-EG commented Jun 27, 2020

I like the UI. It's more compact and takes less away from the map.

I not sure about how the hyperspace target works, though. It's confusing to me.

Just playing around, I ended up zooming around and setting a long hyperjump route with multiple jumps. I decided not to use it (but didn't clear it) and go to Procyon instead.
I'm unsure what 'Automatic system selection' is intended to do now. If I select Sirius (also the first jump in my route that I made), I notice that the view centers on Sirius and the 'info' window also displays Sirius. If I uncheck it and click on Procyon, I see this:
image
What catches my eye (and is indicating conflicting things):

  • The map wasn't moved to Procyon
  • The 'info' window is now displaying Procyon
  • There's a white line from my current system (Sol) to Procyon
  • There's a halo around Sirius
  • Sirius A is no longer selected in the hyperjump route (that first row was highlighted)

I decide to take off and hit hyperjump to see what happens, and end up in Sirius. So I go back and check 'Automatic system selection,' click on Procyon again and hyperjump again....and now I'm in Ross 614 (the next jump in my route after Sirius).
Ok, so it seems that it's using the hyperjump route regardless of what's selected in the map.

I'll just clear the route then. Select Procyon again.....and no hyperjump button (which means there's no target). No combination of 'Automatic system selection' on or off allows me to set a hyperjump target.

I think always using the next jump in the hyperjump route for the target when there is a route set is ok, but there needs to be stronger indications that is happening. But, the idea that I have to use the route planner just to go one system over isn't intuitive to me.
I never really intended to force the use of the hyperjump planner, it was more of a tool that could be used when helpful. It seems like overkill to require it just to get the next system over.

One last tiny thing:
The 'add jump' button never really got a proper icon:
image

@Gliese852
Copy link
Contributor Author

@The-EG Thanks for the feedback!

The 'Automatic System Selection' behaviour did not change in this PR.
It has two functions. When in ON:

  1. When you move around the map by the keyboard, it automatically selects the nearest system.
  2. When you click on a system, it automatically shifts to it.

"Always make the next waypoint the hyperjump target" is one of the bold decisions this PR, what is the motivation:

  1. The player must freely move around the sector map, click on the system, look at the information on it, while the target of hyper-jump does not have to change, otherwise sometimes he will jump not to where he wants, this is unpleasant. Therefore, a simple selection of the system should not lead to a change in the target of a hyper jump, there should be an additional action.
  2. Often, in Pioneer, the goal of the trip is achieved in several jumps (for example, 10). Therefore, the goal of a single hyper-jump is usually not interesting and I would not want to go to the sector map at each point and select the next item manually.
  3. Leave as few entities as possible in the interface.

Therefore I decided to always make the next waypoint the hyperjump target, it satisfies all of the above.

Disadvantages include that a scenario like “created a route, but flew to another place” is impossible, but it seems this is a rare scenario, most likely the player wants to jump to where he planned.

P.S. Use the hyperjump planner for a single jump - one click, I do not think this is too bad.

@The-EG
Copy link
Contributor

The-EG commented Jun 27, 2020

@Gliese852 I don't really like that, but if others are good with it then I'll live with it.

@Gliese852
Copy link
Contributor Author

@The-EG In fact, we still don’t know exactly how successful or unsuccessful the changes are, in any case, this is not the final version, just an attempt.

@Web-eWorks
Copy link
Member

I'd (ideally) like to see an Elite-like context menu pop up when selecting a star on the sector map, with quick access to the "set hyperspace target" and "plot route" buttons (as well as Open System Map and possibly a few other context-specific actions).

This seems to me it would solve the issue at hand, where one might want to inspect a system but not jump to it, or jump to a specific system instead of the next system in the planned route (e.g. to take a detour or go for a more-efficient star).

For now, I'd compromise by defining a double-click on a star in the map to set that star as the next hyperspace target (and if this action was previously bound to Open System Map, moving that to a button on the selected system info). This seems like a good balance of speed and utility, and doesn't require mousing back to the hyperspace planner to set a single jump.

@Gliese852
Copy link
Contributor Author

@Web-eWorks it turns out that we will have to additionally show the target of a hyperjump somewhere?

@Gliese852
Copy link
Contributor Author

Gliese852 commented Jun 29, 2020

@Web-eWorks, @The-EG what if I add a context menu with the items "add after current" "add after selected"? and on a double click, the system will automatically be added into the hjplanner, if it is empty?

@Gliese852 Gliese852 force-pushed the system-sector-views-controls branch 2 times, most recently from 92be9cd to 1edea47 Compare June 30, 2020 13:26
src/SectorView.h Outdated Show resolved Hide resolved
- move detailed route calculation to a separate function, now it does
not execute every frame, only when changing the route or when changing
the equipment
- attach lambda functions to the OnPressed callbacks of these input actions
instead of the generic OnKeyPressed callback
- add some comments
@Gliese852 Gliese852 force-pushed the system-sector-views-controls branch from 1edea47 to 5330100 Compare June 30, 2020 18:16
@Web-eWorks Web-eWorks merged commit 80af631 into pioneerspacesim:master Jul 1, 2020
@Gliese852 Gliese852 deleted the system-sector-views-controls branch December 15, 2020 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants