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

[core:encoding/json] Enumerated arrays as objects, bit sets as arrays, fix map sorting, fix bit set endian #4350

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jakubtomsu
Copy link
Contributor

This PR has a whole bunch of changes to core:encoding/json which I wanted for a long time, as well as a number of bugfixes I found along the way.

Changes:

  • option to store enumerated arrays as maps where keys are either enum values or names
  • option to store bit sets as arrays of either integers or enum names
  • fix map sorting, previously it would return unsupported type when a map isn't sortable. This caused issues in big nested datastructures. Now it sorts the map if possible, otherwise it writes it unsorted. Also currently only maps with string keys are considered sortable, it might be a good idea to add support for all ordered types. But I didn't see a good way to implement that.
  • fix bit set endianness, it now swaps if endian is different than the platform

The reason why I need enumerated arrays and bitsets in a more readable form is to better handle changes in the underlying data structures. Currently it's unusable for e.g. game assets when enums can change over time, since enumerated arrays and bitsets will be out of order etc. And since the index in enumerated arrays and values in bitsets are implicit, it's difficult to procedurally upgrade assets from old format to a new one. Now it should be way more flexible since it's possible to always explicitly use the enum field name or value.

core/encoding/json/marshal.odin Outdated Show resolved Hide resolved
core/encoding/json/marshal.odin Outdated Show resolved Hide resolved
core/encoding/json/marshal.odin Outdated Show resolved Hide resolved
core/encoding/json/unmarshal.odin Outdated Show resolved Hide resolved
@jakubtomsu
Copy link
Contributor Author

@laytan do you know if the underlying type info can be something other than Type_Info_Integer? I can't think of anything.

Otherwise there could either be a regular union type assertion, or do assert(false) in the default switch case to catch this in a debug build.

@laytan
Copy link
Collaborator

laytan commented Oct 12, 2024

@laytan do you know if the underlying type info can be something other than Type_Info_Integer? I can't think of anything.

Otherwise there could either be a regular union type assertion, or do assert(false) in the default switch case to catch this in a debug build.

After doing a type_info_base to unpack Type_Info_Named I think you are right that it can only be Type_Info_Integer (or nil of course). I agree with adding something that would assert/panic if not the case.

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