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

Fix tween_method() type validation #73178

Merged
merged 1 commit into from
Feb 12, 2023

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Feb 12, 2023

Fixes #72808

tween_method() will now validate types and auto-convert between int and float, just like tween_property().

@KoBeWi KoBeWi added this to the 4.0 milestone Feb 12, 2023
@KoBeWi KoBeWi requested a review from a team as a code owner February 12, 2023 21:09
@TokageItLab
Copy link
Member

TokageItLab commented Feb 12, 2023

I think this PR is fine assuming the current implementation.

BTW, what do you think about #53548? Maybe we should discuss whether double should be cast int first and interpolated as in current, or whether all int values should be double once and cast last.

@akien-mga akien-mga merged commit e3b07bf into godotengine:master Feb 12, 2023
@akien-mga
Copy link
Member

Thanks!

@KoBeWi KoBeWi deleted the tinder_for_types branch February 12, 2023 21:42
@KoBeWi
Copy link
Member Author

KoBeWi commented Feb 12, 2023

@TokageItLab Not sure what you mean. I think float/int interpolation is fine. I left a comment in the PR.

@TokageItLab
Copy link
Member

TokageItLab commented Feb 12, 2023

@KoBeWi For example, if you want to transition from 1 to 3.5, int(1)->int(3.5) and int(double(1.0)->double(3.5)) would give different results. Which is ideal?

@KoBeWi
Copy link
Member Author

KoBeWi commented Feb 12, 2023

Good question.
I didn't really see anyone complain about it, so I'd leave it as is for now. Depending on the feedback we might add a warning or rework as you suggest.
I think the most common use-case when mixing ints and floats is either when float is a rounded value or when int is the final value. In both cases the conversion is irrelevant.

@TokageItLab
Copy link
Member

Well, I agree that it is certainly not that common a case and is in the category of warnings and documentation issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants