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: Filter engine build menu by name and NewGRF extra text #10519

Merged
merged 1 commit into from May 1, 2023

Conversation

2TallTyler
Copy link
Member

@2TallTyler 2TallTyler commented Feb 28, 2023

Motivation / Problem

In #8984, @perezdidac started the work of adding a text filter to the build vehicle menu, but hasn't been touched in 10 months. Let's finish it.

Description

Implements #8984 with a change to put the editbox below the other lines:

filter

I added the ability to search NewGRF extra text, which is often used to describe a vehicle's role:

extra_text

It also works with the vehicle name callback added in #10399 (there's an example trainset in that PR):

name_callback

If variants are used (unreleased Iron Horse 2 in this screenshot), a subvariant that meets the filter, nested inside a parent does not, will show up underneath its greyed-out parent variant, collapsed by default.

variants

Closes #8984.

Limitations

I tried swapping the cargo filter dropdown and the editbox in the GUI, and the cargo filter all by itself looked much more wrong than a full-width textbox (which we have elsewhere in the GUI, including the object menu).

swapped_lines

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 touches english.txt or translations? Check the guidelines
  • 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')

@2TallTyler 2TallTyler added preview This PR is receiving preview builds work: still in progress (draft) This Pull Request is a draft labels Feb 28, 2023
@DorpsGek DorpsGek temporarily deployed to preview-pr-10519 February 28, 2023 01:17 Inactive
@audigex
Copy link

audigex commented Feb 28, 2023

2tall

Additional Text length testing - appears to be 2046 characters

@audigex
Copy link

audigex commented Feb 28, 2023

It would be nice if the nested results were expanded down the tree to the search result, I think? Not a huge issue, but that would seem more intuitive to me

src/build_vehicle_gui.cpp Outdated Show resolved Hide resolved
src/build_vehicle_gui.cpp Fixed Show resolved Hide resolved
@DorpsGek DorpsGek temporarily deployed to preview-pr-10519 February 28, 2023 13:17 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-10519 March 3, 2023 21:51 Inactive
@2TallTyler 2TallTyler added size: small This Pull Request is small, and should be relative easy to process needs review: NewGRF Review requested from a NewGRF expert and removed work: still in progress (draft) This Pull Request is a draft labels Mar 3, 2023
@2TallTyler 2TallTyler marked this pull request as ready for review March 3, 2023 22:04
src/build_vehicle_gui.cpp Outdated Show resolved Hide resolved
src/build_vehicle_gui.cpp Outdated Show resolved Hide resolved
src/stringfilter_type.h Outdated Show resolved Hide resolved
src/stringfilter.cpp Outdated Show resolved Hide resolved
src/build_vehicle_gui.cpp Outdated Show resolved Hide resolved
src/build_vehicle_gui.cpp Outdated Show resolved Hide resolved
src/build_vehicle_gui.cpp Outdated Show resolved Hide resolved
src/build_vehicle_gui.cpp Outdated Show resolved Hide resolved
src/build_vehicle_gui.cpp Outdated Show resolved Hide resolved
src/build_vehicle_gui.cpp Outdated Show resolved Hide resolved
src/stringfilter.cpp Outdated Show resolved Hide resolved
@DorpsGek DorpsGek temporarily deployed to preview-pr-10519 April 29, 2023 19:32 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-10519 May 1, 2023 16:22 Inactive
@2TallTyler 2TallTyler removed the needs review: NewGRF Review requested from a NewGRF expert label May 1, 2023
Copy link
Contributor

@rubidium42 rubidium42 left a comment

Choose a reason for hiding this comment

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

Looks okay to me.

@2TallTyler 2TallTyler merged commit aa8830f into OpenTTD:master May 1, 2023
19 checks passed
@2TallTyler 2TallTyler deleted the filt branch May 1, 2023 17:06
mrmbernardi pushed a commit to mrmbernardi/OpenTTD that referenced this pull request May 1, 2023
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants