Calculate distance correctly for animations using byValue#14
Calculate distance correctly for animations using byValue#14noahsark769 wants to merge 2 commits intoataugeron:masterfrom
Conversation
|
@ataugeron bump! Would be awesome to get this out and pushed as a new version of the cocoapod 👍 |
| Pod::Spec.new do |s| | ||
| s.name = "SpriteKit-Spring" | ||
| s.version = "1.0.1" | ||
| s.version = "1.1.0" |
There was a problem hiding this comment.
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.
|
|
||
| initialValue = node.value(forKeyPath: keyPath) as! CGFloat | ||
| initialDistance = initialDistance ?? finalValue - initialValue! | ||
| initialDistance = initialDistance != nil ? initialDistance * initialValue - initialValue : finalValue - initialValue! |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Hey! Awesome library, thanks for building it. I noticed that there's a bug with animations that use
byValue, e.g.:The distance isn't calculated correctly, so the behavior uses
byValueas the distance.For example, say there's a node with a current scale of 1.0 - if we tried to scale it by 1.2, SpriteKit-Spring will currently scale it to
1.0 + 1.2instead of1.0 * 1.2, which in this case would more than double the scale instead of scaling appropariately. I put up an example app at https://github.com/noahsark769/NGSpriteKitSpringTest to demonstrate this.The fix is to calculate the
initialDistancebased on multiplying it byinitialValue.Let me know if I need to do anything else to fix this bug! Thanks!