fix: searching checks both item name and display name

[lucasser]

Added an itemName and a name variable to AbstractGridResource

If there is a stack of paper named "dinnerbone"
To get the item name "Paper" use resource.getItemName()
To get the display name "dinnerbone" use resource.getDisplayName()

GridQueryParser.parseLiteral() checks both itemName and displayName.

[raoulvdberge]

This would fix #1259, right?

[raoulvdberge]

I'm not really fan of this PR, it introduces 3 variants of "names" in the code. And I have no idea how this fixes the problem of RS searching on the registry name "minecraft:paper", like mentioned in the original issue? Is that something that is fixed here? I see getName on Item is still being used.

[lucasser]

Yes, it solves #1259
This will keep track of two different names, the item registry id/base item name, and the display name.
In most cases the two are the same, unless the item gets renamed.
The parser

        return (repository, resource) -> normalize(resource.getItemName()).contains(normalize(node.token().content()))
            || normalize(resource.getDisplayName()).contains(normalize(node.token().content()));

looks at both names. So if you have a renamed piece of paper it'll come up when you search paper (The current behaviour, useful if you are sorting through renamed Apotheosis gear for example) but also when you search for its name

[raoulvdberge]

Question, in your PR description you mention:

To get the item name "Paper" use resource.getItemName()
To get the display name "dinnerbone" use resource.getDisplayName()

But in the message above, you talk about the registry name. That is not mentioned in your PR description, it's a bit confusing.

I would probably change the logic a bit:

• Return a Set<String> instead of adding more "getName"-esque methods (these "getName"-esque methods are not always applicable to all resource types)
• Search on the item name, display name, and registry ID.

[raoulvdberge]

Could you rebase on the latest develop please?

[raoulvdberge]

Also, please create these lists in the constructor. We don't want a bunch of list allocations each time you search something..

[lucasser]

Done

[raoulvdberge]

How does this work, the test setup hasn't changed? What does this test add

[lucasser]

It tests for the renamed search. assertThat(predicate.test(repository, new R(List.of("Dirt", "test")))).isTrue(); has been renamed to test.

[lucasser]

As in the item used in that assert has been renamed to test, and should show up for all the search strings

[raoulvdberge]

Just consolidate this in the other query test.

[raoulvdberge]

Just consolidate this in the other query test.

[raoulvdberge]

Why getLast here? instead of hoverName

[lucasser]

From one of the review comments:

I renamed them to getSearchableNames and getHoverName. getHoverName is only used in tests to check if the items are correct. Should i just remove it then and have the tests check getSearchableNames().last instead?

Do i keep the method just for tests, or get rid of it?