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

Run-Time List Item Type Check #14

Closed
Offroaders123 opened this issue Nov 28, 2022 · 2 comments
Closed

Run-Time List Item Type Check #14

Offroaders123 opened this issue Nov 28, 2022 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@Offroaders123
Copy link
Owner

At the NBT writing stage, add a check that ensures all list items are of the same type as the first item in the list. If they aren't, it will throw a writing error.

@Offroaders123 Offroaders123 self-assigned this Nov 28, 2022
@Offroaders123 Offroaders123 added the bug Something isn't working label Nov 28, 2022
@Offroaders123
Copy link
Owner Author

Added the check for this! Now if you try to serialize your NBT structure, and it has a ListTag with items of multiple types, it will throw an error when it finds the first item of a different type than the first item in the array.

To help prevent this from happening, it helps to define the item type for the ListTags in your NBT objects using type definitions, so then you can get pre-run time linting errors before the buffer writing errors happen, say if you were using TypeScript, or @ts-check or allowJS/checkJS with JavaScript.

@Offroaders123
Copy link
Owner Author

I have to implement this behavior for the SNBT parser too, as that doesn't correctly handle this case yet.

@Offroaders123 Offroaders123 reopened this Jun 21, 2023
Offroaders123 added a commit that referenced this issue Sep 12, 2023
Added checks to the SNBT parser and stringifier that ensures ListTag items are all of the same type. Just like the reader and writer classes, the type is determined by the first item in the array.

Did things a bit different to implement this one, and it ended up working out really well! This time I started things off with updating the test to fail if the SNBT stringifier worked successfully when it shouldn't, then updated the code to make the test pass (or rather, to properly throw when it encounters an improperly-typed list item). Then I did the same thing for the parsing stage, made the test fail if it parsed successfully, then added the proper throw calls when the parsed items weren't all of the same type as the first item in the array. This makes me feel stronger about the checking here, because I passed it data that *shouldn't* work, and it only passes the test if the program doesn't accept that malformed test demo. Great way to do things like that! Now I just want to refactor the test to make it much less ugly, lol. But I also don't want to break it's behavior in doing that, it could cause issues if it lets unchecked changes pass though to the library if they aren't being validated correctly, compared to this version.

Man, I really still have to rework the SNBT parse module, it's so hacky at the moment. At least, it feels that way. It works great though, so I'd rather have that over anything else!

#14
Offroaders123 added a commit that referenced this issue Sep 15, 2023
As with how I've been doing the last few updates, I make a some of the changes from the few previous updates since the last version release, so you can go to each commit about these topics specifically to get more info about what changed, and to see discussion about the process along the way too. I think trying to summarize it again just when I publish it isn't worth it compared to just referencing you towards the original write up about the changes I did the first time around. You'll get more explanations and such, I think it's more worth it :)

- ...and optional ListTag values
- RootTagLike (& RootTag default return types)
- Run-Time List Item Type Check (SNBT)
- Format Options Generic Reverting

#14
#19
#25
#28
#29
#31
#32
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