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

Feature: text filter for the industry directory #8982

Closed
wants to merge 2 commits into from

Conversation

perezdidac
Copy link
Contributor

@perezdidac perezdidac commented Apr 9, 2021

Motivation / Problem

The industry directory and town directory windows have tons of similarities. However, only the town directory window has a filter.

Description

This change adds a filter to the industry directory window. This is especially useful in situations where a user wants to find industries by name, like typing the type rather than using the filters or, even more useful, find industries around a specific city or town. This can be specially useful in large maps and in games where the player customizes town/city names so they are familiar with where they are, etc.

I believe this change doesn't really hurt much since there is some UI real estate available already, and this makes both town and industry directory windows even more consistent.

Here is what it looks like:

image

This was discussed here: #8723

Testing

I play around by making all sort of UI interactions and in different order, such as filtering first by accepted cargo, then filtering by text, then clearing the editbox, etc. It all seems to work great.

Limitations

None

Checklist for review

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

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

src/industry_gui.cpp Outdated Show resolved Hide resolved
src/industry_gui.cpp Outdated Show resolved Hide resolved
LordAro
LordAro previously approved these changes Apr 11, 2021
Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

Code looks good

@LC-Zorg
Copy link

LC-Zorg commented Apr 11, 2021

This is a useful feature, but with the layout you used there is a problem with the width of this window. The addition of the cargoes filtering option has already expanded the window significantly. Maybe try moving these two filters to the second line? Then the window would be even narrower and smaller. Take into account that there are language versions with much longer names than the English ones - then the window will be even wider.

Industries List window with text filter 1

@perezdidac
Copy link
Contributor Author

This is a useful feature, but with the layout you used there is a problem with the width of this window. The addition of the cargoes filtering option has already expanded the window significantly. Maybe try moving these two filters to the second line? Then the window would be even narrower and smaller. Take into account that there are language versions with much longer names than the English ones - then the window will be even wider.

Industries List window with text filter 1

Done! :) thank you!

src/industry_gui.cpp Outdated Show resolved Hide resolved
@PeterN
Copy link
Member

PeterN commented May 1, 2021

All other windows that have a search filter make filter fill the available space. So why not make this filter fill the available space?

@2TallTyler 2TallTyler added preview This PR is receiving preview builds size: small This Pull Request is small, and should be relative easy to process labels Nov 7, 2022
@DorpsGek DorpsGek temporarily deployed to preview-pr-8982 November 7, 2022 20:38 Inactive
@2TallTyler 2TallTyler added the work: minor details This Pull Request has some minor details left to do label Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

8 participants