# Is this proper code for removing a CCSprite under certain conditions?

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];
id fadeTop = [CCSequence actionOne:[CCMoveTo actionWithDuration:1 position:circleTop.position] two:[CCScaleTo actionWithDuration:1 scale:0]];
}
if ([self isLocationBombable:left] && leftB == YES)
{
circleLeft.position = [self positionForTileCoord:left];
id fadeLeft = [CCSequence actionOne:[CCMoveTo actionWithDuration:1 position:circleLeft.position] two:[CCScaleTo actionWithDuration:1 scale:0]];
}
if ([self isLocationBombable:bottom] && bottomB == YES)
{
circleBottom.position = [self positionForTileCoord:bottom];
id fadeBottom = [CCSequence actionOne:[CCMoveTo actionWithDuration:1 position:circleBottom.position] two:[CCScaleTo actionWithDuration:1 scale:0]];
}
if ([self isLocationBombable:right] && rightB == YES)
{
circleRight.position = [self positionForTileCoord:right];
id fadeRight = [CCSequence actionOne:[CCMoveTo actionWithDuration:1 position:circleRight.position] two:[CCScaleTo actionWithDuration:1 scale:0]];
}
}

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?

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.