Game with tile map - Sprite-Kit

Question

I'm a novice programmer learning Objective-C in my spare time. I would greatly appreciate any help or advice with my code. I want to follow best practices whenever possible. I know that I am missing some foundational stuff with my code, so please point that out where you see it. Things like when to make variables instance variables versus properties, when to define things like (strong) for properties, when and where to instantiate variables, etc.

I'm building a simple game to practice and to learn Sprite-Kit. I've been working on it for a few days, and it does function mostly as I want it to. The game model is pure objective-c and generates a map of tiles as well as handles inserting tiles into the map. This is done with an NSDictionary with the keys being the coordinates of the tiles. That code is not included below, but I can post it if need be. My SKScene renders those tiles based on their type, and tells the model to insert new tiles where the screen is tapped. It also manages the touch controls.

The main problems are that the panning around the world and the pinching to zoom move much faster than I would like. I can't figure out how to slow them down, and I'm wondering if I am doing something wrong.

Please let me know if you need to see more code to review it. And please don't hesitate to point out any flaws that you see, both in style and in substance.

Here is the header:

//  TPGameSceneSO.h

#import <SpriteKit/SpriteKit.h>
#import "TPGame.h"

//set up for 60 frames per second
#define kMinTimeInterval (1.0f / 60.0f)

//seems to run better with the delegate enabled
@interface TPGameSceneSO : SKScene <UIGestureRecognizerDelegate>

@property TPGame *theGame;
@property SKSpriteNode *tileMapNode;
@property SKSpriteNode *movableSprites;
@property UIPinchGestureRecognizer *pinchRecognizer;
@property UIPanGestureRecognizer *panRecognizer;

@end

And now the implementation:

enter code here
//  TPGameSceneSO.m

#import "TPGameSceneSO.h"

#define TILE_SIZE CGSizeMake(128,128)
#define TILE_DIMENSION 128
static NSString * const kMovableNodeName = @"movable";

@interface TPGameSceneSO ()
@property (nonatomic) NSTimeInterval lastUpdateTimeInterval;
@end

@implementation TPGameSceneSO {
    CGFloat _tempScale;
    CGPoint *_tempPosition;
    BOOL _contentCreated;
    NSArray *_tilesFromAtlas;
    BOOL _tileHasBeenInserted;
    SKSpriteNode *_selectedNode;
    SKNode *_world;
    CGPoint _tempWorldLocation;
    BOOL _worldMovedForUpdate;
    CGPoint _activeTilePos;
    BOOL _activeTilePosChanged;
}

#pragma mark  -  Utility Methods
////////////The following are utility methods////////////////////////////////
//first two are for randomness
//not sure if i need it in every class that uses it
static inline CGFloat skRandf() {
    return rand() / (CGFloat) RAND_MAX;
}
static inline CGFloat skRand(CGFloat low, CGFloat high) {
    return skRandf() * (high - low) + low;
}
float degToRad(float degree) {
    return degree / 180.0f * M_PI;
}

#pragma mark  -  Initialization
/////////////////Initialization methods//////////////////////////
-(void)didMoveToView:(SKView *)view {
    if(!_contentCreated) {
        [self createSceneContents];
        _contentCreated = YES;
    }
}
-(void)createSceneContents{
    self.scaleMode = SKSceneScaleModeAspectFit;

    //Set up the gesture recognizers
    _panRecognizer = [[UIPanGestureRecognizer alloc]initWithTarget:self action:@selector(handlePanFrom:)];
    [[self view] addGestureRecognizer:_panRecognizer];
    _pinchRecognizer = [[UIPinchGestureRecognizer alloc]initWithTarget:self action:@selector(handlePinch:)];
    [[self view] addGestureRecognizer:_pinchRecognizer];

    //Set up the texture atlas
    [self buildTileTextureAtlas];

    //Set up the world, used for the camera
    _world = [[SKNode alloc] init];
    [_world setName:@"world"];
    [self addChild:_world];

    //Initialize the game object
    self.theGame = [[TPGame alloc]init];

    //Build the tile map
    [self.theGame startGame];

    //Set up the tile map used for rendering and render the initial map
   //self.tileMapNode = [[[SKSpriteNode alloc]init] initWithColor:[SKColor whiteColor] size:self.frame.size];
    self.tileMapNode = [[[SKSpriteNode alloc]init] initWithColor:[SKColor whiteColor] size:CGSizeMake(1024,1024)];
    [self.tileMapNode setName:@"background"];
    self.tileMapNode = [self renderTileMap:self.tileMapNode];
    [_world addChild:self.tileMapNode];

    //Set up the movable sprites
    _movableSprites = [[SKSpriteNode alloc] init];
    [self setUpMovableSprites];
    [_movableSprites setName:@"sprites"];
    [_world addChild:_movableSprites];
}
-(void) setUpMovableSprites {
    NSArray *imageNames = @[@"tile1", @"tile2", @"tile3", @"tile4"];
    for (int i = 0; i < [imageNames count]; i++) {
        SKTexture *temp = _tilesFromAtlas[i];
        SKSpriteNode *sprite = [SKSpriteNode spriteNodeWithTexture:temp];
        sprite.size = CGSizeMake(TILE_DIMENSION, TILE_DIMENSION);
        [sprite setName:kMovableNodeName];
        float offsetFraction = ((float)(i + 1)) / ([imageNames count] + 1);
        [sprite setPosition:CGPointMake(self.size.width * offsetFraction, self.size.height - self.size.height/3)];
        [_movableSprites addChild:sprite];
    }
}
-(void)buildTileTextureAtlas {
    int numFormats = 3;
    NSMutableArray *tileImages = [NSMutableArray array];
    SKTextureAtlas *tileAtlas = [SKTextureAtlas atlasNamed:@"tileimages"];
    int numImages = tileAtlas.textureNames.count;
    for (int i = 1; i <= numImages/numFormats; i++) {
        NSString *textureName = [NSString stringWithFormat:@"tile%d", i];
        SKTexture *temp = [tileAtlas textureNamed:textureName];
        [tileImages addObject:temp];
    }
    _tilesFromAtlas = tileImages;
}

#pragma mark - Update loop
/////////////Here are the updaters/////////////////////
- (void)update:(NSTimeInterval)currentTime {
    // Handle time delta.
    // If we drop below 60fps, we still want everything to move the same distance.
    CFTimeInterval timeSinceLast = currentTime - self.lastUpdateTimeInterval;
    self.lastUpdateTimeInterval = currentTime;
    if (timeSinceLast > 1) { // more than a second since last update
        timeSinceLast = kMinTimeInterval;
        self.lastUpdateTimeInterval = currentTime;

        //doing this here seems to slow down the pan and zoom rate
        _worldMovedForUpdate = YES;
    }
    [self updateWithTimeSinceLastUpdate:timeSinceLast];
}
- (void)updateWithTimeSinceLastUpdate:(NSTimeInterval)timeSinceLast {
    //will build tiles slowly and randomly out from the center
    //[self insertTileInRandomPlace];

    if  (_worldMovedForUpdate) {
        [self moveWorldView];
        [self clearActiveTilePosChanged];
    }
    if (_activeTilePosChanged){
        [self addTile];
    }
    if (_tileHasBeenInserted == YES){
        [_movableSprites removeFromParent];

        [self.tileMapNode removeFromParent];
        [self.tileMapNode removeAllChildren];
        self.tileMapNode = [self renderTileMap:self.tileMapNode];
        [_world addChild:self.tileMapNode];

        [_world addChild:_movableSprites];
    }
}

#pragma mark - Camera Movement
///////////////Methods for moving the world (camera)/////////////////////
-(void) moveWorldView {
    _world.position = _tempWorldLocation;
    [self clearWorldMoved];
}
- (void)clearWorldMoved {
    _worldMovedForUpdate = NO;
}

#pragma mark - Rendering Methods
/////////////////Rendering methods//////////////////
-(SKSpriteNode *) renderTileMap: (SKSpriteNode *)tileMapForRender {
    for (id key in self.theGame.gameTileMap.tileMapDict) {
        TPTile *tempTile = [self.theGame.gameTileMap.tileMapDict objectForKey:key];
        int tileXPos = tempTile.position.x*TILE_DIMENSION;
        int tileYPos = tempTile.position.y*TILE_DIMENSION;
        if (tempTile.revealed == YES) {
            SKSpriteNode *tile = [self renderTile:tempTile.terrainType];
            tile.position = CGPointMake(tileXPos,tileYPos);
            [tileMapForRender addChild:tile];
            [tile setName:@"tile"];
        }
    }
    _tileHasBeenInserted = NO;
    return tileMapForRender;
    }
    -(SKSpriteNode *)renderTile: (int)tileType {
        switch (tileType) {
        case 0:
        {
            SKSpriteNode *renderedTile = [SKSpriteNode spriteNodeWithColor:    [SKColor blackColor] size:TILE_SIZE];
        return renderedTile;
        }
        case 1:
        {
            SKTexture *temp = _tilesFromAtlas[0];
            SKSpriteNode *renderedTile = [SKSpriteNode spriteNodeWithTexture:temp size:TILE_SIZE];
            return renderedTile;
        }
        case 2:
        {
            SKTexture *temp = _tilesFromAtlas[1];
            SKSpriteNode *renderedTile = [SKSpriteNode spriteNodeWithTexture:temp size:TILE_SIZE];
            return renderedTile;
        }
        case 3:
        {
            SKTexture *temp = _tilesFromAtlas[2];
            SKSpriteNode *renderedTile = [SKSpriteNode spriteNodeWithTexture:temp size:TILE_SIZE];
            return renderedTile;
        }
        case 4:
        {
            SKTexture *temp = _tilesFromAtlas[3];
            SKSpriteNode *renderedTile = [SKSpriteNode spriteNodeWithTexture:temp size:TILE_SIZE];
            return renderedTile;
        }
        case 5:
        {
            SKTexture *temp = _tilesFromAtlas[4];
            SKSpriteNode *renderedTile = [SKSpriteNode spriteNodeWithTexture:temp size:TILE_SIZE];
            return renderedTile;
        }
        case 6:
        {
            SKTexture *temp = _tilesFromAtlas[5];
            SKSpriteNode *renderedTile = [SKSpriteNode spriteNodeWithTexture:temp size:TILE_SIZE];
            return renderedTile;
        }
        default:
        {
            SKSpriteNode *renderedTile = [SKSpriteNode spriteNodeWithColor: [SKColor grayColor] size:TILE_SIZE];
            return renderedTile;
        }
    }
}

#pragma mark - Interaction with other classes
//////////////Methods for inserting tiles into the model///////////////////
-(void) addTile {
    _tileHasBeenInserted = YES;
    //have to round these here before inserting them because just doing an int will drop the decimal
    CGFloat tileFloatX = _activeTilePos.x/TILE_DIMENSION;
    CGFloat tileFloatY = _activeTilePos.y/TILE_DIMENSION;
    int tilePosX = roundf(tileFloatX);
    int tilePosY = roundf(tileFloatY);

    CGPoint pointForInsertion = CGPointMake(tilePosX, tilePosY);
    [self.theGame.gameTileMap insertTileToTheMap:pointForInsertion];
    [self clearActiveTilePosChanged];
}
-(void) updateActiveTilePos:(CGPoint)location {
    _activeTilePos = location;
    _activeTilePosChanged = YES;
}
-(void)clearActiveTilePosChanged {
    _activeTilePosChanged = NO;
}
-(void) insertTileInRandomPlace {
    CGPoint tempPosition = CGPointMake(skRand(-5000, 5000), skRand(-5000, 5000));
    _activeTilePos = tempPosition;
    _activeTilePosChanged = YES;
}

#pragma mark - Touch controls
///////////////This is the touch engine///////////////////////////////
-(void)touchesEnded:(NSSet *)touches withEvent:(UIEvent *)event {
    for (UITouch *touch in touches) {
        UITouch *touch = [touches anyObject];
        if(touch.tapCount == 1) {
            CGPoint positionInScene = [touch locationInNode:self];
            [self selectNodeForTouch:positionInScene];
        }
    }
}
-(void) handlePanFrom:(UIGestureRecognizer *)recognizer {
    if (recognizer.state == UIGestureRecognizerStateChanged) {
        if (![[_selectedNode name] isEqualToString:kMovableNodeName]) {
            CGPoint translation = [_panRecognizer translationInView:recognizer.view];
            translation = CGPointMake(translation.x, -translation.y);
            [self panForTranslation:translation];
        } else if ([[_selectedNode name] isEqualToString:kMovableNodeName]) {
            //this is the code that moves the sprites around
            CGPoint translation = [_panRecognizer translationInView:recognizer.view];
            translation = CGPointMake(translation.x, -translation.y);
            CGPoint pos = [_selectedNode position];
            CGPoint newPos = CGPointMake(pos.x + translation.x, pos.y + translation.y);
            SKAction *moveTo = [SKAction moveTo:newPos duration:0.2];
            //[moveTo setTimingMode:SKActionTimingEaseOut];
            [_selectedNode runAction:moveTo];
       }
    }
 }
-(void)panForTranslation:(CGPoint)translation {
    CGPoint position = _world.position;
    CGPoint newPos = CGPointMake(position.x + translation.x, position.y + translation.y);

    //manual control of the world position (not spritekit movement)
    _tempWorldLocation = newPos;
    _worldMovedForUpdate = YES;

    //this would use spritekit to move the world instead of doing it in the update method
    //[_world removeAllActions];
    //SKAction *moveTo = [SKAction moveTo:newPos duration:0.1];
    //[moveTo setTimingMode:SKActionTimingEaseOut];
    //[_world runAction:moveTo];
}
-(void) handlePinch:(UIPinchGestureRecognizer *)sender {
    if (sender.state == UIGestureRecognizerStateBegan) {
        _tempScale = [sender scale];
    }
    if (sender.state == UIGestureRecognizerStateChanged) {
        //for some reason adding .1 slows the rate of movement
        //probably because most of the scale values being outputted are between -2 and 2
        CGFloat zoomOutAmount = (_tempScale / [sender scale]) + .1;
        CGFloat zoomInAmount = ([sender scale] / _tempScale) + .1;
        if([sender scale] < _tempScale) {
            self.size = CGSizeMake(self.size.width*zoomInAmount, self.size.height*zoomInAmount);
        } else if ([sender scale] > _tempScale) {
            self.size = CGSizeMake(self.size.width/zoomOutAmount, self.size.height/zoomOutAmount);
        }
        _tempWorldLocation = CGPointMake(0+self.frame.size.width/2, 0+self.frame.size.height/2);
        _worldMovedForUpdate = YES;
    }
}
-(void)selectNodeForTouch:(CGPoint)touchLocation {
    SKSpriteNode *touchedNode = (SKSpriteNode *)[self nodeAtPoint:touchLocation];

    //this inserts the tile at the location in world node
    CGPoint locationInWorld = [self.scene convertPoint:touchLocation toNode:_world];
    [self updateActiveTilePos:locationInWorld];

    //this is code for selecting the sprites
    if(![_selectedNode isEqual:touchedNode]) {
        [_selectedNode removeAllActions];
        [_selectedNode runAction:[SKAction rotateToAngle:0.0f duration:0.1]];

        _selectedNode = touchedNode;

        if([[touchedNode name] isEqualToString:kMovableNodeName]) {
            SKAction *sequence = [SKAction sequence:@[[SKAction rotateByAngle:degToRad(-4.0f) duration:0.1],
                                                  [SKAction rotateByAngle:0.0 duration: 0.1],
                                                  [SKAction rotateByAngle:degToRad(4.0f) duration:0.1]]];
            [_selectedNode runAction: [SKAction repeatActionForever:sequence]];
        }
    }
    }

@end

Answer 1

I must say that I'm not an expert in Objective-C. I have used it though so I will review what I can and hope that others will fill in for the rest.

First of all, to address your questions about variables and properties. Here's how I think about it:

• If you need a variable only in one method make it a local variable inside that method.
• If you need a variable within multiple methods in the same class, make it an instance variable.
• If you need a variable in many different classes, make it a property.

While you are doing this, try to remember to Tell, don't ask. What this means, simplified, is that you shouldn't do something like this: this.getSomeObject().getOtherObject().doSomething(); (sorry for the Java-syntax there, I hope you understand it). I think your current code handles this quite good, just remember to keep it that way :)

It is also important that you understand how Objective-C deals with, and how you should deal with, memory management. (Objective-C handles this a bit differently from other languages that I am more used to)

The utility methods that you are mentioning, if you are copying those methods to each class that is using them, then you are doing it wrong. Instead put them in one file and use #include for that file in the other files where you are going to need it.

Your indentation of the code is mostly good, but in the end of the renderTileMap method, things seem to go wrong. This seems to continue to the entire renderTile method which is indented one step too much.

Speaking of the renderTile method, it can be improved quite significantly since there's a strong pattern for cases 1 to 6. It's been a while since I wrote Objective-C code but I think this should work, and if not I hope you get the idea of how it should work:

-(SKSpriteNode *)renderTile: (int)tileType {
    if (tileType == 0) {
        SKSpriteNode *renderedTile = [SKSpriteNode spriteNodeWithColor:    [SKColor blackColor] size:TILE_SIZE];
        return renderedTile;
    }
    else if (tileType >= 1 && tileType <= 6) {
        SKTexture *temp = _tilesFromAtlas[tileType - 1];
        SKSpriteNode *renderedTile = [SKSpriteNode spriteNodeWithTexture:temp size:TILE_SIZE];
        return renderedTile;
    }
    else {
        SKSpriteNode *renderedTile = [SKSpriteNode spriteNodeWithColor: [SKColor grayColor] size:TILE_SIZE];
        return renderedTile;
    }
}

Generally I must say that your code looks very good. Very well done for being a beginner!

Answer 2

Instance Variables and Properties

Understanding exactly what a @property is is absolutely crucial to being a good Objective-C programmer. Here's my crash course.

An instance variable is just that. An instance variable. It's pretty straight forward. It's just a variable in that can be accessed by all the methods in your class. And if it's a public variable, it can be accessed outside the class as well.

A @property, however, is different. A @property doesn't even necessarily have an instance variable behind it. It does by default, but a @property is much more than just an instance variable. Moreover, you can have public AND private @property variables. Simon's comments on properties versus variables are inaccurate.

Consider

@interface Person : NSObject
@property (nonatomic, strong) NSString *name;
@end

What have we actually done? As it turns out, this is the equivalent of doing this:

@interface Person : NSObject {
    @private
    NSString *name;
}
- (void)setName:(NSString*)aName;
- (NSString*)name;
@end

@implementation Person
- (void)setName:(NSString*)aName {
    name = aName;
}
- (NSString*)name {
    return name;
}

So by default, a @property creates three things: an instance variable, a setter, and a getter.

But consider this case:

@interface Person : NSObject
@property NSString *firstName;
@property NSString *lastName;
@property (readonly) NSString *fullName;
@end

@implementation Person
- (NSString*)fullName {
    return [NSString stringWithFormat:"%@ %@", self.firstName, self.lastName];
}
@end

By declaring fullName as a readonly property, we've prevented the creation of a setter method. But in the @implementation, what we've also done is overridden the getter method. But something very important happened in this method. We made no reference to _fullName, which would be the instance variable that a @property called fullName would create. As such, we've also prevented the creation of an instance variable.

So, the other thing left is that a @property allows you to use the dot-syntax to access the setters and getters (though you don't have to use it).

Now then, when should you use an @property and when should you use an instance variable?

Well, it really doesn't matter too terribly much. If you need a variable to be visible outside the class, you can still use an instance variable and still give it a getter and setter. The only difference between writing a getter, setter, creating an instance variable and just using a @property? Dot-syntax, that's it.

And the cool thing is, you don't have to limit yourself to the traditional idea of a setter, getter, and instance variable. You can create dot-syntax methods any time you need a method that takes no arguments and returns a value.

Please let me know if you would like some more information about properties/instance variables.

---

Simon's answer does make a good recommendation about condensing your renderTile: method. Although, I'd do it in a different way. Personally, I like switch statements. You can keep the switch and still condense the code:

-(SKSpriteNode *)renderTile: (int)tileType {
    switch(tileType) {
        case 1:
            // do stuff
            break;
        case 2:
        case 3:
        case 4:
        case 5:
        case 6:
            // do stuff
            break;
        default:
            // do stuff
            break;
    }
}

BUT... if you're doing this, let's use an enum, shall we?

typedef NS_ENUM(unsigned short, MyTileTypeEnum) {
    BlackTile = 1,
    TileTextureA = 2,
    TileTextureB = 3,
    TileTextureC = 4,
    TileTextureD = 5,
    TileTextureE = 6
};

So now our switch can look like this:

switch(tileType) {
    case BlackTile:
        // do stuff
        break;
// ... etc

Of course, the names should be chosen for something that's appropriately descriptive in your case. The point here is we've made our code more self-documenting.

It's important for our code to be easy to read. Code is written for humans and no one likes writing comments, so let's make our code more readable by using good variable names as well as using enums.