Skip to content
This repository has been archived by the owner on Dec 18, 2017. It is now read-only.

New icon-based ameneties panel #4

Merged
merged 3 commits into from
Oct 29, 2016
Merged

New icon-based ameneties panel #4

merged 3 commits into from
Oct 29, 2016

Conversation

olegbl
Copy link

@olegbl olegbl commented Oct 29, 2016

@chaorace
Copy link
Member

chaorace commented Oct 29, 2016

💥 💥 💥

This is actually amazing! I didn't think it would be possible to so closely replicate that concept art and you've done a fantastic job of it. I'm gonna play with it first just to make sure all is well, but expect this to get merged today.

Copy link
Member

@chaorace chaorace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nearly perfect, just need to address how to handle line wrapping before proceeding!

<Image ID="AmenetiesStatusIconContainerEntertainment" Anchor="C,C" Size="50,50" Texture="CityPanel_StatusIcon">
<Image ID="AmenetiesStatusIconEntertainment" Anchor="C,C" Offset="-2,-3" Size="35,35" Texture="Stats45" Icon="ICON_PROJECT_CARNIVAL" />
<Label ID="AmenetiesStatusYieldEntertainment" Anchor="C,C" Offset="-2,0" Style="CityPanelSubPanelTitle" String="1" />
<Label ID="AmenetiesStatusLabelEntertainment" Anchor="C,B" Offset="0,-5" Style="UnitPanelStatLabel" Align="Center" String="Entertainment" WrapWidth="parent+16" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entertainment label in 1920x1080 windowed mode seems to have an orphaned letter.

Overall, I dislike using linewraps here because of the potential for issues like these and also the nasty vertical positioning issues it seems to cause wherever it kicks in, like with Luxury Resources. I think the better approach would be to abandon using the WrapWidth property entirely and instead using a "[NEWLINE]" wherever we anticipate that a line is too long for the label. This gives us the freedom to anticipate which labels will take up two lines and tweak the offsets appropriately. Here's an example:

<Label ID="AmenetiesStatusLabelLuxuries" Anchor="C,B" Offset="0,-15" Style="UnitPanelStatLabel" Align="Center" String="Luxury[NEWLINE]Resources"/>

What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's other solutions as well, like using strings that we anticipate won't need wrapping, like truncating "Luxury Resources" to "Luxuries".

Wrapping can happen in weird places depending on screen resolution and
UI scaling option. So, just [NEWLINE] it instead. Also moves the labels
down to below the circles.
@olegbl
Copy link
Author

olegbl commented Oct 29, 2016

Sounds reasonable. I kind of like the labels being below the circles anyway, and that would fix the vertical alignment issue too. Something like this:
image

Copy link
Member

@chaorace chaorace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one last thing!

<Stack StackGrowth="Down" Anchor="C,C">
<Stack StackGrowth="Right" Padding="16">
<Image ID="AmenetiesStatusIconContainerLuxuries" Anchor="C,C" Size="50,50" Texture="CityPanel_StatusIcon">
<Image ID="AmenetiesStatusIconLuxuries" Anchor="C,C" Offset="-2,-3" Size="35,35" Texture="Stats45" Icon="ICON_IMPROVEMENT_PLANTATION" />
Copy link
Member

@chaorace chaorace Oct 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one last thing! Let's replace the irrigation icon for the luxuries bubble with a more appropriate resort icon before we merge this in.

The "Seaside Resort" improvement icon is actually called (confusingly) "ICON_IMPROVEMENT_BEACH_RESORT".

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I'm not sure what a resort has to do with luxury resources (AFAIK, it just gives more gold yield and a tourism output), but I agree that there isn't a good icon for luxury resources (all the resource icons are colored) - so I don't mind replacing it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, didn't mean to impose on your agency over your work. Please feel free to say no to me, I won't bite!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, I know. I just don't think there is a good icon, so I don't have a strong opinion. The beach resort icon looks significantly nicer but is less relevant. I think the labels largely mitigate that UX issue though.

…anel

It may be cleaner than the plantation icon and unfortunately there isn't
a good luxury resource icon that would actually be appropriate.
@chaorace chaorace merged commit ebfee83 into CQUI-Org:master Oct 29, 2016
@chaorace
Copy link
Member

Congratulations on the fantastic work! I can't wait to see how people react when they see the new menu in the next release. Thank you for your contribution

@olegbl olegbl deleted the icon-based-ameneties-panel branch October 30, 2016 19:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants