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

Create JEI-like Advanced Search #485

Merged
merged 14 commits into from
Jun 27, 2024
Merged

Conversation

slprime
Copy link
Member

@slprime slprime commented May 29, 2024

Advanced search:

@ - search by mod (@minecraft)
# - search by tooltip (#UV-Tier)
& - search by id: (&95:15 extratrees:door)
$ - search by ore dictionary ($ore)
% - search by subsets (%block)
| - logical or. multi-item search (wrench|hammer)
- - logical not. exclude items that match the following expression (-@minecraft)
" - multiple words wrapped in quotes are treated as one condition rather than several ("White Stained Glass")

Search Mode (apply on search parts, not full string):

Plain - simple string
Extended - allow only ?, * rules
Regex - allow full regex

In Extended Search Mode:

? - any character (@min?craft)
* - any few characters (@m*ft = @minecraft)
r/.../ - standard java regex (@r/m\w{6}ft/ = @minecraft)

More colors:

image

Settings:

image

P.S.

  1. issue NEI search does not correctly parse string expressions with Mod ID GT-New-Horizons-Modpack#15934
  2. Similar PR (Adopt JEI search tree #480)
  3. WE NEED someone who understands how NotEnoughCharacters works and whether the behavior is preserved. and how much this mod is needed by a new search. I don't know Chinese and can't check.

P.S.2

  1. If this PR is approved i started work with AE2 terminals and holo inventory to update search rules there
  2. If exists ability to get GT ItemStack Tier, add search prefix for it will be excellent

@slprime
Copy link
Member Author

slprime commented Jun 10, 2024

image

move Focus Search Widget on Open config option to Search Widget scope, and rename

@OvermindDL1
Copy link

move Focus Search Widget on Open config option to Search Widget scope, and rename

Just to make sure, the "Search Widget Scope" is a new settings category that does not exist as-is in master right now, correct?

Is there anything in this PR I could assist with in the next few days? Any open sub-issues?

@slprime
Copy link
Member Author

slprime commented Jun 11, 2024

Just to make sure, the "Search Widget Scope" is a new settings category that does not exist as-is in master right now, correct?

yes, scope added in this PR

@vfyjxf
Copy link

vfyjxf commented Jun 11, 2024

  1. WE NEED someone who understands how NotEnoughCharacters works and whether the behavior is preserved. and how much this mod is needed by a new search. I don't know Chinese and can't check.

NotEnoughCharacters just register a SearchField.ISearchProvider to support jei search style and extra search support.
But I may need to change some of the code to be compatible with both the new nei and the old nei.

@vfyjxf
Copy link

vfyjxf commented Jun 11, 2024

And are you using the same tree search as jei? If you ported it, then nech may need additional changes to be more compatible with nei.

@slprime
Copy link
Member Author

slprime commented Jun 11, 2024

And are you using the same tree search as jei? If you ported it, then nech may need additional changes to be more compatible with nei.

No, I don't use the JEI tree. I've changed the search field parser. In the current version, prefixes only worked at the beginning of the search text. Now, it's split into tokens, and an ItemFilter is generated based on them. The only change I made was to deprecate ISearchProvider and add ISearchParserProvider, so that other mods wouldn't break with the new search.

@slprime
Copy link
Member Author

slprime commented Jun 11, 2024

In NotEnoughCharacters exists @, $, & prefixes which are already added the new search

@vfyjxf
Copy link

vfyjxf commented Jun 11, 2024

In NotEnoughCharacters exists @, $, & prefixes which are already added the new search

yes,and chinese support also dependends on ISearchProvider

@slprime
Copy link
Member Author

slprime commented Jun 11, 2024

In NotEnoughCharacters exists @, $, & prefixes which are already added the new search

yes,and chinese support also dependends on ISearchProvider

If I understand the net.minecraft.nechar.NecharSearchFilter class correctly, it is not needed in the new search.

@vfyjxf
Copy link

vfyjxf commented Jun 11, 2024

If I understand the net.minecraft.nechar.Nechar SearchFilter class correctly, it is not needed in the new search.

No, matchesKeywords method provide support of chinese pinyin search

@vfyjxf
Copy link

vfyjxf commented Jun 11, 2024

@slprime also,can you provide an api that allows a certain item filter to be enabled only if the game is in a certain language, like the input method api for rei.

@slprime
Copy link
Member Author

slprime commented Jun 11, 2024

@slprime also,can you provide an api that allows a certain item filter to be enabled only if the game is in a certain language, like the input method api for rei.

yes, I'll add it after work

@slprime
Copy link
Member Author

slprime commented Jun 11, 2024

@vfyjxf, you need api which apply custom ISearchParserProvider only if the game is in a certain language, else use default ISearchParserProvider? correct?

@vfyjxf
Copy link

vfyjxf commented Jun 11, 2024

@vfyjxf, you need api which apply custom ISearchParserProvider only if the game is in a certain language, else use default ISearchParserProvider? correct?

YES, it helps nech not to affect non-Chinese users.

@slprime
Copy link
Member Author

slprime commented Jun 13, 2024

@vfyjxf, for example

      @Override
      public List<Language> getMatchingLanguages() {
        return ISearchParserProvider.getAllLanguages().stream()
                .filter(lang -> lang.getLanguageCode().startsWith("zh_"))
                .collect(Collectors.toCollection(ArrayList::new));
      }
API.addSearchProvider(
        new SearchParserProvider(
                '\0',
                "default",
                EnumChatFormatting.RESET,
                (pattern) -> new PatternItemFilter(pattern)) {

            @Override
            public SearchMode getSearchMode() {
                return SearchMode.ALWAYS;
            }

            @Override
            public List<Language> getMatchingLanguages() {
                return ISearchParserProvider.getAllLanguages().stream()
                        .filter(lang -> lang.getLanguageCode().startsWith("zh_"))
                        .collect(Collectors.toCollection(ArrayList::new));
            }

        });

API.addSearchProvider(
        new SearchParserProvider(
                '#',
                "tooltip",
                EnumChatFormatting.YELLOW,
                (pattern) -> new TooltipFilter(pattern)) {

            @Override
            public List<Language> getMatchingLanguages() {
                return ISearchParserProvider.getAllLanguages().stream()
                        .filter(lang -> lang.getLanguageCode().startsWith("zh_"))
                        .collect(Collectors.toCollection(ArrayList::new));
            }
        });

@ah-OOG-ah
Copy link
Member

  1. How much of a change is this from the default search?
  2. Is this behind a config, or on by default?

@chochem
Copy link
Member

chochem commented Jun 17, 2024

Just to be clear: do I understand correctly that this changes the connection between terms from OR to AND? Then one gets OR instead with |?

I am not even saying I hate that, but that was one of the reasons NECh was rejected.

@chochem
Copy link
Member

chochem commented Jun 17, 2024

there seems to be another issue, in that it no longer searches the internal item names which results in very different results. This is certainly going to be controversial.

here is a before/after:

image

Edit: I have been told that this is what you meant by id in your description. Which is well definetely not correctly named then, But adding & does indeed work.

@chochem
Copy link
Member

chochem commented Jun 17, 2024

here is another issue from testing: I cant search for blood magic items (assuming I am a player who doesnt know the secret password is @awwayof). something like #"blood magic" does not work. I guess the tooltip search is somewhat broken or am I doing something wrong?

@slprime
Copy link
Member Author

slprime commented Jun 18, 2024

Prefix Schema:

image

Item Subset (%). In previous version you use @ to search by this list. Now @ usage for modname (jei style)

image

If you want restore NEI search, you may change config like in image below

image

But have some breaking changes:

  1. to search by subsets you need to use %, not @. Or you can disable the prefix for % and use @ as before. It’s unlikely that you used the like @block search often.
  2. | cannot be disabled.

@slprime
Copy link
Member Author

slprime commented Jun 18, 2024

here is another issue from testing: I cant search for blood magic items (assuming I am a player who doesnt know the secret password is @awwayof). something like #"blood magic" does not work. I guess the tooltip search is somewhat broken or am I doing something wrong?

fix modname. please use @"blood magic"

image

@slprime
Copy link
Member Author

slprime commented Jun 18, 2024

Edit: I have been told that this is what you meant by id in your description. Which is well definetely not correctly named then, But adding & does indeed work.

I used naming from JEI/REI

@slprime
Copy link
Member Author

slprime commented Jun 18, 2024

Just to be clear: do I understand correctly that this changes the connection between terms from OR to AND? Then one gets OR instead with |?

I am not even saying I hate that, but that was one of the reasons NECh was rejected.

image

NEI Search Style (old style). | implement NECh

Item Name/Tooltip/Mod Name include:
rune<space>of<space>super<space>|<space>green<space>backpack

JEI/REI Search Style (new style). | split search to 2 search rules. like OreDict Card

Item Name include: run<or>of<or>super
or Item Name include: green <or> backpack

p.s. sorry for my english )

@slprime
Copy link
Member Author

slprime commented Jun 18, 2024

  1. How much of a change is this from the default search?
  2. Is this behind a config, or on by default?
  1. previous search looked for in full tooltip. now only in display name (if you don't use prefixes)
  2. Create JEI-like Advanced Search #485 (comment)

@evgengoldwar
Copy link

Fantastic change. The new versions of Jei have done the same thing, and it's much more convenient than what Nei has now. Looking forward to seeing this added to the game.

@slprime
Copy link
Member Author

slprime commented Jun 19, 2024

leaving two search options is a bad idea. the code will become much more complex. the new search can work like the old search if you change some settings (in the comment above), not 100% but very close. if you search by name/tooltip/mod name/id or use @ to search for a mod, that will enough

@slprime
Copy link
Member Author

slprime commented Jun 19, 2024

image

@slprime
Copy link
Member Author

slprime commented Jun 19, 2024

How search works (3:50-9:30) https://youtu.be/c77g2imldpk?si=m37tmz3pN_CUAF_n&t=229

@vfyjxf
Copy link

vfyjxf commented Jun 20, 2024

@slprime I would like to ask if this situation is a bug? IConfigureNEI.loadConfig is later than SearchTokenParser.getProviders, usually mod authors choose to register ISearchParserProvider via NEIConfig, but SearchTokenParser has kept a cache before that, which causes ISearchParserProvider registered by other mods to be unavailable unless the player switches languages and clears the cache.

@vfyjxf
Copy link

vfyjxf commented Jun 20, 2024

And how do I override the original ISearchParserProvider, obviously nech needs to override the prefix-less and tooltip ISearchParserProvider to support its additional search functionality, mixin is probably not a good idea.

@slprime
Copy link
Member Author

slprime commented Jun 20, 2024

And how do I override the original ISearchParserProvider, obviously nech needs to override the prefix-less and tooltip ISearchParserProvider to support its additional search functionality, mixin is probably not a good idea.

yes it was a bug

@slprime
Copy link
Member Author

slprime commented Jun 20, 2024

SearchParserProvider

And how do I override the original ISearchParserProvider, obviously nech needs to override the prefix-less and tooltip ISearchParserProvider to support its additional search functionality, mixin is probably not a good idea.

you need registry API.addSearchProvider like in this example or not suitable?
#485 (comment)

@vfyjxf
Copy link

vfyjxf commented Jun 20, 2024

SearchParserProvider

And how do I override the original ISearchParserProvider, obviously nech needs to override the prefix-less and tooltip ISearchParserProvider to support its additional search functionality, mixin is probably not a good idea.

you need registry API.addSearchProvider like in this example or not suitable? #485 (comment)

but this reject my provider i think

@slprime
Copy link
Member Author

slprime commented Jun 20, 2024

SearchParserProvider

And how do I override the original ISearchParserProvider, obviously nech needs to override the prefix-less and tooltip ISearchParserProvider to support its additional search functionality, mixin is probably not a good idea.

you need registry API.addSearchProvider like in this example or not suitable? #485 (comment)

but this reject my provider i think

you provider should be added after the default. this condition searches for the first from the end

Copy link

@Ethryan Ethryan left a comment

Choose a reason for hiding this comment

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

Seems to be working as intended.
And I havn't run into any issues during my testing.

@Dream-Master Dream-Master merged commit b1a34c7 into GTNewHorizons:master Jun 27, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants