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

Completely replace fastnbt with our own NBT crate #420

Open
1 task done
Snowiiii opened this issue Dec 26, 2024 · 13 comments · May be fixed by #486
Open
1 task done

Completely replace fastnbt with our own NBT crate #420

Snowiiii opened this issue Dec 26, 2024 · 13 comments · May be fixed by #486
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@Snowiiii
Copy link
Member

I've searched existing issues and couldn't find a duplicate.

  • I confirm this is not a duplicate.

Is your feature request related to a problem? Please describe.

Our own NBT crate currently does work great when serializing values into NBT, But deserialzing seems not to really work

Describe the solution you'd like

Fix our NBT crate and finally remove fastnbt. This should also finally allow #283 to be merged

@Snowiiii Snowiiii added the enhancement New feature or request label Dec 26, 2024
@Snowiiii Snowiiii self-assigned this Dec 26, 2024
@Snowiiii Snowiiii added the good first issue Good for newcomers label Dec 26, 2024
@Snowiiii Snowiiii added this to Pumpkin Dec 26, 2024
@Snowiiii Snowiiii moved this to Todo Features in Pumpkin Dec 26, 2024
@Snowiiii Snowiiii changed the title Completly replace fastnbt with our own NBT crate Completely replace fastnbt with our own NBT crate Dec 26, 2024
@Snowiiii Snowiiii pinned this issue Dec 26, 2024
@asodugf12
Copy link

Now that I am out of the translation issue, can I take up this one?

@Snowiiii
Copy link
Member Author

Now that I am out of the translation issue, can I take up this one?

Yes, Please would be very nice

@asodugf12 asodugf12 mentioned this issue Jan 2, 2025
@Snowiiii
Copy link
Member Author

Snowiiii commented Jan 2, 2025

@asodugf12 How its going ?

@asodugf12
Copy link

oop, Ive been a bit busy the past few days so I'll start work now. Can I get a briefing on the problems with the current system?

@Snowiiii
Copy link
Member Author

Snowiiii commented Jan 3, 2025

oop, Ive been a bit busy the past few days so I'll start work now. Can I get a briefing on the problems with the current system?

So Serializing (data->NBT) works fine, no problems. But Deserializing (NBT->data) which is done in pumpkin-world does not work for some reason

@asodugf12
Copy link

Is there any specific function that doesnt work, or what is the function output against the desired output. I forgot my charger, so there id a limited time window I could work on things and i just wanna make sure i could be most productive

@Snowiiii Snowiiii unpinned this issue Jan 5, 2025
@Snowiiii
Copy link
Member Author

Snowiiii commented Jan 5, 2025

Is there any specific function that doesnt work, or what is the function output against the desired output. I forgot my charger, so there id a limited time window I could work on things and i just wanna make sure i could be most productive

Hey, I'm not sure, its just not working, I first tough it had something to do with Arrays but after testing not only Arrays are not working, the whole Deserialization is broken. You don't have any "limited time window", But ofc as sooner as better

@asodugf12
Copy link

This comes a little late, I've been diving into serde and I just realised our NBT crate is a little more inspired than I previously thought. Maybe the issues comes from the parts we snipped off?

@asodugf12
Copy link

Also, I'd very much appreciate some examples where out crate fails. Thanks a lot!

@asodugf12
Copy link

From the look of things, I want to try and implement our own Visitor for the deserialization. What do you think?

@neeleshpoli
Copy link
Contributor

neeleshpoli commented Jan 18, 2025

Also, I'd very much appreciate some examples where out crate fails. Thanks a lot!

In instances where fastnbt is used, you can replace it with pumpkin nbt and see the errors that occur. We don't have specific examples.

From the look of things, I want to try and implement our own Visitor for the deserialization. What do you think?

I think alex really wants to get this working, so I think he would be ok with this.

@asodugf12
Copy link

I did replace it and did cargo run without issues. I couldn't test the server however, since I don't have official minecraft. Alex did tell me that the unnamed version worked fine, but named tries to copy too many bytes into a buffer.

I also noticed that from_bytes_unnamed has issues reading nbt files, but I'll look into that later :D

@asodugf12
Copy link

It did not have issues reading nbt files, I used the wrong function lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
3 participants