Move the System Map to PiGUI#4821
Conversation
sturnclaw
left a comment
sturnclaw
left a comment
There was a problem hiding this comment.
The indentation makes it a little difficult to review in the web UI, so I'll pull the PR locally and test it in more depth. So far, I've found a few things for you to fix, if you don't mind!
Gliese852
force-pushed
the
ecraven-systemview-squashed
branch
5 times, most recently
from
5127474 to
3161c08
Compare
|
@Gliese852 you need to give a shout when you feel this PR is ready for re-review |
|
@impaktor I fixed all the errors, tell me please how to inform about it. |
I think this should be discussed, as it feels very strange (to me), to not have rotation on right mouse button, like it is in WorldView and SectorView. I thought it was a bug, and didn't realize there was a middle mouse button. Could one have rotation on RMB, and move functinality on middle mouse button to someplace else? What does the rest of you think? There is logic in middle mouse button, since it's already used for zoom in/out, but having them different on different views is strange. And many people don't have middle mouse button, I suspect, if on a laptop, like me. I haven't looked at the code (yet?), but seems to be a lot of improvements |
|
@impaktor In the worldview, the camera rotates with the middle button, and the ship rotates with the right button. In fact, I play a lot with a view from the outside, so it’s just convenient for me to rotate the camera in a worldview and in a systemview using a wheel. In fact, I do not have a firm position, as it should be, this is a question of one line of code. On the other hand, to play without a mouse, can it be better to make on-screen buttons to rotate the view? And maybe assign physical keys. |
|
@Gliese852 I think it should be consistent with the other views. |
Sounds like a good idea. @nozmajner maybe there should be an icon for such a button? Either way, I think this PR probably needs to touch sectorview as well, to make rotation consistent. Sorry to expand the scope of this. |
|
@impaktor @nozmajner Maybe then do this: make one on-screen button for zoom, one on-screen button for rotation, and assign numpad keys. And do the same in the sectorview? |
|
@impaktor @nozmajner I implemented on-screen buttons for zooming and rotating in the system Map in the last commit. |
|
Doesn't build on my system: |
This comment has been minimized.
This comment has been minimized.
|
Regarding the handling of rotation/zoom mode, here are some words transcribed from IRC: I've settled on implementing a "Capture Mouse" setting to the input system to unify the various methods of handling exclusive mouse input and pre-empting ImGui's mouse handling; I'll get that coded up tomorrow as I don't expect many issues with implementing it. I'd recommend you rebase your PR to remove the changes to Pi.h/Pi.cpp once that's merged (which will be very soon after I get it working). |
sturnclaw
left a comment
sturnclaw
left a comment
There was a problem hiding this comment.
I've left some comments about suggested code structure; it's at your discretion whether to follow them rigidly or not, but please do take them into consideration when updating this PR.
|
Nice. I wonder if the magnify and rotate buttons could be placed somewhere else? E.g. top right corner? @nozmajner thoughts? (see screenshot below) It's still pretty strange to use different buttons for rotation in sector view vs. system view. I'd suggest making sector view work the same way as system view, and also add magnifying glass and rotate button to top right corner. Other than that, good work!
|
I was more thinking about the RMB / MMB behaving differently / opposite in system view vs. sector view. |
|
@impaktor Oh, yes, I just didn’t put it that way, I had in mind all the controls including the keyboard, mouse and on-screen buttons will be copied from systemview to sectorview. |
|
OK. Sounds good. (right click menu for sectore view would be nice as well, but could be outside the scope of this PR, as long as RMB/MMB inconcistency is sorted out in this PR). I played a bit more with this branch, and updated the screenshot in my previous post to show more features. Very nice. |
Gliese852
force-pushed
the
ecraven-systemview-squashed
branch
from
e32068a to
d9ac379
Compare
|
...Those comments were supposed to be part of a review, but apparently Github doesn't do reviews unless you're viewing the whole PR's history and not just a single commit. Looking good so far, just some minor changes and I'll try to get this merged in ASAP! |
|
@Web-eWorks I implemented the latest recommendations in the commit ba3bbe8 |
|
Looks like commit history could need some squashing, ideally |
|
@impaktor Can squash-merge solve this problem? or it should be squashed in advance for some reason? |
|
Well, squash-merge makes the whole thing into a single commit, which is a but brutal on the other end, as I assume you've had to implement various titbits to make this whole PR come together, so ideally they shuld be kept as separate commits, but the small fixes you've done in the review process should be squashed into where the code they fixed was introduced. ...If we are to appeal to our perfectionsit side. |
Gliese852
force-pushed
the
ecraven-systemview-squashed
branch
from
d413105 to
698da86
Compare
The main idea of the interaction of lua side and C++ side is the same as
in the worldview - the C++ View Class builds three-dimensional objects,
some projected points are passed to the lua script, the lua script only
works with screen coordinates.
The difference between the system view and the world view is that not
all objects can have a physical body. If we examine a non-current star
system, we see only system bodies that do not have physical bodies.
Therefore, if for a world view you can simply create an array of
physical bodies, this will not work in a system view.
To solve this problem, struct "Projectable" was created, into which you
can write any point object, system body, ship, Lagrange point,
apocenter, pericenter, etc. Now all these objects can be processed in a
single array.
The names of the types of these objects are recorded in an enum, which
is exported to a enum_table.cpp and read by the script as integers at
the start. I thought it was better to avoid passing strings in such a
hot place in the code. (runs for every object in every frame) Therefore,
type names are written and transmitted as integers.
All colors are now imported from lua: color index names are listed in
the enum, which is exported to the enum_table.cpp. At event
"onGameStart" the lua script reads these names and sends color values to
the class. Now you can change colors at runtime.
* Improvements/fixes: *
- Fix zooming speeds.
- Fix ship's orbits in nonroot frame.
- Remove unused old gui objects.
- Landed ship are now spinning with the planet when rewinding time in a
planner.
- Now you can center the view on any object, including the player’s ship.
- Add right-click context menu to center or set object as target.
Therefore, to rotate the view, set the middle button.
- Add indicators for navigation target and combat target.
* Changes in the files explained *
SystemView.cpp, SystemView.h, system-view-ui.lua, lang/core/en.json
The whole thing, lots of changes. Now the system view draws only the
orbits and the grid. In those places where the icons were drawn, instead
of drawing, they are stored in an array, which is then requested from
the lua.
LuaEngine.cpp
Add a general function that returns the value of an any enum by name
from an enum_table.cpp (for calling from a lua)
Space.cpp
Added a notification for the systemview when the ship is removed from
space, because it can be centered at this time, and an segmentation
fault will occur.
LuaSystemBody.cpp
Added attributes to determine the shape of the icon and set the
navigation target.
enum_table.cpp, enum_table.h
Rescanned enums to pass color settings and object types between Lua and
C++.
LuaGame.cpp
The float does not have enough accuracy to display time up to seconds,
change to double.
icons.svg, themes/default.lua
Add rotate view icon
Gliese852
force-pushed
the
ecraven-systemview-squashed
branch
from
698da86 to
12d0b6c
Compare
|
Hmm, this PR is showing the Travis build, so only new PRs lack it? (general statement) |
Gliese852
force-pushed
the
ecraven-systemview-squashed
branch
from
12d0b6c to
aea0134
Compare
- Now only those orbits whose radius is less than 1% of the screen width are not shown. - Add smooth transition when click object. - Improve system bodies orbits. - Fix that ..apsis and lagrange points were displayed in the wrong place. - After rescanning enums, an error was found in the pigui/Face.h, fixed. fixes pioneerspacesim#4801 - Some indentations to pass clang - Added static_cast to Space.cpp, because it caused a warning at compilation.
Gliese852
force-pushed
the
ecraven-systemview-squashed
branch
from
aea0134 to
ac7e982
Compare
|
Just to repeat what I wrote in IRC, tested it, very nice, but I have the following suggestions:
|
|
@impaktor I planned to do a sector view in another PR (next) |
4 participants
The main idea of the interaction of lua side and C++ side is the same as in the worldview - the C++ View Class builds three-dimensional objects, some projected points are passed to the lua script, the lua script only works with screen coordinates.
The difference between the system view and the world view is that not all objects can have a physical body. If we examine a non-current star system, we see only system bodies that do not have physical bodies. Therefore, if for a world view you can simply create an array of physical bodies, this will not work in a system view.
To solve this problem, struct "Projectable" was created, into which you can write any point object, system body, ship, Lagrange point, apocenter, pericenter, etc. Now all these objects can be processed in a single array.
The names of the types of these objects are recorded in an enum, which is exported to a enum_table.cpp and read by the script as integers at the start. I thought it was better to avoid passing strings in such a hot place in the code. (runs for every object in every frame) Therefore,
type names are written and transmitted as integers.
All colors are now imported from lua: color index names are listed in the enum, which is exported to the enum_table.cpp. At event "onGameStart" the lua script reads these names and sends color values to the class. Now you can change colors at runtime.
Improvements/fixes:
Extra improvements/fixes (second commit):
Changes in the files explained:
The whole thing, lots of changes. Now the system view draws only the orbits and the grid. In those places where the icons were drawn, instead of drawing, they are stored in an array, which is then requested from the lua.
Add a general function that returns the value of an any enum by name from an enum_table.cpp (for calling from a lua)
Add a notification for the systemview when the ship is removed from space, because it can be centered at this time, and an segmentation fault will occur.
Add attributes to determine the shape of the icon and set the navigation target.
Rescanned enums to pass color settings and object types between Lua and C++.
The float does not have enough accuracy to display time up to seconds, change to double.
Add rotate view icon