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

Add support for json lines #39

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cactusdualcore
Copy link

In accordance with SemVer, I bumped the minor version. Not too sure whether this I wanted though.

@cactusdualcore
Copy link
Author

I was thinking about adding hjson support as well. If that is desired, I could implement it too. If possibly, I would include it in this PR before merging so as to only bump the version to 0.12?

@NiklasEi
Copy link
Owner

NiklasEi commented Aug 6, 2024

I don't see the benefit of json lines over a "normal" json file containing an array. Do you have a use case that does not work with normal json?

@cactusdualcore
Copy link
Author

I don't see the benefit of json lines over a "normal" json file containing an array. Do you have a use case that does not work with normal json?

It's two parts preference, one part convenience. There are technical reasons why one would prefer jsonlines over standard json, most importantly improved streamability, i.e. it's trivial to incrementally read and write the jsonlines format.

Ideally, I would have implemented something like

asset_server.load("all_the_assets.jsonl#1"); // loads the asset in the jsonlines file

This would allow short circuiting the asset loader for large files. I don't know enough about Bevy asset loaders to implement this well.

Besides streaming I can't think of any technical advantages of jsonlines though. Either way, it's effortless to convert a json array to jsonlines and back. Just add/remove brackets and commas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants