Skip to content

single node movements#46

Open
bj97301 wants to merge 52 commits intoefremidze:masterfrom
bj97301:master
Open

single node movements#46
bj97301 wants to merge 52 commits intoefremidze:masterfrom
bj97301:master

Conversation

@bj97301
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

bj97301 commented Feb 21, 2018

K fixed

@bj97301
Copy link
Copy Markdown
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
Copy Markdown
Owner

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

Comment thread Sources/Magnetic.swift
/**
Lets the user move all of the nodes at once. On by default.
**/
open var allowAllNodeMovement: Bool = true
Copy link
Copy Markdown
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
Copy Markdown
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?

Comment thread Sources/Magnetic.swift Outdated
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
Copy Markdown
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
Copy Markdown
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.

Comment thread Sources/Magnetic.swift Outdated
var movingNode: SKNode? = nil
var initialTouchLocation: CGPoint? = nil
var initialTouchStartedOnNode: Bool = false
var movingNodeTimer: Timer? = nil
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Owner

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

@efremidze
Copy link
Copy Markdown
Owner

I'll try to include this asap

@bj97301
Copy link
Copy Markdown
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
Copy Markdown

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