ForeverZer0 / SharpNBT

A pure CLS-compliant C# implementation of the Named Binary Tag (NBT) format specification commonly used with Minecraft applications, allowing easy reading/writing streams and serialization to other formats.
MIT License
25 stars 9 forks source link

Allow compound tag with empty name #33

Closed vfrz closed 2 months ago

vfrz commented 2 months ago

Since this commit (https://github.com/ForeverZer0/SharpNBT/commit/bcd41b37f08fdade66f44ad4a71d2156cb06d750) it is not possible to write compound tag with empty name and the root tag name of the level.dat file generated by a minecraft server is empty.

ForeverZer0 commented 2 months ago

That is a definite oversight on my part.

While this PR does fix the more severe regression, it will also re-introduces the possible bug that the commit fixed, which is not permitting the writing of a nameless tag (at any nesting level). The ideal solution is to disallow it (possibly throwing an exception when it is attempted), with the only exception of a root-level compound tag.

If you were willing or wanted to update this PR with that, I will hold off before merging. Otherwise, I will open an issue to fix that at another time, and simply merge this as-is. An incorrectly formed tag being written due to user error is definitely less of a concern than a correctly written tag not being written when it should be. Just let me know if you have the time either way, I thank you for the contribution.

vfrz commented 2 months ago

Alright I understand. I'll think about a solution and update my PR. Right now the TagWriter don't track the "nesting level", do you have any recommendation on how to implement that? Should I add a simple boolean that switch after the first write?

ForeverZer0 commented 2 months ago

@vfrz A simple check if the current instance is a compound tag and additionally has a null parent should be sufficient. Something like:

        if (tag.Parent is ListTag))
            return;
        if (string.IsNullOrEmpty(tag.Name))
        {
            if (!(tag is CompoundTag && tag.Parent is null))
                return;
        }

I didn't test this in the project, so forgive any obvious errors it may have, but hopefully give the general idea.

vfrz commented 2 months ago

@vfrz A simple check if the current instance is a compound tag and additionally has a null parent should be sufficient. Something like:

        if (tag.Parent is ListTag))
            return;
        if (string.IsNullOrEmpty(tag.Name))
        {
            if (!(tag is CompoundTag && tag.Parent is null))
                return;
        }

I didn't test this in the project, so forgive any obvious errors it may have, but hopefully give the general idea.

Oh, I didn't knew about the Parent property, that might do the job, thanks for the insight. I'll give a try later today :)

ForeverZer0 commented 2 months ago

@vfrz Awesome, thanks again!