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 x-axis bug in actions.scaleTo #2471

Merged
merged 1 commit into from
Aug 26, 2022

Conversation

reissbaker
Copy link
Contributor

@reissbaker reissbaker commented Aug 26, 2022

Previously if scaling only on the x-axis, the action would never complete.

===:clipboard: PR Checklist :clipboard:===

  • 📌 issue exists in github for these changes
  • 🔬 existing tests still pass
  • 🙈 code conforms to the style guide
  • 📐 new tests written and passing / old tests updated with new scenario(s)
  • 📄 changelog entry added (or not needed) I think it's not needed for a small bugfix?

==================

Closes #2470

Changes:

Fixes a bug I found in the ScaleTo action, where scaling on the x-axis exclusively may never complete. The isComplete method accidentally tested tx.scale.y against the x-axis, rather than testing tx.scale.x against the x-axis. I wrote the test first, watched it fail, and then fixed the bug and watched the test pass.

It looks like since I'm a first-time contributor I'll need approval from a maintainer to actually run the CI checks, but I pinky swear I ran npm run all locally and it all passed. 😄

Previously if scaling only on the x-axis, the action would never
complete.
Copy link
Member

@eonarheim eonarheim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@reissbaker Thanks for the contribution! This is fantastic!

@eonarheim eonarheim merged commit 52bee8a into excaliburjs:main Aug 26, 2022
@reissbaker
Copy link
Contributor Author

Thanks for the quick merge! :)

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.

actions.scaleTo never completes when scaling exclusively on the x-axis
2 participants