1
\$\begingroup\$

I created an iOS app that recreates the "ball breaking" game where a paddle is used to deflect the ball to break blocks. The main purpose is to get familiar with UI concepts since I have primarily worked on lower layers. I also want to improve my software architecture skills developing a full end-to-end app (currently, I mainly focus on developing 3rd party SDKs).

The full source code is here. The parts of the code I want reviewed and would like feedback on are:

Architecture on how the game state evolves. The current logic, the ViewController when a CADisplayLink to a runloop and sets selector to updateDisplay: whose implementation is below.

As we can see, the ViewController calls a method in the model object (that is storing the game state). The method is updateModelAtTime:withCompletionBlock:.

The idea is that the model will update location of ball based on velocity and time since last firing. It also updates location of paddle based on touch events. It checks if new ball location has collisions with edge, block or paddle. If yes, it changes ball velocity appropriately. It also adds collided blocks to a brokenBlocks array. After doing this it executes the completion block given by the view controller. In the completion block, view controller checks if there are broken blocks. If yes, removes their views from the superView. It checks if number of blocks left is 0. If yes, it sends the game. If there are blocks left, it updates the location of the ball/paddle based on new location.

My question is whether this software architecture or are there any better ways to handle this game view/game state interaction.

The second piece code I want reviewed is the logic used to detect the collisions. Is the code the most optimum possible or are there possible improvements?

Code for question/code review 1

-(void) updateDisplay:(CADisplayLink *)sender
{
    //NSLog(@"Received display link at time %.2f",sender.timestamp);
    __weak ViewController *weakSelf = self;
    [self.myModel updateModelAtTime:sender.timestamp withCompletionBlock:^{

        ViewController *innerSelf = weakSelf;

        if([innerSelf.myModel didGameEnd])
        {
            innerSelf.gameCLock.paused = YES;
            for (BlockView *nextBlockToRemove in self.myModel.brokenBlocks)
            {
                [nextBlockToRemove removeFromSuperview];
            }
            [innerSelf.myModel.brokenBlocks removeAllObjects];
            [innerSelf.ballView removeFromSuperview];

            [innerSelf showAlertWithText:@"Congratulations:  You have broken all the blocks!"
                           andTitle:@"Game Over"
                    andCompletionHandler:^{
                        [innerSelf.myModel initializeGame];
                        [innerSelf.paddleView removeFromSuperview];
                        [innerSelf initializeGameView];
                        innerSelf.gameCLock.paused = NO;
                    }];
        }
        else
        {
            [innerSelf.ballView setFrame:self.myModel.ballRect];
            [innerSelf.paddleView setFrame:self.myModel.paddleRect];
            for (BlockView *nextBlockToRemove in self.myModel.brokenBlocks)
            {
                [nextBlockToRemove removeFromSuperview];
            }
            [innerSelf.myModel.brokenBlocks removeAllObjects];
        }
    }];
}

-(void)updateModelAtTime:(CFTimeInterval)timestamp
     withCompletionBlock:(dispatch_block_t)completionHandler;
{
    CGFloat newX = self.ballRect.origin.x, newY = self.ballRect.origin.y;
    if(self.lastTime)
    {
       newX += (self.ballVelocity.x*(timestamp-self.lastTime));
       newY += (self.ballVelocity.y*(timestamp-self.lastTime));
    }
    self.lastTime = timestamp;

    self.ballRect = CGRectMake(newX, newY,
                               self.ballRect.size.width, self.ballRect.size.height);
    [self adjustIfBallHitEdge];
    [self checkIfHitBlock];
    [self checkIfHitPaddle];
    if (completionHandler)
    {
        completionHandler();
    }
    //NSLog(@"New rect is origin: (%.2f, %.2f) size (%.2f, %.2f)",self.ballRect.origin.x, self.ballRect.origin.y, self.ballRect.size.width, self.ballRect.size.height);
}

Code for question/code review 2

-(void) checkIfHitBlock
{
    BOOL sideImpact = NO;

    for (NSMutableArray *nextCol in self.blocks)
    {
        CGRect blockRect = [[nextCol lastObject] frame];
        if ([nextCol count] && CGRectIntersectsRect(blockRect, self.ballRect))
        {
            if (self.ballRect.origin.y + self.ballRect.size.height < blockRect.origin.y + blockRect.size.height - self.sideDownOffset)
            {
                sideImpact = YES;
            }
            [self.brokenBlocks addObject:[nextCol lastObject]];
            [nextCol removeLastObject];
        }
    }
    if([self.brokenBlocks count])
    {
        if(sideImpact)
        {
            self.ballVelocity = CGPointMake(-1*self.ballVelocity.x, self.ballVelocity.y);
        }
        else
        {
            self.ballVelocity = CGPointMake(self.ballVelocity.x, -1*self.ballVelocity.y);
        }
    }
}

-(void) checkIfHitPaddle
{
    if (CGRectIntersectsRect(self.paddleRect, self.ballRect))
    {
        self.ballRect = CGRectMake(self.ballRect.origin.x, self.ballRect.origin.y - self.paddleRect.size.height,
                                   self.ballRect.size.width, self.ballRect.size.height);
        self.ballVelocity = CGPointMake(self.ballVelocity.x, -1*self.ballVelocity.y);
    }
}

-(void) adjustIfBallHitEdge
{
    if (self.ballRect.origin.x + self.ballRect.size.width >= self.screenWidth)
    {
        self.ballRect = CGRectMake(self.screenWidth-self.ballRect.size.width, self.ballRect.origin.y,
                                   self.ballRect.size.width, self.ballRect.size.height);
        self.ballVelocity = CGPointMake(-1*self.ballVelocity.x, self.ballVelocity.y);
    }
    else if(self.ballRect.origin.x <= 0)
    {
        self.ballRect = CGRectMake(0, self.ballRect.origin.y,
                                   self.ballRect.size.width, self.ballRect.size.height);
        self.ballVelocity = CGPointMake(-1*self.ballVelocity.x, self.ballVelocity.y);
    }
    else if (self.ballRect.origin.y + self.ballRect.size.width >= self.screenHeight)
    {
        self.ballRect = CGRectMake(self.screenWidth/2, self.screenHeight-BALL_START_OFFSET,
                                   self.ballRect.size.width, self.ballRect.size.height);
        self.ballVelocity = CGPointMake(self.ballVelocity.x, -1*self.ballVelocity.y);
    }
    else if(self.ballRect.origin.y <= TOP_OFFSET)
    {
        self.ballRect = CGRectMake(self.ballRect.origin.x, TOP_OFFSET,
                                   self.ballRect.size.width, self.ballRect.size.height);
        self.ballVelocity = CGPointMake(self.ballVelocity.x, -1*self.ballVelocity.y);
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Your second to last conditional uses self.ballRect.origin.y + self.ballRect.size.width and should likely use ballRect's height, not width. I understand that it's highly likely that the width is the height since it's a ball after-all, but you never know what happens down the road when you add "features" haha \$\endgroup\$ – Albert Renshaw Nov 21 '16 at 7:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.