Intern Property instances so reference equality is guaranteed

[magneticflux-]

Closes #19

[2No2Name]

I think the asymmetry of Object::equals is problematic. Is there any guarantee that WeakHashMap handles this the way you intend on all platforms? Could there be a platform where equals is called on the stored value leading to this breaking?
Would it be possible to instead use the Property as the key and the value being the key wrapped in a WeakReference? Then getting the value for any equal Property will return the original object, and you still keep the WeakReference behavior of the WeakHashMap.

[magneticflux-]

I don't know of any implementations that don't call equals on the new object, but this change doesn't seem to have any downsides and it's always good to be cautious, especially with undefined behavior!

[NebelNidas]

Why hasn't this been merged yet? Is it still not safe to use?

[malte0811]

This doesn't explain why it isn't merged yet (since I'm not the one who would need to merge it), but I'm not sure whether this is the right approach. Nothing is stopping modders from adding their own property implementations, which would not be interned by this PR and continue to cause issues.

Maybe I just don't know Yarn names, but I would have expected one of the mixins to target the property implementation for generic enums, rather than the Direction-specific one?

[magneticflux-]

@malte0811 That's true, if a mod implements Property<T> on its own, it won't be cached. That isn't a big issue though because that mod implementation typically won't be used by other mods.

You're right about the mixin, I need to make one that targets the EnumProperty explicitly. Static methods can't be overridden, so the DirectionProperty mixin is still needed.

[malte0811]

As discussed on Discord: Mods adding their own property implementation is still a concern, but a purely theoretical one for the time being. So IMO it is reasonable to merge this and deal with the issue if and when it actually turns up (never, likely as not).