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

Calculate distance correctly for animations using byValue #14

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Calculate distance correctly for animations using byValue #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a93e10
Fix a bug where animations using byValue did not use the correct dist…
May 25, 2017
fa7e61a
Update podspec version
May 25, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion SpriteKit-Spring.podspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

Pod::Spec.new do |s|
s.name = "SpriteKit-Spring"
s.version = "1.0.1"
s.version = "1.1.0"
Copy link
Owner

Choose a reason for hiding this comment

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

Let's not mix code changes with version bumps in the same PR. I'll take care of bumping the version and updating the pod once all PRs are merged.

s.summary = "SpriteKit API reproducing UIView's spring animations with SKAction"
s.homepage = "https://github.com/ataugeron/SpriteKit-Spring"
s.screenshots = "https://ataugeron.github.io/SpriteKit-Spring/bounce_1.gif", "https://ataugeron.github.io/SpriteKit-Spring/bounce_2.gif", "https://ataugeron.github.io/SpriteKit-Spring/bounce_3.gif"
Expand Down
2 changes: 1 addition & 1 deletion SpriteKit-Spring.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ public extension SKAction {
if initialValue == nil {

initialValue = node.value(forKeyPath: keyPath) as! CGFloat
initialDistance = initialDistance ?? finalValue - initialValue!
initialDistance = initialDistance != nil ? initialDistance * initialValue - initialValue : finalValue - initialValue!
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for catching this issue. However I don't think this is the correct fix. It appears the behavior of the "by" param in SKAction is inconsistent:
SKAction.scaleX(by: 0.5, y: 0.5, duration:0) // will divide xScale and yScale by 2
SKAction.moveBy(x: 0.5, y: 0.5, duration: 0) // will add 0.5 to position.x and position.y

For this reason I don't think we should change the behavior of the animate(keyPath:...) functions, but just adjust the behavior of the scaleBy(...) functions above to compute the correct byValue to give to animate(keyPath:...).

Does this make sense?

Copy link
Author

Choose a reason for hiding this comment

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

Wow you're right, great catch! I agree, since scale is the only multiplicative SKAction we should handle it as a special case in the scaleBy functions.

finalValue = finalValue ?? initialValue! + initialDistance

var magicNumber: CGFloat! // picked manually to visually match the behavior of UIKit
Expand Down