1
\$\begingroup\$

This is a "answer" to my own question over at stackoverflow, head over there to see what I am trying to accomplish. https://stackoverflow.com/questions/16744109/deleting-the-proper-ccsprite-from-an-array-in-a-schedule .

I ended up doing a rather interesting approach, and I think this might work.

  -(void)explosionFromPoint:(CGPoint)explosionPoint withSprite:(CCSprite*)sprite;
{
    //int 
    explosionLenght += 1;
    if (explosionLenght >= 7) //Just for testing purposes, don't have a way to increase it naturally. 
    {
        explosionLenght = 1;
    }

    BOOL topB = YES;
    BOOL leftB = YES;
    BOOL bottomB = YES;
    BOOL rightB = YES;

    int bombX =    (explosionPoint.x + 1);
    int bombY =    (explosionPoint.y + 1);
    int bombNegX = (explosionPoint.x - 1);
    int bombNegY = (explosionPoint.y - 1);

    CGPoint top = ccp(explosionPoint.x, bombY);
    CGPoint left = ccp(bombNegX, explosionPoint.y);
    CGPoint bottom = ccp(explosionPoint.x, bombNegY);
    CGPoint right = ccp(bombX, explosionPoint.y);

    if (![self isLocationBombable:top])
    {topB = NO;}
    if (![self isLocationBombable:left])
    {leftB = NO;}
    if (![self isLocationBombable:bottom])
    {bottomB = NO;}
    if (![self isLocationBombable:right])
    {rightB = NO;}

    for (int i = 0; i <= explosionLenght; i++) {

        int bombX =    (explosionPoint.x + i);
        int bombY =    (explosionPoint.y + i);
        int bombNegX = (explosionPoint.x - i);
        int bombNegY = (explosionPoint.y - i);

        CGPoint top = ccp(explosionPoint.x, bombY);
        CGPoint left = ccp(bombNegX, explosionPoint.y);
        CGPoint bottom = ccp(explosionPoint.x, bombNegY);
        CGPoint right = ccp(bombX, explosionPoint.y);

        CCSprite *circleTop    = [[CCSprite alloc]initWithFile:@"Circle.png"];
        CCSprite *circleLeft   = [[CCSprite alloc]initWithFile:@"Circle.png"];
        CCSprite *circleBottom = [[CCSprite alloc]initWithFile:@"Circle.png"];
        CCSprite *circleRight  = [[CCSprite alloc]initWithFile:@"Circle.png"];

        if ([self isLocationBombable:top] && topB == YES)
        {
            circleTop.position = [self positionForTileCoord:top];
            [self addChild:circleTop];
            id fadeTop = [CCSequence actionOne:[CCMoveTo actionWithDuration:1 position:circleTop.position] two:[CCScaleTo actionWithDuration:1 scale:0]];
            [circleTop runAction:fadeTop];
        }
        if ([self isLocationBombable:left] && leftB == YES)
        {
            circleLeft.position = [self positionForTileCoord:left];
            [self addChild:circleLeft];
            id fadeLeft = [CCSequence actionOne:[CCMoveTo actionWithDuration:1 position:circleLeft.position] two:[CCScaleTo actionWithDuration:1 scale:0]];
            [circleLeft runAction:fadeLeft];
        }
        if ([self isLocationBombable:bottom] && bottomB == YES)
        {
            circleBottom.position = [self positionForTileCoord:bottom];
            [self addChild:circleBottom];
            id fadeBottom = [CCSequence actionOne:[CCMoveTo actionWithDuration:1 position:circleBottom.position] two:[CCScaleTo actionWithDuration:1 scale:0]];
            [circleBottom runAction:fadeBottom];
        }
        if ([self isLocationBombable:right] && rightB == YES)
        {
            circleRight.position = [self positionForTileCoord:right];
            [self addChild:circleRight];
            id fadeRight = [CCSequence actionOne:[CCMoveTo actionWithDuration:1 position:circleRight.position] two:[CCScaleTo actionWithDuration:1 scale:0]];
            [circleRight runAction:fadeRight];
        }
    }

    [currentBombs addObject:sprite];

    NSLog(@"Explosion done, call checkdamage");

    [self schedule:@selector(checkDamageForBomb)];
    [self performSelector:@selector(removeSprite:) withObject:sprite afterDelay:3];
}

-(void)removeSprite:(CCSprite *)sprite{

    if([currentBombs containsObject:sprite])
    {
        NSLog(@"Found sprite in array, TERMINATHOORRR");
        [currentBombs removeObject:sprite];
    }
    else {
        NSLog(@"Didn't find the object in array, didn't delete!");
        [self stopCheckDamage];
    }

}

-(void)stopCheckDamage{

    [self unschedule:@selector(checkDamageForBomb)];

}

-(void)checkDamageForBomb{
    for (CCSprite* bomb in currentBombs) 
    {
        CGPoint bombPos = [self tileCoordForPosition:bomb.position];

        for (int i = 0; i <= explosionLenght; i++) {

            CGPoint playerPos = [self tileCoordForPosition:_cat.position];

            int bombX =    (bombPos.x + i);
            int bombY =    (bombPos.y + i);
            int bombNegX = (bombPos.x - i);
            int bombNegY = (bombPos.y - i);

            CGPoint centre = bombPos;
            CGPoint top = ccp(centre.x, bombY);
            CGPoint left = ccp(bombNegX, centre.y);
            CGPoint bottom = ccp(centre.x, bombNegY);
            CGPoint right = ccp(bombX, centre.y);

            //pastebin.com/biuQBfnv

            if (CGPointEqualToPoint(top, playerPos) || CGPointEqualToPoint(left, playerPos) || CGPointEqualToPoint(bottom, playerPos) || CGPointEqualToPoint(right, playerPos))
            {
                NSLog(@"Player hit");
                [currentBombs removeObject:bomb];
                break;
            }
        }
    }
}

Is this an efficient way to do such a thing?

\$\endgroup\$
3
\$\begingroup\$

I don't know if this is the kind of answer you're looking for, but as this is code review, I'm going to at least review some aspects of your code.

First thing I notice is explosionLenght. It's misspelled. You transposed the last two characters. It should be explosionLength.


Additionally, the following bits of code can be combined and refactored. Instead of:

// stuff

BOOL topB = YES;
BOOL leftB = YES;
BOOL bottomB = YES;
BOOL rightB = YES;

// stuff

if (![self isLocationBombable:top])
{topB = NO;}
if (![self isLocationBombable:left])
{leftB = NO;}
if (![self isLocationBombable:bottom])
{bottomB = NO;}
if (![self isLocationBombable:right])
{rightB = NO;}

// stuff

You can instead, just write this:

BOOL topB = [self isLocationBombable:top];
BOOL leftB = [self isLocationBombable:left];
BOOL bottomB = [self isLocationBombable:bottom];
BOOL rightB = [self isLocationBombable:right];

The following section is problematic to me:

CCSprite *circleTop    = [[CCSprite alloc]initWithFile:@"Circle.png"];
CCSprite *circleLeft   = [[CCSprite alloc]initWithFile:@"Circle.png"];
CCSprite *circleBottom = [[CCSprite alloc]initWithFile:@"Circle.png"];
CCSprite *circleRight  = [[CCSprite alloc]initWithFile:@"Circle.png"];

Primarily because it's contained within a loop... you're accessing the same file 4 times to initialize a variable, which is already not helpful. And then you do this once per iteration through the loop. I don't know a whole lot about Sprites and don't necessarily understand everything that's going on here, but... I know that you don't need to be doing this.

Grab the data from the file once and outside the loop.

CCSprite *circleTop = [[CCSprite alloc] initWithFile:@"Circle.png"];

Again, I'm not super familiar with CCSprite. But from here, you can either... use this same variable instead of using 4 variables.

Or if you really want 4 variables, you can do this:

CCSpring *circleLeft = circleTop;

(and do that a few more times)

This assigns each variable as a pointer to the same memory location.

But if you truly do need different memory locations, then after you grab from file once, the best method for initializing the others will be doing this:

CCSprite *circleLeft = [circleTop copy];

This will copy, bit for bit, the values in the memory location that circleTop points to into a new memory address and make circleLeft a pointer to that location, and it's certainly more efficient then loading from file 4 times.

| improve this answer | |
\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.