[1.12.2] NBT List with one Key?

[Draco18s]

/me scans the code you posted.

 

Line 114-116?

Oh yeah, that might do it.

Why would you ever return null from a serialize function?

[HalestormXV]

21 minutes ago, Draco18s said:

/me scans the code you posted.

 

Line 114-116?

Oh yeah, that might do it.

Why would you ever return null from a serialize function?

Becasue I thought that over-riding in the functions part of the code would resolve it since in this particular capability i used

public static class LearnedSpellsProvider implements ICapabilitySerializable<NBTTagCompound>

instead of

public static class LearnedSpellsProvider implements ICapabilitySerializable<NBTBase>

 

[Draco18s]

29 minutes ago, HalestormXV said:

Becasue I thought that over-riding in the functions part of the code would resolve it

But you returned null. Returning null is never¹ the right answer.

 

^1. There are cases where it's fine, but only when you know what it is that null means in that context. "This is how I save to disk" should never ever--not in a billion lifetimes of the universe--ever be true.

[HalestormXV]

7 hours ago, Draco18s said:

But you returned null. Returning null is never¹ the right answer.

 

^1. There are cases where it's fine, but only when you know what it is that null means in that context. "This is how I save to disk" should never ever--not in a billion lifetimes of the universe--ever be true.

I seemed to have fixed it with this updated code: https://hastebin.com/hufinerope.java

 

I tried testing, but becasue every re-run of the environment generates a new UUID i can't tell if it is actually saving the old values or not or just replacing whats there with new values.

[Draco18s]

2 hours ago, diesieben07 said:

Instead of your convert2Primative you can use Ints.toArray from Guava.

I find this particularly hilarious as the post where I said that there already existed a method to do the conversion (which was the same post I told him to use a list) he replied with "I didn't know that, so I made one." Though I was referring to the toArray method in the List<T> class. I didn't know about the Guava version.

[HalestormXV]

I didn't relaize a guava version existed. Thank you, it is far more conveneient. And draco I thought you were talking about something else. My mistake.

int[] convertedList = Ints.toArray(knownSpells);
nbt.setIntArray("LearnedSpells", convertedList);

Much better and cleaner.

 

 

On 3/4/2018 at 11:05 AM, diesieben07 said:

• LearnedSpellsHandler should not implement anything. And this is a terrible class name. This class is not even needed, you should put the static field for @CapabilityInject into LearnedSpellsMain.

• ILearnedSpells (your capability) should not extend INBTSerializable. It should also not have the sync method, this is an implementation detail that should only go into the actual implementation (LearnedSpellsFunctions, another terrible class name).

• LearnedSpellsStorage::readNBT checks if nbt is an NBTTagIntArray and if so casts it to NBTTagCompound. This will always crash or never do anything. In your case it will never do anything, since you always write an NBTTagCompound, so the if statement will never be true.

• Instead of your convert2Primative you can use Ints.toArray from Guava.

So what i crossed out is a simple fix for me, no problem. But if the capability should not extend anything then are you saying that the storage class should be the one doing everything that is within the functions when it comes to the read and write of the NBT. So should i scrap what is in the LearnedSpellsFunctions (writeNBT, serializeNBT, and deserializeNBT) and move it into the storage or something else entirely. Sure this may seem like relatively simply stuff but unfortunatly documentation on the capabilities is not as abundant as some of the other things. And most of the research I did find and examples I did follow did so this way (with much simpler capabilities). Now be that the wrong way is clearly not for me to decide but I am just working off what I have found and read. The ProjectE Primer on Capabilities is probably the only one that has advanced Capability manipulation in it but even that doesn't fit what I am trying to do. 

 

So once again, yes this all may seem simple and chalk full of stupid errors lol but that is how I am learning this. And generally when posting on these forums in particula me personally follow through with all the suggestions and learn from it as you may or may not already know. So unfortuantly that is happening with this.

 

I even tried doing a (literally just to try an alternative)

    return new NBTTagIntArray(getListFunction);
}

and having the getListFunction do basically everything that was being done before and got stopped by the fact that NBTTagIntArray is private. 

Edited March 5, 2018 by HalestormXV

[Draco18s]

On ILearnedSpells (your capability) should not extend INBTSerializable :

https://github.com/Draco18s/ReasonableRealism/blob/master/src/main/java/com/draco18s/hardlib/api/capability/RawMechanicalPowerHandler.java#L8

 

On LearnedSpellsStorage::readNBT checks if nbt is an NBTTagIntArray and if so casts it to NBTTagCompound :

You should read the NBT by reversing whatever writeNBT does: read out an NBTTagIntArray.

[HalestormXV]

52 minutes ago, Draco18s said:

On ILearnedSpells (your capability) should not extend INBTSerializable :

https://github.com/Draco18s/ReasonableRealism/blob/master/src/main/java/com/draco18s/hardlib/api/capability/RawMechanicalPowerHandler.java#L8

 

On LearnedSpellsStorage::readNBT checks if nbt is an NBTTagIntArray and if so casts it to NBTTagCompound :

You should read the NBT by reversing whatever writeNBT does: read out an NBTTagIntArray.

Okay, thank you for the example. So in reading it over I see that in your handler, you don't even have a write or a read, you kept those within the storage? So now in my case I should for sure have some functions that would return that particular instance's list this way it can be called from the storage.

 

So ideally in my functions I should have something simply like:

@Override
public NBTTagCompound serializeNBT()
{
    NBTTagCompound nbt = new NBTTagCompound();
    int[] convertedList = Ints.toArray(knownSpells);
    nbt.setIntArray("LearnedSpells", convertedList);
    return nbt;
}

@Override
public void deserializeNBT(NBTTagCompound nbt)
{
    int[] convertedList = Ints.toArray(knownSpells);
    convertedList = nbt.getIntArray("LearnedSpells");
}

 

And in my storage i should have something along the lines of

public NBTBase writeNBT(Capability<ILearnedSpells> capability, ILearnedSpells instance, EnumFacing side)
{
    NBTTagCompound nbt = new NBTTagCompound();
    nbt.setIntArray("LearnedSpells", instance.getSpellList);
    return nbbt;
}

@Override
public void readNBT(Capability<ILearnedSpells> capability, ILearnedSpells instance, EnumFacing side, NBTBase base)
{
    NBTTagCompound nbt = (NBTTagCompound) base;
    instance.setSpellList(nbt.getIntArray("LearnedSpells"));
}

 

I'm guessing?

Edited March 5, 2018 by HalestormXV

[Draco18s]

24 minutes ago, HalestormXV said:

    int[] convertedList = Ints.toArray(knownSpells);
    convertedList = nbt.getIntArray("LearnedSpells");

Why are you performing the conversion on line 1? What is the point of this? Whatever it does, the result is immediately overwritten by line 2.

return nbbt;

You have a typo.

 

diesieben07 may have more input as well. There's something I'm scratching my head over, but can't articulate what I think you're doing wrong.

[HalestormXV]

    int[] convertedList = Ints.toArray(knownSpells);
    convertedList = nbt.getIntArray("LearnedSpells");

Actually a really simple answer for that one, I just didn't remove it yet. At this point it is just a placeholder until I write the getSpellList and setSpellList functions. Once I do that i will be able to get rid of the known spells entirely. 

 

EDIT:

Well by making all the changes I am back to where I started. The int list stays and updates. So if I add spells 13, 2, 6 into the NBT tag they all stay there and if you open the NBT you can see that the capability is there and the data gets stored in the correct order. Then once you log out that data stays also. 

 

Once you log back in though and add a new ID to the list the old data gets erased and is replaced with an empty tag all over again just containing that ID. Perhaps this is because I am creating a new tag?

https://hastebin.com/izajuwugum.java

 

Once I get this functionality working properly i can work on proper naming and what not. Right now the issue seems to be the data is being overwritten each time I log back into the character. Which to me says soemthing is maybe(?) wrong with my read, write, serilize, and deserilize.

 

Or maybe it is because instead of adding to an existing list I am simply "setting" a new list each time instead of adding to an existing if one does in fact exists? I can't seem to find something that would add an entry to the list instead of setting it each time.

Edited March 5, 2018 by HalestormXV
Added updated code.

[HalestormXV]

4 hours ago, diesieben07 said:

• Your LearnedSpellsFunctions still implements INBTSerializable. Why?
• Why is convertedList a field now? This will be immediately out of date. Do not duplicate data like that.

Point one, because I forgot to backspace it out, quite literally I just forgot. I know it does nothing with this new code. 

Point two, so rather than have it be a field it should simply just be a local variable? Or don't use it at all and simply just use the Ints.toArray

[Draco18s]

You don't need it at all. You never did. I don't know why my last comment made you think "oh, this should be a property" and not "oh, I need to not do this thing he told me not to do."

On 3/5/2018 at 10:20 AM, Draco18s said:

What is the point of this? Whatever it does, the result is immediately overwritten by line 2.

 

[HalestormXV]

1 hour ago, Draco18s said:

You don't need it at all. You never did. I don't know why my last comment made you think "oh, this should be a property" and not "oh, I need to not do this thing he told me not to do."

 

 

Okay but a field is still needed (just not initialized) that is going to be holding the data right. Surely you can't do it all via local. 

[Draco18s]

    nbt.setIntArray("LearnedSpells", Ints.toArray(knownSpells));  // :O SUCH MAGIC

 

[HalestormXV]

21 minutes ago, diesieben07 said:

Yes, but you should have one field. Not two fields holding the same data.

Alright.

 

17 minutes ago, Draco18s said:

    nbt.setIntArray("LearnedSpells", Ints.toArray(knownSpells));  // :O SUCH MAGIC

 

Yes I figured that part out already. But no matter which way I look at it or change it the results are still the same:

https://hastebin.com/evupalutuv.java

Data is stored and saved and once you log back in the tag data is erased but and an empty tag remains. Which leads me to believe that my deserializeNBT

is the next place to work on? All it is simply doing as the code stands now is simply getting the array and doing nothing with it correct? So logic would say that the next step for the deserializeNBT is to take the array it retrieved and place it back into the List<Integer> knownSpells correct?

Edited March 6, 2018 by HalestormXV

[Draco18s]

        @Override
        public void deserializeNBT(NBTTagCompound nbt)
        {
            nbt.getIntArray("LearnedSpells");
        }

 

Oooh, a value! *throws it in the garbage.*

[HalestormXV]

30 minutes ago, diesieben07 said:

 

Okay, so in other words, get rid of deserialize and serialized completely from the Functions class of this code. Leave it in the Provider. I only have to worry about the read and write functions? Which would coincide with what 

30 minutes ago, Draco18s said:

        @Override
        public void deserializeNBT(NBTTagCompound nbt)
        {
            nbt.getIntArray("LearnedSpells");
        }

 

Oooh, a value! *throws it in the garbage.*

because I am not letting the Provider do its job since I am overwriting it within my functions. Correct? Like I said, I know this is probably simple and perhaps I am over-complicating this because I am trying to understand it so in the future I don't run into these issues. So I apologize for the ignorance but I do appreciate it.

Edited March 6, 2018 by HalestormXV

[Draco18s]

14 minutes ago, HalestormXV said:

because I am not letting the Provider do its job since I am overwriting it within my functions. Correct?

No. Christ.

getIntArray returns a value that you then proceed to do FUCK ALL with. You want to read a value out of the NBT? STORE IT SOMEWHERE. It does not magically get saved into your LearnedSpells property.

Edited March 6, 2018 by Draco18s

[HalestormXV]

1 hour ago, Draco18s said:

No. Christ.

getIntArray returns a value that you then proceed to do FUCK ALL with. You want to read a value out of the NBT? STORE IT SOMEWHERE. It does not magically get saved into your LearnedSpells property.

Got it.

1 hour ago, diesieben07 said:

Yes. The saving should usually be handled by your IStorage, or otherwise through other means (not INBTSerializable). INBTSerializable is for your provider only.

Thanks.

 

I've managed to get it working it would seem. Here is the code. I am not sure if this is where you were both trying to push me (i swear the two of you are like Yin&Yang which is quite awesome btw) but it is functioning as it should. Though is it correct?

https://hastebin.com/ebutefejom.java

[HalestormXV]

44 minutes ago, diesieben07 said:

The heck? What is this supposed to achieve? You are setting the value of spellList, which is then immediately dumped because you do not use it anymore.

 

Why are you calling getIntArray here? You just created the NBTTagCompound, this will always be empty. And you are ignoring the result anyways...

 

Why are you logging this at "info" level? This is not logging levels work. Also, don't call getLogger all the time, it's slow.

And why are you not using the setSpellList method you made (it doesn't work, that's probably why, but you should fix it instead).

 

Another thing, your readNBT method calls learnedSpell on an instance of ILearnedSpells. Either this does not compile or learnedSpell in LearnedSpellsFunctions is missing @Override.

1) The setSpellList is useless at this point. I originally thought that I had to add everything all at once and that's why I originally made it. Right now it is completely useless since the learnedSpell can just be looped. (although a player will only get one spell at a time anyway) Unless I figure something out in the future. 

 

2) Because i have been staring at this for way to long and got incredibly frustrated with it and should have just deleted it since it is not necessary.

 

3) The logging is just there for me to make sure it is working. I could use a system.out or whatever but I was just checking to see if it works and it is easier for me to see that in my console when the message appears.

 

4) InteliJ already is overriding it.

Edited March 7, 2018 by HalestormXV

[HalestormXV]

6 hours ago, diesieben07 said:

 

This statement makes zero sense.

InteliJ is where I do the code and the @Override annotation is already there, it just wasn't pasted. Probably because I erased some notes before pasting to hastebin and the annotation might have been accidently deleted in the paste.

 

Since I am still on this topic, and rather than making a whole new thread. Can this be Synced with ByteBufUtils.writeTag(buf, nbt) and ByteBufUtils.readTag(buf) in packets or does my Packet need to take apart the int[] and place them into bytes and reassemble the bytes into the int[]? as I want the player to be able to eventually open a GUI and see a list of spells or at least key-press and see the list in chat. 

[HalestormXV]

1 hour ago, diesieben07 said:

Then this has nothing to do with IntelliJ. You simply failed to copy-paste (which is a pretty big accomplishment in it's own right).

 

You can in theory just send over the NBT tag. However it is more efficient to send over the array of ints.

Yes, I have failed a copy-paste and thus is the result of staring at a piece of code too long and wanting to be done with it.

 

1 hour ago, diesieben07 said:

You can in theory just send over the NBT tag. However it is more efficient to send over the array of ints.

Actually I take that back, the server in SMP is the one who knows about the capability data not the client so the client needs to fetch it from the server not vice versa. Right? But if I want to add something to the known spells with an item right click or something like that, then it would have to be sent from client to server?

 

Edited March 7, 2018 by HalestormXV

[HalestormXV]

7 minutes ago, diesieben07 said:

The server would sent it to the client when needed, the client does not need to request it.

As for right clicks, those are already sent to the server by the game engine, you do not need to worry about that.

Well that about solves it then I guess. The only way a player in my mod will ever learn a spell is by a right click so that will automatically be handled. Well it has been a long and gruling "class" but I think it can be marked as solved. As bothersom as I have been to both you and @Draco18s and as frustrating it is dealing with someone who has never utilized what was needed in this particular instance I do appreciate the efforts and the "drill sergeant" methods that were employed as I won't soon forget what was learned in creating this.

[jabelar]

On 3/4/2018 at 7:08 AM, HalestormXV said:

I tried testing, but becasue every re-run of the environment generates a new UUID i can't tell if it is actually saving the old values or not or just replacing whats there with new values.

 

Are you talking about the player UUID? You should be able to run with same player in development environment by adding the --username and --password arguments to your run configuration. I'm not sure in IntelliJ where that is done, but in Eclipse you edit the run configuration arguments. 

 

This thread made me chuckle. NBT is theoretically really simple, but I admit it can hurt your head, mostly because you can combine things in multiple ways -- lists in compounds, compounds in lists, lists of tags, lists of primitives and so forth. Also, the order of construction/deconstruction has to be matched and any typo will kill you. I highly recommend when working with NBT that you print the contents out to console/logger at each step to get a sense of what is really happening. Seems like you got NBT working now?

 

EDIT: I wish Forge would have provided methods for syncing capabilities to client. While not that hard to implement it is such a common modding need. Someday maybe I'll suggest a pull request for that, although maybe there was a reason they chose not to in the first place.

Edited March 7, 2018 by jabelar

[HalestormXV]

2 hours ago, jabelar said:

 

Are you talking about the player UUID? You should be able to run with same player in development environment by adding the --username and --password arguments to your run configuration. I'm not sure in IntelliJ where that is done, but in Eclipse you edit the run configuration arguments. 

 

This thread made me chuckle. NBT is theoretically really simple, but I admit it can hurt your head, mostly because you can combine things in multiple ways -- lists in compounds, compounds in lists, lists of tags, lists of primitives and so forth. Also, the order of construction/deconstruction has to be matched and any typo will kill you. I highly recommend when working with NBT that you print the contents out to console/logger at each step to get a sense of what is really happening. Seems like you got NBT working now?

 

EDIT: I wish Forge would have provided methods for syncing capabilities to client. While not that hard to implement it is such a common modding need. Someday maybe I'll suggest a pull request for that, although maybe there was a reason they chose not to in the first place.

Oh I didn't know you can put in the user name and password like that. Yeah InteliJ has the option to edit the run config. I may give that a try. Yeah NBT in thought is really simply, like this was one of the few things I knew EXACTLY what I wanted to do but just couldn't get it to work the way I wanted. 

 

Yes the NBT is working now thankfully. There is no issue, I have yet to test in an SMP environment but it shouldn't make much of a difference. I even managed to set up a packet that will allow the client to fetch the results on a keypress and display a list of what they know/have learned. However, the only thing which I may need to do next is figure out how to set up a Sync packet. Mainly, just to lay out the foundation if I plan to make a GUI or something like that in the future to display it in a more "fansy" way. I know there is the ByteBufUtils.writeTag(buf, nbt) and ByteBufUtils.readTag(buf) but to be quite honest I don't think I want to look at this specific SpellLearning code for a little while lol. However if you have any tips or suggestions or samples on how I would lay the packet out I am all ears. As i said it is not something I need right now to make this function but it would provide a solid base should I decide to do something like a GUI or something in the future.