Skip to content

Add helper methods to CustomValue and its implementations#338

Open
BoogieMonster1O1 wants to merge 8 commits intoFabricMC:masterfrom
BoogieMonster1O1:mapfix
Open

Add helper methods to CustomValue and its implementations#338
BoogieMonster1O1 wants to merge 8 commits intoFabricMC:masterfrom
BoogieMonster1O1:mapfix

Conversation

@BoogieMonster1O1
Copy link
Copy Markdown

@BoogieMonster1O1 BoogieMonster1O1 commented Nov 25, 2020

CustomValue's implementations do not implement equals or hashCode. This causes issues when storing them in lists and maps, where it is impossible to remove CustomValues from a list, or get the value associated with a CustomValue key from a map.
This also affects any DynamicOps relying on CustomValues as FieldDecoders get returned null when reading a CustomValue keyed map.

This makes CustomValue's implementations implement equals, hashCode and toString.
This also adds a stream method to CvObject and CvArray, and a keySet method to CvObject for convenience.

Comment thread src/main/java/net/fabricmc/loader/api/metadata/CustomValue.java Outdated
Comment thread src/main/java/net/fabricmc/loader/api/metadata/CustomValue.java Outdated
Comment thread src/main/java/net/fabricmc/loader/api/metadata/CustomValue.java Outdated
Comment thread src/main/java/net/fabricmc/loader/api/metadata/CustomValue.java Outdated
Comment thread src/main/java/net/fabricmc/loader/metadata/CustomValueImpl.java Outdated
Comment thread src/main/java/net/fabricmc/loader/api/metadata/CustomValue.java Outdated
Comment thread src/main/java/net/fabricmc/loader/metadata/CustomValueImpl.java Outdated
@liach liach added the bug label Jun 24, 2021
@liach
Copy link
Copy Markdown
Contributor

liach commented Jun 24, 2021

The custom value's status as not being value-based should be a bug. @sfPlayer1 What's your view on this whole thing? Will you include the new utilities besides fixing the value-based nature of these classes?

@liach
Copy link
Copy Markdown
Contributor

liach commented Jun 30, 2021

@sfPlayer1 Even though the addition of stream methods are actually optional (as they can always stream an iterable), I wonder if we want to treat our custom values as data classes (as opposed to identity objects as they are right now)

@Player3324
Copy link
Copy Markdown
Contributor

Not sure, it's really just a simple way to expose the data from the mod jsons. Implementing toString has some benefits in the debugger, but uses that depend on equals/hashCode having specific semantics are unclear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants