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

fix(core): introduce DojoStore trait to handle some specific storage cases #3051

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

remybar
Copy link
Contributor

@remybar remybar commented Feb 19, 2025

Description

In the current Dojo version, Option<T> and custom enums are not correctly stored in the world storage, because the first variant id is 0 and cannot be differenciated from an uninitialized variant.

To fix that, this PR introduces the DojoStore trait.

  • Every data structure implement the DojoStore trait which, by default, encapsulates Serde calls.
  • The implementation of the DojoStore trait for custom enums and Option<T> handles the variant index incrementing and returns Default::default() as default value during deserialization if the variant index is 0. That means, each custom enums must implement the Default trait.

As these changes may break the world storage, we also introduce the DojoLegacyStorage derive attribute. This attribute must be used on models for which we want to keep the current storage behavior. So, by default, model (de)serialization is done throughDojoStore. If DojoLegacyStorage is set, model (de)serialization is done through Serde.

Tests

  • Yes
  • No, because they aren't needed
  • No, because I need help

Added to documentation?

  • README.md
  • Dojo Book
  • No documentation needed

Checklist

  • I've formatted my code (scripts/prettier.sh, scripts/rust_fmt.sh, scripts/cairo_fmt.sh)
  • I've linted my code (scripts/clippy.sh, scripts/docs.sh)
  • I've commented my code
  • I've requested a review after addressing the comments

@remybar remybar force-pushed the fix-enum_options_storage branch from dd5fa51 to ba0f063 Compare February 26, 2025 11:09
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.

1 participant