Skip to content

Fix enum codec #1621

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

Merged
merged 3 commits into from
May 4, 2022
Merged

Fix enum codec #1621

merged 3 commits into from
May 4, 2022

Conversation

filmor
Copy link
Member

@filmor filmor commented Nov 23, 2021

A boxed enum value can't be casted directly to an integer, but using System.Convert
functions instead works fine.

What does this implement/fix? Explain your changes.

...

Does this close any currently open issues?

...

Any other comments?

...

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Add yourself to AUTHORS
  • Updated the CHANGELOG

@filmor filmor mentioned this pull request Nov 23, 2021
@lostmsu
Copy link
Member

lostmsu commented Nov 23, 2021

Is there a test for UInt64.MaxValue? Removal of try ... catch ... looks suspicious.

@filmor
Copy link
Member Author

filmor commented Nov 23, 2021

Apparently there were no tests at all for this :)

I'll look into creating some.

A boxed enum value can't be casted directly to an integer, but using `System.Convert`
functions instead works fine.
@filmor filmor force-pushed the fix-enum-codec branch from 734e549 to df3b569 Compare May 4, 2022 06:15
@filmor filmor force-pushed the fix-enum-codec branch from 9e70b8f to 7d6e27a Compare May 4, 2022 06:40
@filmor
Copy link
Member Author

filmor commented May 4, 2022

After finally understanding what you meant, it works now ;)

The old codec didn't handle ulong either, that's why I didn't consider it.

@filmor filmor merged commit ac75e0c into master May 4, 2022
@filmor filmor deleted the fix-enum-codec branch May 4, 2022 07:38
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