Offroaders123 / NBTify

A library to read and write NBT files on the web!
http://npm.im/nbtify
MIT License
42 stars 5 forks source link

SNBT Improved Error Parsing Messages #39

Open Offroaders123 opened 1 year ago

Offroaders123 commented 1 year ago

A reference to the discussion from https://github.com/Offroaders123/Dovetail/issues/5:

I also think I need to look into improving the error messages with as to what part of the SNBT content isn't valid/parsing correctly. Say for this error, I could add a reference that it's in the array at key InvalidBecauseTheValuesArentOfTheSameType at item index [2], where that item is of type ShortTag, but the ListTag here can only accept ByteTag values. Something along the lines of that.

Current SNBT Parsing Error Message

I think I can better accomplish this kind of handling if I bring the SNBT parser implementation back to a fully functional approach, rather than managing it with a class kind of thing. I can use callbacks of some sort which could allow the use of parameters that have the current key name and value or something like that, same with the current array index, item type, and other similar metadata-related things. It's like how you can use [].map((item,index,array) => {}) to get information about the iteration, as well as the source data you're working with, right from the callback.

Maybe I can look again into a more functional approach for the rest of the NBT reading and writing too, it doesn't have to be exclusively for SNBT handling. I do think the use of a class to manage the internal ArrayBuffer and DataView references is a little simpler to understand than passing them around everywhere too though. So, we'll see about that one. I feel SNBT's work with string manipulation and appending is a bit more akin to simple return types though, hence why it works naturally with a functional approach. It's not as straightforward to do that for the NBT parsing/writing counterparts.

Offroaders123 commented 1 year ago

Further started to address implementing this enhancement in the commit above!

The naming of tag types here makes me want to migrate to an enum for the TAG constant mapping. That would make name appearance much easier, as well as just nicer here, plain out of the box. I don't like having to deal with the type-magic that comes with enums though. So, once again, we'll see. (Sleepy-self still talking here, I mean like using things such as List, ListTag, or TAG.LIST/LIST instead of the TAG number IDs themselves. I don't think the numerical IDs are very explanatory to the user, because they are just numbers. I know what each of the IDs are, but the error has to be debuggable by non-NBT fanatics like moi)

image