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

Allow a database to be specified in both rbx_xml and rbx_binary #375

Merged
merged 13 commits into from
Nov 10, 2023

Conversation

Dekkonot
Copy link
Member

@Dekkonot Dekkonot commented Nov 9, 2023

This allows the reflection database to be specified for serializing/deserializing in rbx_binary and rbx_xml. It does this by having the structs for these crates store a reflection database rather than using rbx_reflection_database inline. This doesn't change how the crates are used normally since they use the bundled database by default, but it provides the opportunity for tools like Rojo to specify their own database.

I took some liberties here also, and cleaned up the name of lifetimes a bit. Specifically, I've changed it so that all lifetimes that refer to a reflection database use a 'db lifetime and all lifetimes that refer to a WeakDom use a 'dom lifetime. This is to avoid confusion because 'a is not specific or helpful.

@Dekkonot Dekkonot changed the title Allow a database to be specified manually in both rbx_xml and rbx_binary Allow a database to be specified in both rbx_xml and rbx_binary Nov 9, 2023
Copy link
Contributor

@nezuo nezuo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I appreciate the approach taken in the PR as it doesn't result in any breaking changes.

@Dekkonot Dekkonot merged commit 1257a0d into rojo-rbx:master Nov 10, 2023
2 checks passed
@Dekkonot Dekkonot deleted the unbundle-the-db branch November 10, 2023 17:51
Dekkonot pushed a commit that referenced this pull request Dec 13, 2023
Closes #345

Because of #375, it's possible to continue using
`DecodePropertyBehavior::IgnoreUnknown` and simply swap to the
in-progress database via `DecodeOptions::reflection_database`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants