[1.12.1] ImmutableDataHolder not included with @Has event filter

[theboomer]

Asked to post this here by Zidane re: #spongedev chat.

The following logical event filter does not work: public void onPremove(ChangeBlockEvent.Pre event, @First @Has(PistonData.class) LocatableBlock lb) { with either @First or @Root as the error thrown is: Annotated type for data filter is not a DataHolder

and this seems to be because locatableblock is an immutable data holder, @Has only looks at mutable data holders ..

Full Error entry: https://pastebin.com/PJSz3Np6

[dualspiral]

Adding my comment from the channel:

It'll be because LocatableBlock isn't a DataHolder, it's an ImmutableDataHolder I don't think ImmutableDataHolder is of type DataHolder, and the filter only checks for DataHolder types: https://github.com/SpongePowered/SpongeCommon/blob/bleeding/src/main/java/org/spongepowered/common/event/filter/delegate/HasDataFilterDelegate.java#L56

This is indeed the case, @Has and @Supports only support DataHolders and ImmutableDataHolder does not inherit DataHolder - but the filters only use methods on the ValueContainer which is common to the DataHolder and ImmutableDataHolder - should we change the filters to check for the ValueContainer type, or is there some reason why they check for DataHolder?

[Deamon5550]

Yeah it should be changed to use ValueContainer

[dualspiral]

I was looking at the wrong get method, it doesn't have get(Manipulator.class) on the ValueContainer which the filter needs - this is because mutable and immutable data holders use manipulators that extend from different classes. So scratch that part of what I said.