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

Enum attributes #396

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Enum attributes #396

wants to merge 1 commit into from

Conversation

krakow10
Copy link

@krakow10 krakow10 commented Feb 23, 2024

For #383

This is my guess as to how to implement this, assuming there is some sort of string in the serialized attributes. It builds but it does not work. Some sort of reflection database error that I don't know what it means.

thread 'main' panicked at /home/quat/git/rbx-dom/rbx_reflection_database/src/lib.rs:7:76:
could not decode reflection database because: invalid type: integer `0`, expected struct Enum
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@krakow10
Copy link
Author

krakow10 commented Feb 23, 2024

println!("heyyy name={} value={}", enum_name, enum_value);

image
I guessed right 😎
still don't know how to work the reflection database

@Dekkonot
Copy link
Member

It builds but it does not work. Some sort of reflection database error that I don't know what it means.

This is because you're changing the (de)serialization of Enum! As a result, the reflection database can't be deserialized by the new version you're making.

Fixing this is an involved processs because we bootstrap the database. As a result I don't really recommend you try to fix it because it's potentially error prone and involves a bit of juggling with reflection databases.

PS: We have the enum attributes documented in the spec file we maintain. You didn't have to guess. 😄

@krakow10
Copy link
Author

krakow10 commented Feb 23, 2024

What about a separate explicit enum type, like this?

@krakow10 krakow10 force-pushed the attribute-21 branch 3 times, most recently from 502e563 to 1d4f34c Compare February 25, 2024 01:52
@krakow10
Copy link
Author

This branch is now set up so that it suppresses the enum attribute errors and provides weakly typed enum functionality. The explicit enum is moved to a different branch, never to be heard from again.

@Dekkonot
Copy link
Member

This implementation would fail a round-trip test because Enums no longer contain information on their type.

@krakow10
Copy link
Author

Well yes... but this is still useful as an error-suppressing patch while the full functionality isn't available

@Dekkonot
Copy link
Member

I will put aside time next week to fix this properly since it's showing up in the wild now, but I'm not willing to merge a feature that doesn't roundtrip. We won't be making a new release without the full feature anyway, so it being temporary fix is moot.

If it's desperate, I'd suggest using a custom fork of whatever tool you're using for the next little while.

@krakow10
Copy link
Author

I don't think it should be merged either which is why I have it as a draft, sorry for the misunderstanding. I'm not trying to get this incomplete feature merged, just a publicly available error fixer

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