Add getBrush helper for use with instanceof pattern matching

[rainbowdashlabs]

Motivation

While working on a custom brush with the world api I developed several utility methods to make it easier working with world edit tools and especially brush tools.
I also noticed that some methods which would be nice to avoid boilerplate code are missing. This PR adds these methods which hopefully will help people working with brushes and tools.

Changes

Method to check tool bounding

boolean isTool(ItemType)
boolean isBrushTool(ItemType)
I added two methods which allows to check if a tool or a brush tool is bound on an item.
Previously the only check for a brush tool was to retrieve the tool and make an instance check. Retrieving the brush tool always creates a brush tool and makes it not possible to check for existence directly. This use case should be covered by the api actually.

Method to get tools based on their assignable class

<T extends Tool> Optional<T> getTool(ItemType, Class<T>)
One step further would be to also remove a second instance check which would be often on the user side.
A user might want to get its registered tool and needs to check if the returned tool matches the class of its tool.
The tool is retrieved by this method and checked for the required tool class.
The result already contains the tool cast to the requested class.
The result is wrapped into an optional to make the user aware of possible missing return values.

Method to get a brush based on their assignable class

<T extends Brush> Optional<T> getBrush(ItemType, Class<T>)
This method is a extension of the previous getTool method.
It checks for a BrushTool, extracts the containing brush and checks if the brush is assignable to the provided class.

I am a bit unsure where the logic of this class should be. It is more a delegate a to the brush class and part of the actual logic could also be included in the BrushTool class itself. However I think that dividing it wouldn't be a benefit so I left it in the LocalSession for now.

Gradle

In order to build and test my changes I needed to update to 7.2. I can remove this change, but it may be required to pass the actions.

[me4502]

Thanks - this isn't a proper review as it's nearly midnight and I'm doing this from my phone, but my two immediate concerns are:

• This increases the number of calls to getTool in a few methods, would have to check performance implications
• We do have plans to overhaul tools in the future, and we'd have to ensure that these methods don't interfere with that - as adding methods just to deprecate them is non-ideal

[rainbowdashlabs]

This increases the number of calls to getTool in a few methods, would have to check performance implications

You could reduce them to one when you would use optionals. This way you would only require one lookup. I already thought about that, but that would actually add even more methods and I didn't wanted to bloat the class even more.

    /**
     * Get the tool assigned to the item if a tool is assigned.
     *
     * @param item the item type
     * @return A optional holding the tool if a tool is bound.
     */
    public Optional<Tool> getToolIfBound(ItemType item) {
        return Optional.ofNullable(tools.get(item));
    }

This method could reduce the calls to 1.

Or we stick to the nullable method and doing null checks. Both would work nearly the same way. I just like Optionals since they have a clearer communication to api users

[wizjany]

honestly not a fan of jamming optionals in to parts of the api that don't use them at all.

[rainbowdashlabs]

honestly not a fan of jamming optionals in to parts of the api that don't use them at all.

Actually thought someone has to start it. But if you prefere Nullable annotations over Optionals I can stick with it. At the end it's just a suggestion from my side.

[octylFractal]

Because master/7.3.0-SNAPSHOT is on 16 now anyways, I think most of this PR is unnecessary. Here are my proposals:

1. Deprecated getBrushTool. There's really no need for this any more, given item (4) below.
2. To replace (1), add Brush getBrush(ItemType).
3. Remove the is methods from your PR, I don't think they're really needed when a simple null-check suffices.
4. Remove T getTool(ItemType item, Class<T> toolClass) -- this can be done with getTool(item) instanceof T tool already, which does the null-check and cast in one go
5. getBrush(ItemType item, Class<T> brushClass) can now similarly be handled with getBrush(item) instanceof B brush (see 2).

This dramatically lowers the API surface, while still improving the API to be usable.

P.S. If you don't understand what instanceof T t is, read JEP 394. It's super cool!

[rainbowdashlabs]

Because master/7.3.0-SNAPSHOT is on 16 now anyways, I think most of this PR is unnecessary. Here are my proposals:

1. Deprecated `getBrushTool`. There's really no need for this any more, given item (4) below.

2. To replace (1), add `Brush getBrush(ItemType)`.

3. Remove the `is` methods from your PR, I don't think they're really needed when a simple `null`-check suffices.

4. Remove `T getTool(ItemType item, Class<T> toolClass)` -- this can be done with `getTool(item) instanceof T tool` already, which does the null-check and cast in one go

5. `getBrush(ItemType item, Class<T> brushClass)` can now similarly be handled with `getBrush(item) instanceof B brush` (see 2).

This dramatically lowers the API surface, while still improving the API to be usable.

P.S. If you don't understand what instanceof T t is, read JEP 394. It's super cool!

Sounds like a good change tho.
I wasn't aware that you are already on java 16. I was a bit confused since your contribution file still says java 8 for compilation and source :D
Pattern matching for instance checks is amazing indeed.

1. If you want to deprecate this logic it would be great since I think registering a default brush without explicit implying it with the method name is really confusing.

2. This function is nullable like the get tool I guess and just adds a instance check right?
   Do you want still provide a convenience method to return a already registered BrushTool or should users just start with using the BrushTool constructor? I think it is rarely used until now.

3. With the change to java 16 this is indeed not required anymore. Looking forward to see java 16 on more servers.

4. and 5. Yes.

[octylFractal]

This function is nullable like the get tool I guess and just adds a instance check right?

Yes, return getTool(item) instanceof BrushTool tool ? tool.getBrush() : null;.

Do you want still provide a convenience method to return a already registered BrushTool or should users just start with using the BrushTool constructor? I think it is rarely used until now.

I'm not sure yet, I'll consider after this PR what to do with everything using the deprecated method. Don't worry about cleaning that up for now, just apply the deprecation.

[rainbowdashlabs]

I applied the changes.

Regarding the failing workflows: I don't know in which way these are related to my changes. I don't see any changes which could cause these.

[octylFractal]

oof, outdated checkstyle. if you can figure that out, push it with this, otherwise i'll just fix that later.

[codecov]

Codecov Report

Merging #1926 (6a6b94d) into master (09bdf0a) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

❗ Current head 6a6b94d differs from pull request most recent head ae09955. Consider uploading reports for the commit ae09955 to get more accurate results

@@             Coverage Diff              @@
##             master    #1926      +/-   ##
============================================
- Coverage     11.13%   11.13%   -0.01%     
  Complexity     1045     1045              
============================================
  Files           836      836              
  Lines         35824    35825       +1     
  Branches       4044     4044              
============================================
  Hits           3989     3989              
- Misses        31651    31652       +1     
  Partials        184      184              

Impacted Files	Coverage Δ
...core/src/main/java/com/sk89q/jnbt/CompoundTag.java	9.09% <ø> (ø)
...dit-core/src/main/java/com/sk89q/jnbt/ListTag.java	0.00% <ø> (ø)
...rc/main/java/com/sk89q/worldedit/LocalSession.java	0.00% <0.00%> (ø)
...va/com/sk89q/worldedit/command/tool/BlockTool.java	0.00% <ø> (ø)
.../worldedit/command/tool/DoubleActionBlockTool.java	0.00% <ø> (ø)
...c/main/java/com/sk89q/worldedit/entity/Player.java	0.00% <ø> (ø)
...a/com/sk89q/worldedit/util/asset/AssetLoaders.java	38.77% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 09bdf0a...ae09955. Read the comment docs.