X Tutup
Skip to content

Move Economy & Trade -view to pigui#4967

Merged
impaktor merged 3 commits intopioneerspacesim:masterfrom
impaktor:econ-trade
Nov 28, 2020
Merged

Move Economy & Trade -view to pigui#4967
impaktor merged 3 commits intopioneerspacesim:masterfrom
impaktor:econ-trade

Conversation

@impaktor
Copy link
Member

@impaktor impaktor commented Oct 3, 2020

Thoughts

Working with this, I think there's a lot of stuff that's overlapping with "ship info". I'd suggest "ship info" only show the static info on that ship's stats, and equipment, and this "economy & trade view" shows all the stuff about cargo, fuel, interna-tank mass, and range. Possibly, this could be outside the scope of this PR.

Also, I had a wrestling match against the colorselectbutton since, in my estimation, it makes no sense, in how it tints the button, but for now, it's some kind of deactivated when landed (one cannot jettison when landed). I got the feeling I was using the wrong function, since it's not doing what I want, which is what the name suggestes it's supposed to do. Theme-issues.

Bug exposed in master?

I remember playing with this in june/july, and finding some inconsistency in how we define "cargo space" relative internal fuel mass, but I'm not sure I notice what it was. I.e. (back then) I thought the "Used" cargo space displayed in the cargo gauge bar was wrong.

What remains

At the moment I'm not quite sure how to do the first two in this list:

  • Move jettison-buttons further right?
  • Fix there not being any margin to the left
  • Clean up / remove commented out code experiments
  • Tool tip doesn't seem to work?
  • Settle on what info should be displayed / how?
    • should progress gauges be shown (at all? / or like now, clustered under "summary")?
    • should we show cash?
  • squash some commits

2020-11-10-092015_1600x900_scrot

@WKFO
Copy link
Contributor

WKFO commented Oct 3, 2020

I have no strong opinions on progress gauges, but cash should be shown. Isn't it the only place where we can see cash in-flight?

@The-EG
Copy link
Contributor

The-EG commented Oct 3, 2020

As a straight conversion of what's there, this is good.

But, I feel like there's a struggle to find a way to fill up the more of the screen on some of these windows (not just this one). Do they really need to be dedicated full screen displays?

As far as gauges or not go: I'd approach it like any data analysis visualization. Why am I opening this screen in the first place, and when I do what's the question(s) I'm trying to answer?

For an example:
I'm making a multiple jump trip with a full cargo hold, which means I don't have enough room to also carry enough hydrogen for the jumps. I'll need to pump fuel down after each jump to make the next one.
So, why am I opening this window? To pump fuel down to the cargo hold to make another hyperjump.
What questions do I need to answer?

  • How much fuel do I currently have in the fuel tanks?
    Do I need to stop in a system along the way to refuel? Displaying the remaining Delta-v is interesting here, if I had a way of knowing how much I'd need to make the final leg of the trip (ie. navigating to and landing at a station) this value could be really useful. As is, a percentage is definitely needed, I probably don't want to go under 50%.
  • How much fuel do I need to make the next jump?
    I actually have to flip between windows to get this currently...or I'll just fill any empty space with hydrogen and hope I didn't misjudge how much space I'd need.
  • How much fuel do I have currently in cargo? (I may need 2T, but already still have 1T left in cargo)
    Related to the above, but how much do I need to move...
  • How do I move the fuel?
    Fairly obviously, the buttons.

Going through that, I'd argue that the 'pump down,' 'refuel,' fuel and hyperjump fuel info/gauges are all related and could actually be grouped visually. I think a grouping like that would also help make the interface more intuitive as well. If we want gauges, having the fuel gauge where the current fuel info is (above the pump down, refuel buttons) and the hyperjump fuel gauge below the buttons could give a nice visual que of what's going on.

@impaktor impaktor mentioned this pull request Oct 3, 2020
5 tasks
@impaktor
Copy link
Member Author

impaktor commented Oct 4, 2020

These are the dynamic variables, that I think aught to be shown together with pump up/down buttons and info on cargo loaded:
shipinfo

(Note: I've with this PR also changed font to medlarge, to make it consistent with the other screens)

the other info is static, and is more for when the player want to drool on their nice ship, and check when upgrading equipment, or planning what upgrade to buy.

@Gliese852
Copy link
Contributor

Thanks for the font! What if you also show the fuel use rate?

@bszlrd
Copy link
Contributor

bszlrd commented Oct 4, 2020

I don't think fuel use rate is much useful. Knowing how much time you need to burn up your fuel isn't that important information compared to deltaV.

@Gliese852
Copy link
Contributor

@nozmajner deltaV depends on the load of the ship, while this value is constant for the ship, and characterizes the efficiency of the engines, if I understood it correctly. Besides, it seems to me it will add a sense of scale.

@bszlrd
Copy link
Contributor

bszlrd commented Oct 4, 2020

For that Exhaust velocity or ISP is a more standard measure in space flight. Admittedly not that intuitive though.
How even fuel use looks for our ships?

@Gliese852
Copy link
Contributor

Gliese852 commented Oct 4, 2020

@nozmajner
for sinonatrix:

   "effective_exhaust_velocity" : 19500000.0,
   "forward_thrust" : 5500000.0,

since

FuelUseRate = forward_thrust / effective_exhaust_velocity * 0.001 

use rate = 0.00282 ton/sec, I guess -> 1.015 ton/hour

get it from:

float Propulsion::GetFuelUseRate()

EDITED 2 times, sorry

@Gliese852
Copy link
Contributor

@nozmajner deltaV does not take into account the fact that you can carry additional fuel in the hold

@impaktor impaktor force-pushed the econ-trade branch 2 times, most recently from ba7988a to 43e1e27 Compare October 28, 2020 10:46
@impaktor impaktor force-pushed the econ-trade branch 2 times, most recently from a99c60d to df7f3de Compare November 13, 2020 10:59
@impaktor impaktor force-pushed the econ-trade branch 2 times, most recently from 43d49f9 to 085c7bd Compare November 25, 2020 10:09
@impaktor impaktor force-pushed the econ-trade branch 5 times, most recently from 4c028de to d7b44d4 Compare November 27, 2020 20:44
@impaktor
Copy link
Member Author

Current state:
2020-11-27-214225_1600x900_scrot

Nozmajner's mockup:
mTptHbs

@impaktor impaktor marked this pull request as ready for review November 27, 2020 20:48
@sturnclaw
Copy link
Member

sturnclaw commented Nov 28, 2020

@impaktor I'm sorry, I just couldn't help myself 😛
Fixed a few minor issues I noticed - the window padding is now correct and no longer offset to the right, we rebuild the cargo list only when it changes, and the cargo meter now actually displays the percentage of cargo space and not total equipment space.
EDIT: oh, and now when you have more than two digits of cargo it doesn't clip the quantity anymore. That was important to get fixed. Feel free to adjust the value I used for itemSpacing - I copied from the other screens, but it's a little too much on the vertical axis, especially for the cargo list.

Also made the TabView debug reloading interface simpler and fixed a bug with the ImGui stack cleanup handler that wasn't cleaning up fully and was causing C++ assertions when a script errored out. Feel free to squash my commits as needed.

image

impaktor and others added 3 commits November 28, 2020 11:41
- Removed double padding from window
- Reload the ship cargo list when it changes, not every frame
- Made the cargo gauge display the cargo capacity, not the equipment space
- Fixed a bug in ImGuiStack cleanup
- Simpler mechanism for registering reloaded views
- Call refresh() on reloaded tabs to fix bugs
@impaktor impaktor merged commit 6461966 into pioneerspacesim:master Nov 28, 2020
@impaktor impaktor deleted the econ-trade branch November 28, 2020 11:23
@impaktor
Copy link
Member Author

@Web-eWorks thanks! I've squashed my temporary commit, and rebased & merged to master.
Might we expect a PR that will purge master of NewUI later? :)

@sturnclaw
Copy link
Member

Indeed you shall!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

X Tutup