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

Split TagReader and TagWriter into separate sub-classes #22

Open
ForeverZer0 opened this issue Aug 27, 2023 · 0 comments
Open

Split TagReader and TagWriter into separate sub-classes #22

ForeverZer0 opened this issue Aug 27, 2023 · 0 comments
Assignees
Labels
enhancement New feature or request optimization Performance improvement
Milestone

Comments

@ForeverZer0
Copy link
Owner

Problem

There are currently three different ways that NBT can be serialized:

Format Endianness VarInt
Java Big None
Bedrock (file) Little None
Bedorck (network) Little Yes. Sometimes they are ZigZag encoded using the ProtoBuf style, other times not.

I will reserve disparaging Microsoft for needlessly complicating one of the most simple serialization formats in existence. It is par for the course, and not even unexpected anymore. I would be more surprised if they didn't mess it up.

Regardless, the TagReader and TagWriter class accounts for all scenarios using the format options passed to its constructor. While this works in practice, it means most methods have to do additional checks to see what the appropriate way to read the data is.

Proposal

Make TagReader and TagWriter abstract, with a factory method to create an instance. Internally these different ways of reading/writing can be split into separate classes that do not require any runtime checks.

Something akin to this psuedo-code:

public static TagReader Create(Stream stream, Format format)
{
    return format switch
    {
        Format.Java => new JavaReader(stream),
        Format.BedrockFile => new BedrockFileReader(stream),
        Format.BedrockNetwork => new BedrockNetworkReader(stream),
        _ => throw new ArgumentOutOfRangeException()
    };
}

The actual implementations can be completely transparent and need not even be part of the public API.

@ForeverZer0 ForeverZer0 added enhancement New feature or request optimization Performance improvement labels Aug 27, 2023
@ForeverZer0 ForeverZer0 added this to the 2.0 milestone Aug 27, 2023
@ForeverZer0 ForeverZer0 self-assigned this Aug 27, 2023
@ForeverZer0 ForeverZer0 changed the title Split TagReader into separate subclasses Split TagReader and TagWriter into separate sub-classes Aug 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request optimization Performance improvement
Projects
None yet
Development

No branches or pull requests

1 participant