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

Feat(code): Update UI for current system #166

Merged
merged 5 commits into from
Dec 11, 2024

Conversation

MadisonianX
Copy link

@MadisonianX MadisonianX commented Dec 3, 2024

Feature

This PR addresses the comment in issue #164 that the system name being above the flagship area might make people look for other system data (such as system solar power and solar wind from said PR) over there.

Summary

Uses flagship name in the top right box instead of current system, which is now moved top and center with a smaller, no frills version of the "Currently Selected System" banner from Map Panel.

Screenshots

Before:
Screenshot 2024-12-03 at 3 48 03 AM

After:
Screenshot 2024-12-03 at 3 33 13 AM
Screenshot 2024-12-03 at 3 34 13 AM

Slight transparency to the current system banner so that objects are partially visible behind it.
Screenshot 2024-12-03 at 3 54 15 AM
Screenshot 2024-12-03 at 3 33 22 AM

Testing Done

Jumped around to new systems; current system name displays in new location while flagship name displays in old system location.

Code borrowed from MenuPanel::Draw().

Uses flagship name in the top right box instead of current system, which is now moved top and center with a smaller version of the "Currently Selected System" banner from Map Panel.
@TheGiraffe3
Copy link

Would it be possible to move the jump minimap down slightly so that it doesn't overlap? Nice idea on the transparency, but I think it would be better not to overlap if we don't have to.

@MadisonianX
Copy link
Author

Would it be possible to move the jump minimap down slightly so that it doesn't overlap? Nice idea on the transparency, but I think it would be better not to overlap if we don't have to.

The opacity was for flying over objects and I only noticed the hyperspace link map later on; will gladly move it down but I cannot figure out where that thing is declared.

@Zitchas
Copy link
Member

Zitchas commented Dec 4, 2024

I like the change in having the flagship name on the top right instead of the system.

Having the current system name in the top center definitely looks good, although I'm reluctant/cautious about putting things in the center of a side (especially the top and bottom center). The reason for this is that screens are generally square or rectangular, and the middle of a side is the closest point of the edge to the flagship, and thus the most likely spot to block or interfere with something that is closing on the player. Given that most people play with rectangular screens set in landscape, this means that the top and bottom centers are the closest point to the flagship. In gameplay, a ship may be able to shoot at the player from the edge of the screen in the top center where it wouldn't be able to reach the player from the left or right edge.

But I can't argue with the fact that it does look good there.

Another idea that might look good/functional too: What about moving it over to the RADAR area, and have two grey boxes over there. One (probably the top one) that says "Current Location: (system name here), with the solar power and wind stats to the right of it), and then right below it a second grey box for the current "Destination: (system name here), and have the two new stats to the right of that. In both cases perhaps having those stats inside the grey boxes so it is clear that they pertain to the systems in question. That would make it easy for the player to tell at a glance that their destination system has worse solar wind than their current one, for instance.

@MadisonianX
Copy link
Author

Here's two alternative displays for the system name near the radar:

Screenshot 2024-12-04 at 8 45 03 AM Screenshot 2024-12-04 at 9 06 13 AM

Digging into the code I thought maybe flagship->GetTargetSystem()->SolarPower() might work similar to how the system name is called, but it didn't; there's probably some way of using GetTargetSystem() to call the data but I'm just not seeing it yet by reverse engineering code. For now I'll just focus on the system name location for this PR, but here's a mockup of what I presume you're looking for with all the features combined. I can see how that'd be useful when making a string of jumps to decide when to interrupt for a ramscoop detour.

Screenshot 2024-12-04 at 10 38 59 AM

@TheGiraffe3
Copy link

I might prefer the second option as opposed to the first; it feels less cluttered.
The final mock-up looks good, though I wonder if it'd be better without the boxes.

@MadisonianX
Copy link
Author

The second option doesn't match the curve of the radar as nicely as at present but I definitely prefer the system name centered at the top of the radar. Updated with the second option for testing.

data/_ui/interfaces.txt Outdated Show resolved Hide resolved
@Zitchas
Copy link
Member

Zitchas commented Dec 9, 2024

I think I agree that the second one (current system name above the RADAR) is probably the clearer one. It directly connects the RADAR to the name, and thus the current system to the name.

@Zitchas Zitchas added UI Everything related to the User Interface & Design (both artwork and code) enhancement New feature or request labels Dec 9, 2024
@Zitchas
Copy link
Member

Zitchas commented Dec 9, 2024

Code looks good, aside from that one whitespace that I posted a suggestion/fix for. I'm going to test it out, but probably looking to merge this in <24h.

@Zitchas Zitchas merged commit 0ac593a into endless-sky:experimental Dec 11, 2024
23 checks passed
@MadisonianX MadisonianX deleted the System-Name-UI branch December 14, 2024 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request UI Everything related to the User Interface & Design (both artwork and code)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants