Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversion between NBTs and Strings losslessly #1854

Open
ustc-zzzz opened this issue Jun 27, 2018 · 12 comments
Open

Conversion between NBTs and Strings losslessly #1854

ustc-zzzz opened this issue Jun 27, 2018 · 12 comments

Comments

@ustc-zzzz
Copy link
Contributor

Is there any way to convert between NBTTagCompounds (maybe DataContainers in SpongeAPI) and Strings losslessly? For example, in forge mods we can use net.minecraft.nbt.JsonToNBT#getTagFromJson and net.minecraft.nbt.NBTTagCompound#toString to complete the conversion between those two types, but in SpongeAPI I could not find methods which look like they could do these things.

I have considered converting an NBT to JSON, HOCON, or something else, but both JSON and HOCON only have six types (String, Number, Boolean, Null, Array, and Object) while the number of NBT types is much greater (for example, there are six number types for NBT: Byte, Short, Int, Long, Float, and Double, so I cannot recognize what the type is when I try to deserialize a number in a JSON). What I need is lossless conversion by using SpongeAPI.

@limbo-app limbo-app added the status: needs triage This label is automatically applied to new issues and pull requests to indicate they require triage label Jun 27, 2018
@Cybermaxke
Copy link
Contributor

Cybermaxke commented Jun 27, 2018

Converting Containers is done by using DataFormats:
https://github.com/SpongePowered/SpongeAPI/blob/bleeding/src/main/java/org/spongepowered/api/data/persistence/DataFormats.java#L32
No idea if all data types are persisted in the JSON and HOCON formats.

@ustc-zzzz
Copy link
Contributor Author

ustc-zzzz commented Jun 27, 2018

@Cybermaxke well, org.spongepowered.api.data.persistence.DataFormats#NBT seems OK...It's a binary format, maybe I should escape some weird characters or just apply base64 on it...so is there other solutions for human-readable strings, or can we just add another field for human-readable strings (it can be named NBT_UTF_8_STRING or something else) to SpongeAPI and implement it?

@ryantheleach
Copy link
Contributor

ryantheleach commented Jun 28, 2018

We should consider having a 'mojangson' option for outputting and parsing, DataContainers to NBT, to string left up to the implementation to implement.

This would be the most portable and well recognized option in the MC community.

Or are there still ambiguous serialization cases with it?

@Cybermaxke Lantern et all, could either recreate it, or use their own format when using this serializer?

@Cybermaxke
Copy link
Contributor

Cybermaxke commented Jun 28, 2018

@ryantheleach I don't really have a true mojangson parser in lantern atm, but I will try to implement it and then port it over to sponge.

@ryantheleach
Copy link
Contributor

@Cybermaxke my suspicion, at least for SpongeCommon is that it would be easier to just go to NBT then let Minecraft turn it into Mojangson.

My hesitations about adding it to the API were:

  1. if it would explicitly state it to be Mojangson.
  2. our (lack of) control over Mojangson.
  3. whether implementations would be allowed to differ.

We could say it's just a format that has the same capabilities, retaining types for the primitives that are supported by NBT.

E.g. A NBTText format, that can differ by implementation.

Then, as long as you can always round-trip it to text, then you satisfy the requirement?

@Cybermaxke
Copy link
Contributor

Plugin devs may not expect different output types when using a specific data format. This is something that should be consistent across implementations, if you want a different format, provide a new one. Another option is to provide the same parser across implementations and make it compatible with mojangson, but we can add our own custom parsing capabilities (if desired).

I am also directly parsing the mojangson data into a DataView to reduce overhead of converting the data twice.

@ryantheleach
Copy link
Contributor

ryantheleach commented Jun 28, 2018

I'd shy away from adding customized parsing capabilities, I'm keen to see something that's portable from vanilla communities.

I'm not sure of the maturity of the projects, but they may be a starting point as they are MIT Licensed.

https://github.com/momothereal/Mojangson
https://github.com/Eisenwave/eisen-nbt

Edit: Just realized I might be coming off a bit unrespectful of the existing code in LanternNBT. That wasn't my intention.

@Cybermaxke
Copy link
Contributor

Cybermaxke commented Jun 28, 2018

LanternNBT doesn't have any Mojangson parsing capabilities, at least not atm. If I support it for LanternNBT, I am going to add custom parsing capabilities to support all its data types, while retaining vanilla compatibility if the data gets serialized.

@Cybermaxke
Copy link
Contributor

Cybermaxke commented Jun 28, 2018

@ryantheleach The parser I was working is mostly done, maybe more error handling. And I need to do more testing, you can checkout my progress here:
LanternPowered/Lantern#59

@ryantheleach
Copy link
Contributor

Other options are SNBT (Mojang) or NBTT (MinecraftDev IntelliJ plugin)

@ryantheleach ryantheleach added system: command type: enhancement and removed status: needs triage This label is automatically applied to new issues and pull requests to indicate they require triage labels Aug 3, 2018
@ryantheleach
Copy link
Contributor

For lack of a better place to organize this, I'm putting it under Commands due to Vanilla commands and Brigadier having support for parsing SNBT in 1.13

@dualspiral
Copy link
Contributor

This is very much a data question, not a command one. I'm not sure if this is possible in 1.16+ builds yet though...

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

No branches or pull requests

5 participants