SpongePowered / Sponge

The SpongeAPI implementation targeting vanilla Minecraft and 3rd party platforms.
MIT License
389 stars 211 forks source link

NBTTranslator cannot handle Enums #1762

Open NeumimTo opened 6 years ago

NeumimTo commented 6 years ago

I am currently running

I'm trying to implement AbstractSingleEnumData but every time i offer the datamanipulator the action ends with "Unable to translate object to NBTBase:".

at org.spongepowered.common.data.persistence.NbtTranslator.getBaseFromObject(NbtTranslator.java:164) ~[NbtTranslator.class:1.12.2-7.1.0-BETA-16]
at org.spongepowered.common.data.persistence.NbtTranslator.getBaseFromObject(NbtTranslator.java:150) ~[NbtTranslator.class:1.12.2-7.1.0-BETA-16]
at org.spongepowered.common.data.persistence.NbtTranslator.getBaseFromObject(NbtTranslator.java:150) ~[NbtTranslator.class:1.12.2-7.1.0-BETA-16]
at org.spongepowered.common.data.persistence.NbtTranslator.containerToCompound(NbtTranslator.java:91) ~[NbtTranslator.class:1.12.2-7.1.0-BETA-16]
at org.spongepowered.common.data.persistence.NbtTranslator.containerToCompound(NbtTranslator.java:72) ~[NbtTranslator.class:1.12.2-7.1.0-BETA-16]
at org.spongepowered.common.data.persistence.NbtTranslator.translateData(NbtTranslator.java:279) ~[NbtTranslator.class:1.12.2-7.1.0-BETA-16]

see https://github.com/SpongePowered/SpongeCommon/blob/31b25e39eb8b4b2dde382de7f147b5477e054485/src/main/java/org/spongepowered/common/data/persistence/NbtTranslator.java#L164

Could it be possible to store enum as a string?

my datamanipulator code: https://pastebin.com/y3GFsxzr

Zidane commented 6 years ago

It could be @NeumimTo . Wanna PR us a fix?

NeumimTo commented 6 years ago

tbh I have no idea how datamanipulators internally works. i would need someone who can i harrass with bunch of questions.

NeumimTo commented 6 years ago

eh.. ill give it a try

JBYoshi commented 6 years ago

@NeumimTo I think you can fix this by changing your toContainer() methods to add the name() of the value instead of the value itself, and then updating your from() method to use SocketType.valueOf(). Sponge uses something like this to store its CatalogType values as well (it actually uses CatalogTypes instead of enums, but you can use the same approach).

NeumimTo commented 6 years ago

@JBYoshi i was thinking about that as well, but then what is the purpurose of AbstractSingleEnumData when it cannot store/load enum type. i would expect sponge will abstract these calls name/valueof away. If i still have to store data internally as a string and sponge wont do automatic mapping i belive AbstractSingleEnumData should be removed from api. and we will just continue use AbstractSingleData

gabizou commented 4 years ago

Copy pasting from my comment on the PR, the issue is more aligned to the implementation of MemoryDataView not handling objects that are not naturally serializable (primitives and strings and list/map/array).