-
Notifications
You must be signed in to change notification settings - Fork 3
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
Issue #3 - Changed classes to be records #5
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks okay to me. Love all the getters records remove.
Only looked at the changes in GitHub, not IntelliJ, but it all looks good to me. Waiting for review from the big boss Cassy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, I'd like to test it before merging, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After testing on our dev server, this error occurred when loading the server's pre-existing data. You can use this sample playerdata to test your code further.
@funnyboy-roks @Majekdor @Cassy343 So after testing a bit, I found out it's an error from Gson itself not being able to deserialize records properly, it's a known Gson issue. There are workarounds provided by the community, but the simplest fix is just to revert Birthday back to a class. |
I changed some classes in the data package to be records. I chose the classes to be converted based on how the previous code declared attributes in the class, I chose classes that declared all the attributes as final.
Then after converting those classes to records I had to change how other classes retrieved data using the generated getters from records.