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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

@perezdidac
Copy link
Contributor

@perezdidac perezdidac commented Apr 10, 2021

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')
src/build_vehicle_gui.cpp Outdated Show resolved Hide resolved
@stormcone
Copy link
Contributor

@stormcone stormcone commented Apr 10, 2021

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

@perezdidac perezdidac commented Apr 11, 2021

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

@perezdidac perezdidac commented Apr 17, 2021

ping ping!

@perezdidac
Copy link
Contributor Author

@perezdidac perezdidac commented Apr 20, 2021

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

@frosch123
Copy link
Member

@frosch123 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

@perezdidac perezdidac commented Apr 22, 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)

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

@frosch123 frosch123 commented Apr 22, 2021

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

@perezdidac perezdidac commented Apr 23, 2021

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants