HiveBrain v1.2.0
Get Started
← Back to all entries
patternswiftMinor

Sprite-Kit/Swift game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
swiftgamespritekit

Problem

I was writing a game in Swift and Sprite Kit and it is very simple. It works perfectly other than the fact that it gets a bit laggy sometimes. I'd like to use this experience as both a learning tool, and to get the 'lagginess' resolved. My code is relatively short, actually:

```
import SpriteKit
import UIKit
import Foundation

var lastImage: UIImage?
class GameScene: SKScene, SKPhysicsContactDelegate {

let obstacleCategory : UInt32 = 0x1 1){
var duration = action.duration
duration -= 0.2
largeCircle.removeAllActions()
action = SKAction.rotateByAngle(CGFloat(M_PI), duration: duration)
largeCircle.runAction(SKAction.repeatActionForever(action))
}

}

func decelerate(){
if(action.duration, withEvent event: UIEvent) {
/ Called when a touch begins /

for touch in (touches as! Set) {
let location = touch.locationInNode(self)
if(location.x1){
let wait = SKAction.waitForDuration(0.1)
let block = SKAction.runBlock{self.accelerate()}
let sequence = SKAction.sequence([wait,block])
self.runAction(SKAction.repeatActionForever(sequence), withKey: accel)
}
}else{
if(action.duration, withEvent event: UIEvent) {
self.removeActionForKey(accel)
self.removeActionForKey(decel)
}

var probability : UInt32 = 25
override func update(currentTime: CFTimeInterval) {
if(scoreLabel.text.toInt() >= 4){
rightSlowDownLabel.alpha -= 0.1
leftSpeedUpLabel.alpha -= 0.1
}
if(newGoToUpdateFunFun){
UIGraphicsBeginImageContext(controller.view.frame.size)

CGContextBeginPath(UIGraphicsGetCurrentContext())

self.view?.drawViewHierarchyInRect(CGRect(origin: CGPoint.zeroPoint, size: UIGraphicsGetImageFromCurrentImageContext().size), afterScreenUpdates: true)
lastImage = UIGraphicsGetImageFromCurrentImageContext()
newGoToUpdateFunFun = false
self.paused = tru

Solution

You've posted quite a lot of code, and as such, for now I'm going to focus simply on a big picture overview of your code (and its organization).

This is the first thing that stands out to me is that you've got property declarations interspersed through all of your method declarations. Move all of the properties to the top.

There's a lot of code in didMoveToView(), and some of it is duplicated:

leftSpeedUpLabel.text = "Touch the left half to accelerate"
leftSpeedUpLabel.position = CGPoint(x: (self.frame.midX - 30), y: (self.frame.midY*2/3))
leftSpeedUpLabel.fontColor = UIColor.whiteColor()
leftSpeedUpLabel.fontSize = 20
leftSpeedUpLabel.fontName = "Heiti SC"
self.addChild(leftSpeedUpLabel)
rightSlowDownLabel.text = "Touch the right half to decelerate"
rightSlowDownLabel.position = CGPoint(x: (self.frame.midX + 30), y: (self.frame.midY*5/3))
rightSlowDownLabel.fontColor = UIColor.whiteColor()
rightSlowDownLabel.fontSize = 20
rightSlowDownLabel.fontName = "Heiti SC"
self.addChild(rightSlowDownLabel)


Copy & paste isn't a design pattern. Any time you find yourself copy-pasting like this, it's time to create a method:

func setupLabel(#label: SKLabelNode, withText text: String, atPoint point: CGPoint) {
    label.text = text
    label.position = point
    label.fontColor = UIColor.whiteColor()
    label.fontSize = 20
    label.fontName = "Heiti SC"
    self.addChild(label)
}


And now we can simply do this:

self.setupLabel(
    label: leftSpeedUpLabel,
    withText:"Touch the left half to accelerate", 
    atPoint: CGPoint(x: (self.frame.midX - 30), y: (self.frame.midY*2/3))
)
self.setupLabel(
    label: rightSlowDownLabel,
    withText:"Touch the right half to decelerate", 
    atPoint: CGPoint(x: (self.frame.midX + 30), y: (self.frame.midY*5/3))
)


And ultimately, we should just refactor most of the code in didMoveToView() down into individual methods that didMoveToView() calls so that the method is simply fewer lines. There's no reason for an individual method to be so long.

We also have some duplication here:

func accelerate(){
    if(action.duration>1){
        var duration = action.duration
        duration -= 0.2
        largeCircle.removeAllActions()
        action = SKAction.rotateByAngle(CGFloat(M_PI), duration: duration)
        largeCircle.runAction(SKAction.repeatActionForever(action))
    }

}

func decelerate(){
    if(action.duration<3){
        var duration = action.duration
        duration += 0.2
        largeCircle.removeAllActions()
        action = SKAction.rotateByAngle(CGFloat(M_PI), duration: duration)
        largeCircle.runAction(SKAction.repeatActionForever(action))
    }

}


We can turn this into a single method.

func alterDuration(duration: NSTimeInterval) {
    var duration = action.duration + acceleration

    // ensure duration is between 1 and 3, inclusive
    duration = max(min(duration,3),1)

    // be careful of == with floating point comparisons:
    if abs(duration - action.duration) > 0.0001 {
        // if we actually changed duration
        largeCircle.removeAllActions()
        action = SKAction.rotateByAngle(CGFloat(M_PI), duration: duration)
        largeCircle.runAction(SKAction.repeatActionForever(action))        
    }
}


And now we can keep the accelerate() and decelerate() methods if we want:

func accelerate() {
    alterDuration(-0.2)
}

func decelerate() {
    alterDuration(0.2)
}


Beyond this, you have a lot of magic numbers and magic strings and other commonly used constants that should certainly be cleaned up.

Code Snippets

leftSpeedUpLabel.text = "Touch the left half to accelerate"
leftSpeedUpLabel.position = CGPoint(x: (self.frame.midX - 30), y: (self.frame.midY*2/3))
leftSpeedUpLabel.fontColor = UIColor.whiteColor()
leftSpeedUpLabel.fontSize = 20
leftSpeedUpLabel.fontName = "Heiti SC"
self.addChild(leftSpeedUpLabel)
rightSlowDownLabel.text = "Touch the right half to decelerate"
rightSlowDownLabel.position = CGPoint(x: (self.frame.midX + 30), y: (self.frame.midY*5/3))
rightSlowDownLabel.fontColor = UIColor.whiteColor()
rightSlowDownLabel.fontSize = 20
rightSlowDownLabel.fontName = "Heiti SC"
self.addChild(rightSlowDownLabel)
func setupLabel(#label: SKLabelNode, withText text: String, atPoint point: CGPoint) {
    label.text = text
    label.position = point
    label.fontColor = UIColor.whiteColor()
    label.fontSize = 20
    label.fontName = "Heiti SC"
    self.addChild(label)
}
self.setupLabel(
    label: leftSpeedUpLabel,
    withText:"Touch the left half to accelerate", 
    atPoint: CGPoint(x: (self.frame.midX - 30), y: (self.frame.midY*2/3))
)
self.setupLabel(
    label: rightSlowDownLabel,
    withText:"Touch the right half to decelerate", 
    atPoint: CGPoint(x: (self.frame.midX + 30), y: (self.frame.midY*5/3))
)
func accelerate(){
    if(action.duration>1){
        var duration = action.duration
        duration -= 0.2
        largeCircle.removeAllActions()
        action = SKAction.rotateByAngle(CGFloat(M_PI), duration: duration)
        largeCircle.runAction(SKAction.repeatActionForever(action))
    }

}

func decelerate(){
    if(action.duration<3){
        var duration = action.duration
        duration += 0.2
        largeCircle.removeAllActions()
        action = SKAction.rotateByAngle(CGFloat(M_PI), duration: duration)
        largeCircle.runAction(SKAction.repeatActionForever(action))
    }

}
func alterDuration(duration: NSTimeInterval) {
    var duration = action.duration + acceleration

    // ensure duration is between 1 and 3, inclusive
    duration = max(min(duration,3),1)

    // be careful of == with floating point comparisons:
    if abs(duration - action.duration) > 0.0001 {
        // if we actually changed duration
        largeCircle.removeAllActions()
        action = SKAction.rotateByAngle(CGFloat(M_PI), duration: duration)
        largeCircle.runAction(SKAction.repeatActionForever(action))        
    }
}

Context

StackExchange Code Review Q#94937, answer score: 9

Revisions (0)

No revisions yet.