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: build vehicle name filter #8984

Closed
wants to merge 2 commits into from
Closed

Conversation

perezdidac
Copy link
Contributor

Motivation / Problem

As discussed in #8723, users will benefit from having the option to filter the Build Vehicle list by typing a string. This would be especially useful when playing with NewGRF that include tons of vehicles.

Description

This change adds a textbox to the Build Vehicle window so users can enter a keyword to find vehicles that match that keyboard. This works for all the types of vehicles and it's compatible with the existing filters.

This is what it looks like:

image

The text boxes do not immediately get focus to not break user's hotkeys.

Limitations

Nope.

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')

@stormcone
Copy link
Contributor

The text boxes do not immediately get focus to not break user's hotkeys.

A hotkey can be added to make the edit box get the focus, like in #8908. Of course it could be in a separate commit or PR. :)

@LordAro
Copy link
Member

LordAro commented Apr 11, 2021

Seems to me that the hotkey can be rolled into this PR, probably as a separate commit

@perezdidac
Copy link
Contributor Author

Seems to me that the hotkey can be rolled into this PR, probably as a separate commit

There you go!

@perezdidac
Copy link
Contributor Author

ping ping!

@perezdidac
Copy link
Contributor Author

Hi devs, could you make a deployment so people can test this feature?

@TrueBrain TrueBrain added the preview This PR is receiving preview builds label Apr 20, 2021
@DorpsGek DorpsGek temporarily deployed to preview-pr-8984 April 20, 2021 19:11 Inactive
@frosch123
Copy link
Member

frosch123 commented Apr 21, 2021

NewGRF often put some extra properties/text into the panel below the list.
How about the filter would check multiple texts:

  • Vehicle name: search for "bristol"
  • Refittable cargotypes: search for "coal"
  • Extra NewGRF text: search for "freight" (as used in "type of service" in the screenshot)

@perezdidac
Copy link
Contributor Author

NewGRF often put some extra properties/text into the panel below the list.
How about the filter would check multiple texts:

  • Vehicle name: search for "bristol"
  • Refittable cargotypes: search for "coal"
  • Extra NewGRF text: search for "freight" (as used in "type of service" in the screenshot)

Responding to your request on IRC, I tried having it below and I gotta say I don't quite like the way it looks
image

Regarding adding the other stuff for the filter to match, it's quite a bit of work, for cargo types a similar approach to ShowRefitOptionsList() needs to be used, and for NewGRF I tried a similar approach from ShowAdditionalText() but most of the strings ended up with "(undefined string)" on them, I am not sure why. I would love to push these new requests for the filters to a separate PR if that's ok for you.

@frosch123
Copy link
Member

Hmm, a text filter for cargo-refittability is probably a bad idea after all. It won't work with build+refit, so would readd the old confusion about that.

@perezdidac
Copy link
Contributor Author

Hmm, a text filter for cargo-refittability is probably a bad idea after all. It won't work with build+refit, so would readd the old confusion about that.

Yeah I kinda agree with that. What do you think of the filter in a third row vs with the rest above?

@2TallTyler
Copy link
Member

I agree that filtering refittable cargos is not a good use of this filter. There's already a drop-down for that.

My concern with the current implementation of the filter textbox to the right of the cargo types dropdown is that it makes said dropdown very narrow. Long cargo names (such as in Paints & Coatings in FIRS 4 Steeltown) are cut off. The full row textbox in the most recent screenshot isn't particularly attractive but it does avoid this issue.

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 work: needs more work This Pull Request needs more work before it can be accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants