Don't register a TypeSerializer for arbitrary Number subclasses #122
Conversation
We have no way way to construct arbitrary subclasses of Number. By registering a predicate that accepts only kown subclasses, we ensure that users attempting to serialize an unknown Number subclass get a proper exception, instead of a silent deserialization failure.
|
I'd like to get this merged as soon as possible, as it fixes a fairly severe issue. |
|
I'm fine with the change, but we need to make a decision about whether "backwards incompatible" bug fixes are being merged or not before 4.0. (see discussion in #120) |
|
From my perspective, any plugin currently relying on |
| || Short.class.equals(clazz) | ||
| || Byte.class.equals(clazz) | ||
| || Float.class.equals(clazz) | ||
| || Double.class.equals(clazz); |
There was a problem hiding this comment.
I would have expected more types then this being supported by default. BigDecimal?
There was a problem hiding this comment.
My intent with this PR is just to fix the behavior of the existing serialized. We could probably add support for BigDecimal in another PR
|
@Zidane Thoughts on whether or not this should constitute a breaking change? |
|
I am of the opinion it isn't breaking because it is inherently broken and it is our responsibility to mitigate that over a "technical break" with the exception being thrown. |
|
@Aaron1011 @Zidane would #95 be considered in the same boat then? It prevents serialization of some DataContainers / NBT. |
We have no way way to construct arbitrary subclasses of Number. By
registering a predicate that accepts only kown subclasses, we ensure
that users attempting to serialize an unknown Number subclass get a
proper exception, instead of a silent deserialization failure.
Fixes #121