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.

Sign up for GitHub

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

Jump to bottom

Improve NBT editor #401

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Improve NBT editor #401

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c398cae
Fixed a but with NBTFile objects, ValueErrors how replace the value f…
Podshot Apr 2, 2021
ecb8443
Fixed an issue with container NBT types
Podshot Apr 9, 2021
4cdc9c1
Improved NBT Editor by catching errors in tag values, better tree upd…
Podshot May 25, 2021
0519d26
Added function typing, renamed some internal functions, unified Parse…
Podshot May 25, 2021
9518c2d
Implemented rebuilding of NBT data from ParsedNBT objects
Podshot May 25, 2021
f5f9350
Added __init__() super calls and improved image dictionary
Podshot Aug 20, 2021
370c02a
Swapped out the file on ben's PC for a bytes object
gentlegiantJGC Aug 25, 2021
225cad5
Started reworking the nbt_parse() function and how the NBT tree is built
Podshot Sep 6, 2021
aedb991
Fixed an issue with unintentionally creating a nested compound tag wh…
Podshot Oct 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion amulet_map_editor/api/wx/ui/nbt_editor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ def _save(self, _):

if __name__ == "__main__":
level_dat = nbt.load(
r"C:\Users\gotharbg\Documents\Python Projects\Amulet-Core\tests\worlds_src\java_vanilla_1_13\level.dat"
b"\x0A\x00\x0B\x68\x65\x6C\x6C\x6F\x20\x77\x6F\x72\x6C\x64\x08\x00\x04\x6E\x61\x6D\x65\x00\x09\x42\x61\x6E\x61\x6E\x72\x61\x6D\x61\x00"
Copy link
Member Author

@Podshot Podshot Aug 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an invalid NBT bytestring, since the TAG_String is not wrapped in a TAG_Compound, which is the reason this string crashes the UI

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No that is a valid nbt format.
The first byte (0A) is the start of the compound tag

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And a tag string can be standalone in the binary format it just isn't used

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought all valid NBT had to be wrapped in a TAG_Compound? The NBTFile object shouldn't be acting as that TAG_Compound

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm referencing this page for whether the byte string is valid NBT or not: https://wiki.vg/NBT#Specification

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are misreading it a bit. I assume you are referring to this line:
Every NBT file will always implicitly be inside a tag compound, and also begin with a TAG_Compound. The main word being implicitly

Normally a dictionary will own the keys and store the values under those keys. Binary NBT is defined a bit backwards.
Instead of the compound tag owning the keys, the tag itself owns the keys. This is because the root tag also has a name.

The implicit compound tag is there to store the leading name however it is only used in Java level.dat files. Everywhere else it is blank. The NBTFile class is just there to handle the leading name if it isn't blank.

It is a bit complex. I am not sure if I explained it that well

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The level.dat file in Java has a double nested compound tag which might be where the confusion is coming from. Try loading up a bedrock level.dat file. I think those only have one compound tag

Copy link
Member

@gentlegiantJGC gentlegiantJGC Aug 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the code you will need to read a bedrock level.dat file

with open(path, "rb") as f:
    f.read(8)
    dat = f.read()
    level_dat = nbt.load(
        dat,
        little_endian=True
    )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also while doing some debugging I realised our nbt library cannot de-serialise binary root tags that are not compound tags. This isn't really an issue though since I don't think it is ever used but they can be constructed either from SNBT or by directly using the classes so the viewer should be able to display them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two solutions to this are allowing the NBTFile to contain all tag types and
a) make the API less specific and have code get the actual nbt object
b) modify the API to pass all calls to the actual object
or just mark the issue as won't fix and leave NBTFile as handling just compound tags

)

app = wx.App()
Expand Down