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

Circular Data Structure Handling #22

Open
Offroaders123 opened this issue Jan 20, 2023 · 0 comments
Open

Circular Data Structure Handling #22

Offroaders123 opened this issue Jan 20, 2023 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@Offroaders123
Copy link
Owner

Thinking about the behavior of JSON.stringify(), I remembered that it throws an error when encountering circular object references. I realized that I haven't specifically handled that in NBTify, so you simply get to a Maximum call stack exceeded error when writing a circular object structure to NBT. This isn't ideal, and I should detect this before the user runs into it, just like JSON.stringify() does.

Detecting and fixing circular references in JavaScript - Stack Overflow
TypeError: cyclic object value - MDN

Found an example of how to detect them on MDN, this one can be used as a JSON replacer callback, and it will remove any circular object references before serializing to JSON. While this essentially makes the circular object "fully" serializable, I will go with throwing an error instead, like JSON.stringify() does, since I want it to be something that should be handled specifically by the user, where they can choose what to do with the circular references, rather than implicitly removing them inside of NBTify.

const getCircularReplacer = () => {
  const seen = new WeakSet();
  return (key, value) => {
    if (typeof value === "object" && value !== null) {
      if (seen.has(value)) {
        return;
      }
      seen.add(value);
    }
    return value;
  };
};

JSON.stringify(circularReference, getCircularReplacer());
// {"otherData":123}

Because of that, it also further sways me to throw an error for inconsistent ListTag item types, rather than silently excluding them from the written NBT buffer (#21).

I think I will silently remove unsupported JavaScript types during the writing process though, like for null, undefined, Symbol, and function values (#20). That's what JSON.stringify() does for any function values on objects, and I think it keeps things straightforward to be able to simply pass most objects right into NBTify, but also still be explicit in throwing knowledgeable errors for object structure issues, like circular object references and array item type inconsistencies.

@Offroaders123 Offroaders123 added the bug Something isn't working label Jan 20, 2023
@Offroaders123 Offroaders123 self-assigned this Jan 20, 2023
Offroaders123 added a commit to Offroaders123/Bob that referenced this issue Nov 29, 2024
Heck yeah!!! JSON replacer-reviver now works completely in implementing a recursive object structure.

Offroaders123/NBTify#22
Offroaders123/NBTify#33
Offroaders123 added a commit that referenced this issue Nov 30, 2024
Checking that the order properties are logged is in the same order that JSON.parse() logs them. Stringifying should be BFS, while parsing should be DFS. It's the order that they are traversed as the file is read/written through.

In theory this means that you would be able to write a replacer/reviver pair and it would work both with JSON and NBT, if you did things in a certain way. This seems like a very neat thing that could make converting between the formats much more safe. It seems like a natural progression for adding custom primitives to either format, by way of essentially transpiling them in.

Come to think of it, this would actually work very well for converting to and from NBTify primitives to Prismarine-NBT objects as well, that might be a nice way to add that to NBTify itself without needing to implement custom functions or options for that outright.

#33
#22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant