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

feat: Enum: Add EnumConverter with underlying type lookup #9

Merged
merged 3 commits into from
May 10, 2024

Conversation

riffy
Copy link
Contributor

@riffy riffy commented May 8, 2024

Add enum support. Only supports enum numbers

grafik

Copy link
Owner

@duydang2311 duydang2311 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Have you tested this between client and webview? I think it might not work because webview only considers double for number types, that was why if you have a look at numeric converters, they are all casted to double for that reason to keep the compatibility.

chore: Enum: Discrete differentiation between prefix and suffixes (makes it easier to extend later if necessary)
@riffy
Copy link
Contributor Author

riffy commented May 9, 2024

Thanks for the PR!

Have you tested this between client and webview? I think it might not work because webview only considers double for number types, that was why if you have a look at numeric converters, they are all casted to double for that reason to keep the compatibility.

I've not tested it with a webview, but since all numeric enums are now casted as doubles, this shouldn't be a problem

@duydang2311
Copy link
Owner

duydang2311 commented May 10, 2024

Actually, string/char enum isn't a real thing in C# so you can simply get rid of it for this PR. Can you update it so that it just has the underlying type only please? This looks good to be merged without the string/char thing.

I had an initial idea of supporting enum serialization as string that you would annotate your class/property with [MValueMapEnum(EnumMappingStrategy.ByName)]. The converter then uses enumValue.ToString() for write, and Enum.Parse<YourEnum>(reader.NextString()) for read.

@riffy
Copy link
Contributor Author

riffy commented May 10, 2024

I have removed bool, string and char. Only (double) casting is left.

@duydang2311
Copy link
Owner

I have removed bool, string and char. Only (double) casting is left.

Perfect, thanks!

@duydang2311 duydang2311 merged commit f96d23f into duydang2311:dev May 10, 2024
@duydang2311 duydang2311 mentioned this pull request May 11, 2024
5 tasks
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.

None yet

2 participants