-
-
Notifications
You must be signed in to change notification settings - Fork 386
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
Fix/number converters #5079
Fix/number converters #5079
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.
I suggest adding a test for every new Number converter. This may also cause issues with existing syntaxes due to the nature of using Number as the return type rather than it's actual type like Hunger of a player returning Double. Most syntaxes just return Number (Mainly addons, but Skript still has some old syntaxes that do this)
@TheLimeGlass How would you imagine writing such tests in a script? The only types scripts can use are number (java.lang.Number) and integer (java.lang.Long), and no syntaxes require a specific number type, all take %number%. This requirement would no longer be needed after this PR, as converters would take care of it. However, it would also not be worth changing, unless the syntax element requires the wrapper type instead of the primitive, which is not often. I also don't see how it could cause issues, can you elaborate on that? |
As discussed, JUnit tests are gonna be best for this solution. |
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.
@TPGamesNL conflicts
Description
Adds more number converters, from the generic
java.lang.Number
type to specific subclasses.Target Minecraft Versions: any
Requirements: none
Related Issues: #4445