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.

Sign up for GitHub

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

Jump to bottom

Make sure to update euler or quaternion rotation before returning it to the user #7540

Merged
merged 1 commit into from Apr 5, 2023
Merged

Make sure to update euler or quaternion rotation before returning it to the user #7540

merged 1 commit into from Apr 5, 2023

Conversation

britzl
Copy link
Contributor

@britzl britzl commented Apr 3, 2023

This fixes an issue when getting the rotation or euler property of a game object in a go.animate() callback where the value was not updated to the final value at the time when the property was read.

In the case when getting an euler angle using go.get() and the euler angle has not changed it is first updated from the quaternion value. And in the case when getting a quaternion rotation using go.get() and the euler angle has changed the quaternion is updated with the value from the euler angle.

Fixes #7253

PR checklist

  • Code
    • Add engine and/or editor unit tests.
    • New and changed code follows the overall code style of existing code
    • Add comments where needed
  • Documentation
    • Make sure that API documentation is updated in code comments
    • Make sure that manuals are updated (in github.com/defold/doc)
  • Prepare pull request and affected issue for automatic release notes generator
    • Pull request - Write a message that explains what this pull request does. What was the problem? How was it solved? What are the changes to APIs or the new APIs introduced? This message will be used in the generated release notes. Make sure it is well written and understandable for a user of Defold.
    • Pull request - Write a pull request title that in a sentence summarises what the pull request does. Do not include "Issue-1234 ..." in the title. This text will be used in the generated release notes.
    • Pull request - Link the pull request to the issue(s) it is closing. Use on of the approved closing keywords.
    • Affected issue - Assign the issue to a project. Do not assign the pull request to a project if there is an issue which the pull request closes.
    • Affected issue - Assign the "breaking change" label to the issue if introducing a breaking change.
    • Affected issue - Assign the "skip release notes" is the issue should not be included in the generated release notes.

@britzl
Check if euler has changed before returning value
f69a3ec 
Check if the euler angle has changed before returning a rotation or euler value in GetProperty.

When getting an euler angle:
If the euler angle hasn't changed we should update the euler angle with the value from the quaternion

When getting a quaternion:
If the euler angle has changed it is likely that we are getting the quaternion in a go.animate callback, in which case we should update the quaternion with the value of the euler angle
@britzl britzl requested a review from JCash April 3, 2023 11:11
@britzl britzl merged commit 347ff83 into dev Apr 5, 2023
22 checks passed
@britzl britzl deleted the Issue-7253-animate-euler-and-go-get-returns-old-value branch April 5, 2023 08:37
Jhonnyg pushed a commit that referenced this pull request Apr 14, 2023
@britzl @Jhonnyg
Check if euler has changed before returning value (#7540)
4f6f1e6 
Check if the euler angle has changed before returning a rotation or euler value in GetProperty.

When getting an euler angle:
If the euler angle hasn't changed we should update the euler angle with the value from the quaternion

When getting a quaternion:
If the euler angle has changed it is likely that we are getting the quaternion in a go.animate callback, in which case we should update the quaternion with the value of the euler angle
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JCash JCash JCash approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Animate complete function is called earlier than expected when animating euler property
2 participants
@britzl @JCash