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

Implement Hash, PartialOrd, Eq, Ord for types #22

Closed
PonasKovas opened this issue Oct 5, 2024 · 4 comments
Closed

Implement Hash, PartialOrd, Eq, Ord for types #22

PonasKovas opened this issue Oct 5, 2024 · 4 comments
Assignees

Comments

@PonasKovas
Copy link

Hello, it would be awesome if the Nbt types could implement these traits, if possible.

And also, quick unrelated question - can the nbt writing methods ever fail with any error other than IO? maybe they should return Result<..., io::Error> instead of crab_nbt::Error?

@goteusz-maszyk
Copy link
Member

Well, hashmaps niether do support hashing nor ordering. We cannot also implement Eq because of floats.

@goteusz-maszyk goteusz-maszyk closed this as not planned Won't fix, can't repro, duplicate, stale Oct 7, 2024
@SzczurekYT
Copy link
Contributor

Poor research, not all of them, but some of the traits can be actually implemented.

@SzczurekYT SzczurekYT reopened this Oct 7, 2024
@SzczurekYT SzczurekYT self-assigned this Oct 7, 2024
@SzczurekYT
Copy link
Contributor

Alright
I thought more of the traits are actually missing then they are.
Sorry for calling you out @goteusz-maszyk
Your research was generally correct, but a bit inaccurately described in the comment.
This led @Norbiros to make this comment on #18

Maybe switch to Vec<String, NbtTag> instead of Hashmap to preserver order, and resolve #22

which made me a little upset.

The actual situation is as follows:

  • Hash, Eq, Ord can't be implemented in any way because nbt can store floats, which do not implement those traits. So putting stuff into a Vec instead of HashMap as @Norbiros suggested is not going to solve this issue in any way, and only going to cause more problems.
  • PartialOrd can be actually implemented and it is tracked here Implement PartialOrd for nbt types #23

@SzczurekYT
Copy link
Contributor

TLDR: PartialOrd has been implemented and other traits can't be implemented because of floats

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

No branches or pull requests

3 participants