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

single node movements #46

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

single node movements #46

wants to merge 52 commits into from

Conversation

bj97301
Copy link
Contributor

@bj97301 bj97301 commented Feb 21, 2018

Checklist

Motivation and Context

I wanted to allow the user to move an individual node.

Description

Allows for single node movements. Also exposed a field for making it easier to select a node: nodeSelectionForgivenessDistance.

@bj97301
Copy link
Contributor Author

bj97301 commented Feb 21, 2018

Found an issue while trying to select a node on an iPad. Looking into it now.

@bj97301
Copy link
Contributor Author

bj97301 commented Feb 21, 2018

K fixed

@bj97301
Copy link
Contributor Author

bj97301 commented Feb 22, 2018

Going to take a look at the codebeat thing tomorrow

This will help the node stay under the user's finger when the physics
strength is higher.
@efremidze
Copy link
Owner

Don't worry about Codebeat, Ill review the PR later today

/**
Lets the user move all of the nodes at once. On by default.
**/
open var allowAllNodeMovement: Bool = true
Copy link
Owner

Choose a reason for hiding this comment

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

aren't allowSingleNodeMovement and allowAllNodeMovement mutually exclusive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First, thank you for the feedback. They could be but it more fun having both on like this:
2018-02-23 10_48_42

Should update the code comments to be more descriptive?

movingNodeTimer?.invalidate()
movingNodeTimer = nil
}
movingNodeTimer = Timer.scheduledTimer(timeInterval: 0.01, target: self, selector: #selector(self.moveNode(timer:)), userInfo: ["touchLocation":touchLocation, "node": node], repeats: true)
Copy link
Owner

@efremidze efremidze Feb 22, 2018

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would love to but it seems to only be available in iOS 10 or later.

var movingNode: SKNode? = nil
var initialTouchLocation: CGPoint? = nil
var initialTouchStartedOnNode: Bool = false
var movingNodeTimer: Timer? = nil
Copy link
Owner

Choose a reason for hiding this comment

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

maybe use a state machine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whats a state machine? Got any good examples?

@bj97301
Copy link
Contributor Author

bj97301 commented Feb 23, 2018

@efremidze I am still making some tweaks fyi. I will give you a heads up when its more finalized.

@bj97301
Copy link
Contributor Author

bj97301 commented Feb 23, 2018

@efremidze I am feeling better about the pr now. Feel free to check out the new changes at your convenience.

@efremidze
Copy link
Owner

Ill review it this week. I might want to make some changes.

@efremidze
Copy link
Owner

I'll try to include this asap

@bj97301
Copy link
Contributor Author

bj97301 commented Apr 12, 2018

Woot. Thank you. Now that xcode 9.3 is out let me know if you want me to resolve the conflicts first.

@chintanMaisuriya
Copy link

Hello guys,
I didn't found this individual node movement feature in the latest release. How can I get this?
Please help, Thanks.

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.

7 participants