Added some information about DataFormat #606

johnfg2610 · 2017-06-05T14:29:12Z

Since i recently learned about this and though it would be useful for others i decided to put it in the docs

ST-DDT · 2017-06-05T16:26:08Z

source/plugin/data/serialization.rst

+
+DataFormat
+==========
+A alternative to DataTranslators is using :javadoc:`DataFormat.readFrom` or :javadoc:`DataFormat.writeTo` which allows you to convert a data container this allows you to store a data container as a hocon, json or even NBT you can of course retrive data using data format.


You have to add your class to the javadoc-import block at the top of the page. Also I'm not sure whether you can/should reference methods like this.

ahh i see thank you and i only named the methods like that because i was guessing with the error it has been corrected :)

Deamon5550 · 2017-06-05T17:27:57Z

source/plugin/data/serialization.rst

+
+.. code-block:: java
+
+        public Optional<DataContainer> fromJsonFormat(InputStream inputStream){


This is an odd example considering its just wrapping the dataformat to squash the error and returns a null instead of an optional (!).

Inscrutable

Apart from the corrections, we also need to know which SpongeAPI versions this PR belongs to.

Inscrutable · 2017-06-06T10:17:23Z

source/plugin/data/serialization.rst

+
+DataFormat
+==========
+A alternative to DataTranslators is using :javadoc:`DataFormat` which allows you to convert a data container this allows you to store a data container as a hocon, json or even NBT you can of course retrive data using data format.


This sentence is a bit wonky, and far too long. How about this:
"An alternative to DataTranslators is to use :javadoc:DataFormat, which allows you to store a data container as a hocon, json or NBT file. You can also retrieve data using DataFormat."
It is also preferable to format class names etc like this, wherever they appear.

Inscrutable · 2017-06-06T10:26:04Z

source/plugin/data/serialization.rst

+==========
+A alternative to DataTranslators is using :javadoc:`DataFormat` which allows you to convert a data container this allows you to store a data container as a hocon, json or even NBT you can of course retrive data using data format.
+
+This is very useful if your for example using a database to store information as you can then serialize any data container to for example json format


your -> you are (or you're)
you can use e.g. instead of "for example"
it needs at least one comma, preferably after "information"

johnfg2610 · 2017-06-06T14:25:33Z

Json is new in sponge API 6 but hocon and NBT have been in for a while basically since the beginning but uses different methods prior to API 5
so this is for Sponge API 5-7

stephan-gh · 2017-06-12T10:57:31Z

source/plugin/data/serialization.rst

+
+DataFormat
+==========
+An alternative to DataTranslators is to use :javadoc:DataFormat, which allows you to store a ``DataContainer`` as a hocon, json or NBT file. You can also retrieve data using ``DataFormat``.


HOCON and JSON should be written in capital letters, just like NBT

stephan-gh · 2017-06-12T11:01:31Z

source/plugin/data/serialization.rst

+
+.. code-block:: java
+
+    public OutputStream toJsonFormat(Player player){


It might be better to use a specific example, like writing a ItemStack to a file and loading it back from it. You use an arbitrary ByteArrayOutputStream here which is returned as OutputStream so the return value of this method is essentially useless without additional casting. Additionally, how often do you actually serialize an entire Player on your own, and how would you load it back? The example below does only show how to read it back into a DataContainer.

i will use itemstack but i havnt written it to a file because that isnt really what i would expect people to do with it there more likly to use DataTranslators for that i was more thinking this would be used for databases etc also im not 100% sure how to go from datacontainer to the orignal object havnt been able to work that out yet

dualspiral · 2017-06-12T15:54:25Z

source/plugin/data/serialization.rst

+
+DataFormat
+==========
+An alternative to DataTranslators is to use :javadoc:DataFormat, which allows you to store a ``DataContainer`` as a HOCON, JSON or NBT file. You can also retrieve data using ``DataFormat``.


It doesn't necessarily create files. It transforms DataContainers into HOCON, JSON or NBT formats, which could be converted into a file, but in your example below, you don't create a file.

That last sentence should read something like:

You can also re-create DataContainers using these DataFormats.

dualspiral · 2017-06-12T15:54:27Z

source/plugin/data/serialization.rst

+==========
+An alternative to DataTranslators is to use :javadoc:DataFormat, which allows you to store a ``DataContainer`` as a HOCON, JSON or NBT file. You can also retrieve data using ``DataFormat``.
+
+This is very useful if you're for example using a database to store information, as you can then serialize any ``DataContainer`` to json format.


There are a few instances in your commit still where you need to update "json" to JSON. Also, this sentence doesn't seem like it fits well - it might be better to say something like:

For example, you can use DataFormat to create a JSON representation of a DataContainer, which can then be easily stored in a database as JSON. DataFormat can then be used to rebuild the DataContainer from this JSON when required.

It's possible it could be worded better than that, but this sentence doesn't read well as it is.

dualspiral · 2017-06-12T16:01:51Z

source/plugin/data/serialization.rst

+        return outputStream;
+    }
+
+**Code Example: Serializing a ItemStack from json format**


That's not what this example is - it's deserializing into a DataContainer from JSON.

I think what Deamon was getting at is this full example doesn't really tell you much, all you are doing is wrapping the result in an optional, returning an empty Optional if it fails. It doesn't tell you much - you might be better served showing how you then could use the resultant DataContainer, perhaps showing how to re-create the ItemStack from the container? That would fit nicely with your example above - it's showing that the returned DataContainer could be a valid ItemStack - which it would be in the context of the previous example.

ok ive learned how to do this now and will add it to the example shortly

… to a itemstack(if possible)

…irectly serialise a itemstack

dualspiral

Thank you @johnfg10 for listening to our suggestions and taking the time to do this and share your knowledge. I look forward to seeing this being merged.

gabizou

Deserialization isn't taking advantage of a few quality of life shortcuts that exist on the objects already being used.

gabizou · 2017-06-13T15:15:42Z

source/plugin/data/serialization.rst

-
-                }
+    public static Optional<ItemStackSnapshot> getItemStackSnapshotFromDataContainer(DataContainer dataContainer){
+        final Optional<DataBuilder<ItemStackSnapshot>> dataBuilderOptional =            Sponge.getDataManager().getBuilder(ItemStackSnapshot.class);


All of the data manager stuff is unnecessary, all you need to do is call dataContainer.getSerializable(ItemStackSnapshot.class, query) or however the parameters are ordered.

i tried doing that in my plugin and it didnt work correctly for me so i went with this method(as i know it works) although i will retest and make sure

well i suppose i must of used it incorrectly the first time hmm il update the example shortly

gabizou · 2017-06-13T15:16:16Z

source/plugin/data/serialization.rst

+        final Optional<DataBuilder<ItemStackSnapshot>> dataBuilderOptional =            Sponge.getDataManager().getBuilder(ItemStackSnapshot.class);
+        if (dataBuilderOptional.isPresent()){
+            DataBuilder<ItemStackSnapshot> itemStackSnapshotDataBuilder = dataBuilderOptional.get();
+            Optional<DataView> dataViewOptional = dataContainer.getView(DataQuery.of("item"));


Where are you getting the item query from?

fixed now sry forgot to remove it i pulled that example out of one of my plugins

gabizou · 2017-06-14T12:10:17Z

source/plugin/data/serialization.rst

+        return outputStream;
+    }
+
+**Code Example: Deserializing a ItemStackSnapshot from json format**


So I know it's been discussed before that you want to show how to use the DataContainer resulting from being converted from JSON, but I feel that it should still be shown how to convert it from JSON to a DataContainer, @dualspiral, I'm open for suggestions. I don't want to make it back and forth, but the wording is a little awkward since you state here that it's deserializing from JSON format, but there's no mention of JSON.

So, the original thought of mine is that while it doesn't matter really what we deserialise to, having an example in context is often beneficial - an ItemStackSnapshot is a classic example of what you might want to serialise/deserialise. Right now, this code example is not useful, because it now skips the step of exactly what we want to show.

Effectively, the goal is to say "hey, if you don't want to serialise your DataContainer into configuration, then DataFormat is for you". I hate to say this, but many developers learn by example, and may be stuck on the point that ByteArrayOutputStreams and input streams are here, I suspect some don't know what they do and is going to detract from the actual goal. I also expect most developers will use this for databases, and just want to get a string out to save into the database.

On the flip side, saying "here is a data container, here is JSON, here is the container again" is not going to wash either - why would a developer care about a simple data container?

So, how about, rather than providing two examples, we consider providing a sample class here that developers can use that is essentially a template on how we might want to store and retrieve a data container backed object? Something like JsonItemStackSnapshotConverter (or whatever you like) that contains two commented methods, toJson and fromJson, that illustrates how to get a String from an ItemStackSnapshot, and vice versa. That way, we get the context of when we might use this (item serialisation), and how this would be performed?

is https://pastebin.com/VxtsLG5u ok @dualspiral ?

Inscrutable · 2017-09-03T03:10:27Z

We still have 2 pending reviews here. Can we get this cleaned up and merged soon?

stephan-gh · 2017-09-03T09:56:10Z

source/plugin/data/serialization.rst

+
+.. code-block:: java
+
+    public ByteArrayOutputStream toJsonFormat(ItemStackSnapshot itemStack){


Can you add a space before {

I also think it would be easier to understand if you don't return the ByteArrayOutputStream, but rather use a specific example like writing a ItemStackSnapshot to a file. I don't think anyone would use the method like this.

done in the new pr

stephan-gh · 2017-09-03T09:56:40Z

source/plugin/data/serialization.rst

+
+    public ByteArrayOutputStream toJsonFormat(ItemStackSnapshot itemStack){
+        DataContainer playerDataContainer = itemStack.toContainer();
+        DataFormat dataFormat = DataFormats.JSON;


I don't think it makes any sense to store this in a variable first, just access it directly using DataFormats.JSON.writeTo below.

done in the new pr

johnfg2610 · 2017-09-03T15:39:05Z

ive had to open a new pull request as i apparently deleted the fork as some point

Johnfg10 added 2 commits June 5, 2017 15:28

Added some information about DataFormat

f445d7b

fixed javadoc error

ee3187c

ST-DDT requested changes Jun 5, 2017

View reviewed changes

Johnfg10 added 3 commits June 5, 2017 17:59

added the javadoc imports(and cleaned up a little

bd276db

Fixed imports blocks to comply better with sponges standards

8e24d51

fixed spacing

c1c2a35

Deamon5550 reviewed Jun 5, 2017

View reviewed changes

cleaned up example

266e50e

Inscrutable requested changes Jun 6, 2017

View reviewed changes

fixed text formatting as suggested also example update

682ab87

Inscrutable approved these changes Jun 7, 2017

View reviewed changes

Inscrutable added this to the v5.0.0 milestone Jun 7, 2017

ST-DDT approved these changes Jun 7, 2017

View reviewed changes

stephan-gh suggested changes Jun 12, 2017

View reviewed changes

moved exampls to ItemStacks and fixed text formating

2ba4819

dualspiral reviewed Jun 12, 2017

View reviewed changes

Johnfg10 added 5 commits June 12, 2017 19:19

fixed JSON formatting

c51b48c

improved deserializing example it now deserializing the datacontainer…

2d4fc37

… to a itemstack(if possible)

changed the examples to ItemStackSnapshot as its unlikely you would d…

230d3fa

…irectly serialise a itemstack

removed static and updated imports also fixed typos

fa8305f

text improvements

918b262

dualspiral approved these changes Jun 13, 2017

View reviewed changes

gabizou requested changes Jun 13, 2017

View reviewed changes

Johnfg10 added 2 commits June 13, 2017 19:14

fixed some left over code from my project

bf3dd18

changed the example over to the build in method

4c4233d

gabizou reviewed Jun 14, 2017

View reviewed changes

stephan-gh suggested changes Sep 3, 2017

View reviewed changes

johnfg2610 mentioned this pull request Sep 3, 2017

Added some information about DataFormat #623

Closed

johnfg2610 closed this Sep 3, 2017


		.. code-block:: java

		public Optional<DataContainer> fromJsonFormat(InputStream inputStream){


		.. code-block:: java

		public OutputStream toJsonFormat(Player player){


		.. code-block:: java

		public ByteArrayOutputStream toJsonFormat(ItemStackSnapshot itemStack){

Added some information about DataFormat #606

Added some information about DataFormat #606

Conversation

johnfg2610 commented Jun 5, 2017

ST-DDT Jun 5, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Inscrutable left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnfg2610 commented Jun 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dualspiral left a comment

Choose a reason for hiding this comment

gabizou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnfg2610 Jun 13, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Inscrutable commented Sep 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnfg2610 commented Sep 3, 2017

ST-DDT Jun 5, 2017 •

edited

johnfg2610 Jun 13, 2017 •

edited