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

Unified GUI buttons for renaming and scrolling to entities #8455

Merged
merged 5 commits into from Jan 11, 2021

Conversation

frosch123
Copy link
Member

@frosch123 frosch123 commented Dec 27, 2020

Motivation / Problem

Many windows have some button to rename the entity, scroll to the entity, or open an extra viewport at the entity's location.
Previous ideas to unify or change this are:

Description

This PR adds small icon buttons to the window title bars:

  • Left to the caption: a button for renaming the entity.
  • Right of the caption: a button for the entity's location.
    • Click scrolls to the entity's location.
    • Ctrl-click opens an extra viewport.
    • Vehicles only: double-click follows the vehicle.
  • The vehicle GUI gains another button for scrolling to the current order's target location.

Overview of changes:

Function Old button New button
Rename depot/station/town/waypoint Button at bottom Button in window title
Rename vehicle Button in vehicle detail's window title Button in vehicle view's window title
Scroll to depot/industry/station/town/waypoint location Button at bottom, click / ctrl-click Button in window title, same click / ctrl-click
Vehicle location Button on the right, click to scroll, ctrl-click to follow Button in window title, click to scroll, ctrl-click for viewport, double-click to follow
Vehicle order location Ctrl-click on vehicle status button right of vehicle status, click / ctrl-click like in other windows
LandInfo, Signs no scroll button Button in window title, click / ctrl-click

Limitations

  • The sprites are placeholder sprites. If the concept is liked, maybe someone draws prettier ones.
  • Company window is not changed. This needs more thought on the whole window, which would warrent a new PR.
    • Location could be the head quarter.
    • There is the company name and the manager name.
    • The current buttons for both are pretty weird already.
  • News window is not changed for now. Same deal, if changed, then in a new PR.
    • News items also refer to locations. Sometimes one, sometimes two.
    • They do not quite fit the schema of the other windows though.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

@frosch123 frosch123 added the needs review: NewGRF Review requested from a NewGRF expert label Dec 27, 2020
@frosch123 frosch123 changed the title Windowbuttons Unified GUI buttons for renaming and scrolling to entities Dec 27, 2020
@TrueBrain TrueBrain added the preview This PR is receiving preview builds label Dec 27, 2020
@DorpsGek DorpsGek temporarily deployed to preview-pr-8455 December 27, 2020 22:46 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-8455 December 27, 2020 22:48 Inactive
@2TallTyler
Copy link
Member

I quite like the new scroll buttons. Can't comment on renaming things as I almost never do so.

Your idea of having the company window location scroll to the HQ makes sense to me.

@TrueBrain
Copy link
Member

I really like what you did here! It is very awesome :D

Code looks good; I am surprised it is only this few amount of lines of code ..

There is, however, one thing that pisses off my OCD: the rename icon:

image

It is not centered :( Also, I did not expect the letter A, honestly. I was more expecting a pencil. But that might be fully my bias. Here are some other icons people use for rename:
https://icons8.com/icons/set/rename

But, as always, this is purely personal preference. The alignment, however, is not :D Sorry :P

@TrueBrain TrueBrain added candidate: yes This Pull Request is a candidate for being merged size: small This Pull Request is small, and should be relative easy to process work: minor details This Pull Request has some minor details left to do and removed needs review: NewGRF Review requested from a NewGRF expert labels Dec 28, 2020
@2TallTyler
Copy link
Member

I second the pencil suggestion, although the 'A' doesn't bother me.

Also, other icons (close, pin, etc) only use black and are a higher resolution. I know these are stand-in sprites, but it's something to keep in mind when choosing the final graphics.

@TrueBrain TrueBrain added this to the 1.11.0 milestone Jan 5, 2021
@andythenorth
Copy link
Contributor

andythenorth commented Jan 5, 2021

Not-very-good-yet pencil and eye icons.
pencil-eye-icons-8455

@frosch123
Copy link
Member Author

Icons in context of the game.

  • frosch' icons
    windowicons1

  • andy's icons
    windowicons2

@TrueBrain
Copy link
Member

Eye-icon of the first is much more clear to me (the second doesn't really look like an eye, sorry ..). Pencil both look good to me .. second maybe slightly better than the first. My 2 cents :)

@andythenorth
Copy link
Contributor

Eyelashes. Yeah they look bad. The idea was to make the shape sit better inside the square bounding box.
eye-lol

@frosch123
Copy link
Member Author

frosch123 commented Jan 5, 2021

More icons:
windowicons4

@andythenorth
Copy link
Contributor

Binoculars! Badoculars!
eye-lol-2

@mattkimber
Copy link
Contributor

Other icon ideas... not sure either of them are particularly good or convey meaning well, but I may as well contribute something.

tv

camera

@andythenorth
Copy link
Contributor

Legendary target icon...
eye-lol-3

@andythenorth
Copy link
Contributor

andythenorth commented Jan 5, 2021

Weird non-standard target-icon snowflake thing I invented.
eye-lol-4

@andythenorth
Copy link
Contributor

Map pin oof
eye-lol-5

@frosch123
Copy link
Member Author

I cannot keep up...
windowicons5

@Wolfolo
Copy link

Wolfolo commented Jan 5, 2021

image
Not really liking them, but here are my entries too

@mattkimber
Copy link
Contributor

I've had a play with the film camera too...

tv_cam

@frosch123
Copy link
Member Author

Enough options now?
windowicons6

@andythenorth
Copy link
Contributor

Last go, random made up tyre-iron (started as 4 inward pointing arrows, but they don't fit) 😺
eye-lol-6

@mattkimber
Copy link
Contributor

Inward pointing arrows, I think it looks a bit too light though.
image

@LC-Zorg
Copy link

LC-Zorg commented Jan 9, 2021

Hi
As this is quite important to me, I would have a few comments.

Name change
It's not bad, but is it really necessary? I am neither for nor against. Perhaps I will become more convinced for it over time.
Meanwhile, I think Andy's pencil looks better. Possibly, I will also add my masterpiece below to choose from. ;)

Location
I don't like this change very much. I'm not entirely against that, I see the point, but there are quite a few arguments against.

First of all: the eye icon - however it will be, I wouldn't want to press it. I really don't. Unfortunately, the other icons also don't look convincing. The TVs and camcorders look nice, but have a weak link to the function of the button.
Second: for vehicles, this is a feature that I use quite often and the button size and location are not convenient.
Third: what about the location of the current icon on the right side of the vehicle window? Blank space? Vehicle window reduced?
Fourth: not only me, but probably other players also got used to the current layout (vehicle window).

In the case of vehicles, I would not change the current state - it is not worth it, it would only be a change for the worse. In the case of other windows, I am least not not convinced to the map location / pin icon - it is not beautiful, it fits weakly with the rest, but it is logical and suits the function best. A snowflake would also be an option - it looks a bit like a selection on a map. Possibly I would also have such an icon...
Icons of rename and location v02

It is still not beautiful, but it looks similar to the current icon in the vehicle window. It is visually bigger, but I think it's good - the remaining icons concern changes within the window itself, while this one changes the view of the game, so it should, or at least, can be slightly different.
The graphic also shows my variation on the pencil. ;)

By the way, it's a bit offtop, but since it relates to these changes, I would like to ask, have you thought about a little retouching of some of the current icons? In the graphics, I slightly improved the others:

  • X greater by 1 in each direction
  • square larger by 2 pixels (4> 6)
  • clearer pin
  • centered scroll arrows
    These are small changes to the standard GUI size that would improve readability a bit. Another possible change - also referring to the topic - could be a slight enlargement of the button area of ​​the top bar by 2 pixels (h: 14> 16). This is not a big difference, but in the case of today's average resolution and screen size for the laptops (15" FHD), where the double size is still too big, such a change would be beneficial.

@frosch123
Copy link
Member Author

frosch123 commented Jan 9, 2021

@LC-Zorg: The icons added here have to fit with the original base graphics.
You seem to use some different baseset. OpenGFX and friends can define their own sprite, and do not have to use the one added here.

@LC-Zorg
Copy link

LC-Zorg commented Jan 9, 2021

This is original graphics, but as I wrote, I changed a few icons a bit.
This is what they look like in combination with the unchanged:
Icons of rename and location v00

@glx22
Copy link
Contributor

glx22 commented Jan 9, 2021

Some observations after a quick test:

  • WID_VV_ORDER_LOCATION tooltip is not filled later.
  • STR_VEHICLE_VIEW_.*_STATE_START_STOP_TOOLTIP need an update.

@frosch123 frosch123 added the needs review: NewGRF Review requested from a NewGRF expert label Jan 11, 2021
@DorpsGek DorpsGek temporarily deployed to preview-pr-8455 January 11, 2021 19:33 Inactive
@frosch123
Copy link
Member Author

I cleaned up the tooltip texts, decided for the icon, and resized the icon so it fits in with the original baseset.
This PR is ready now.

@TrueBrain
Copy link
Member

Did all kinds of testing with all the windows changed; looks really good to me, couldn't find any weirdness. This is an OK for me.

I did not approve, as @glx22 had some open comments I could not parse so not validate that they were fixed. If he is happy with it, let's merge this! :)

Copy link
Contributor

@glx22 glx22 left a comment

Choose a reason for hiding this comment

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

I'm happy with the fixed tooltips

@frosch123 frosch123 merged commit e339188 into OpenTTD:master Jan 11, 2021
frosch123 added a commit to frosch123/nml that referenced this pull request Jan 30, 2021
FLHerne pushed a commit to OpenTTD/nml that referenced this pull request Jan 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
candidate: yes This Pull Request is a candidate for being merged needs review: NewGRF Review requested from a NewGRF expert preview This PR is receiving preview builds size: small This Pull Request is small, and should be relative easy to process work: minor details This Pull Request has some minor details left to do
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants