2
\$\begingroup\$

I'm working on a game for iOS devices and this is the function we use to create server-loaded buildings. The server loading is in another function and everything works just fine, but I was wondering if this function could be prettier? This function runs every 60 seconds as we reload the whole base (with the buildings in) frequently, maybe I should reuse the buildings if nothing has changed server-side?

There's another function that just clears all the subviews in the UIView "sandbox" to reset the base upon reload.

-(void)loadBuilding:(BuildingButton *)building 
{    
    //building.backgroundColor = [UIColor blackColor];

    if(building.buildUpgrading > 0 && building.buildLevel == 1)  //Building is being built
    {
        HJManagedImageV *constructionImage = [[HJManagedImageV alloc] init];
        constructionImage.url = [NSURL URLWithString:[NSString stringWithFormat:@"http://www.mysite.com/img/base/construction/%@",building.buildConstrImg]];
        constructionImage.greyscale = greyscale;
        constructionImage.resize = 1;
        constructionImage.reloadSuperview = 1;
        constructionImage.animateSetImage = YES;
        constructionImage.delegate = building;
        //[constructionImage showLoadingWheel];
        [appDelegate.objManager manage:constructionImage];
        constructionImage.frame = CGRectMake(0, 0, constructionImage.image.size.width, constructionImage.image.size.height);

        [building addSubview:constructionImage];
        [building bringSubviewToFront:constructionImage];
        [building setNeedsDisplay];
        [constructionImage release];
    } else { //Building is normal!
        //---------------------------------------------------------------------------------------------------------
        //ANIMATED IMAGES IMPLEMENTS HERE -------------------------------------------------------------------------
        if([building.buildImage isEqualToString:@"oil_refinery_desert.gif"])  //Images that need animating, use ID instead?
        {
            UIImage *image1 = [UIImage imageNamed:@"oil_refinery_desert01.png"];
            UIImage *image2 = [UIImage imageNamed:@"oil_refinery_desert02.png"];
            UIImage *image3 = [UIImage imageNamed:@"oil_refinery_desert03.png"];
            UIImage *image4 = [UIImage imageNamed:@"oil_refinery_desert04.png"];
            UIImage *image5 = [UIImage imageNamed:@"oil_refinery_desert05.png"];
            //Make GREY!
            if (greyscale) {
                image1 = [self convertImageToGrayScale:image1];
                image2 = [self convertImageToGrayScale:image2];
                image3 = [self convertImageToGrayScale:image3];
                image4 = [self convertImageToGrayScale:image4];
                image5 = [self convertImageToGrayScale:image5];
            }
            NSArray *images = [[NSArray alloc] initWithObjects:image1, image2, image3, image4, image5, nil];
            UIImageView *animationSequence = [[UIImageView alloc] initWithFrame:CGRectMake(0, 0, image1.size.width, image1.size.height)];
            animationSequence.animationImages = images;
            [images release];
            [building addSubview:animationSequence];
            [building bringSubviewToFront:animationSequence];
            [building setNeedsDisplay];
            //[building setBackgroundColor:[UIColor redColor]];
            [building setFrame:CGRectMake(building.frame.origin.x, building.frame.origin.y, image1.size.width, image1.size.height)];
            animationSequence.animationDuration = 0.75;
            animationSequence.animationRepeatCount = 0;
            [animationSequence startAnimating];
            [animationSequence release];
        } else {
            //[building setImage:imageLoader.image forState:UIControlStateNormal];
            HJManagedImageV *imageLoader = [[HJManagedImageV alloc] initWithFrame:CGRectMake(0,0,0,0)];
            imageLoader.url = [NSURL URLWithString:[NSString stringWithFormat:@"http://www.mysite.com/img/base/%@",building.buildImage]];
            imageLoader.greyscale = greyscale;
            imageLoader.resize = 1;
            imageLoader.reloadSuperview = 1;
            imageLoader.animateSetImage = YES;
            imageLoader.delegate = building;
            //[imageLoader showLoadingWheel];
            [appDelegate.objManager manage:imageLoader];
            [building addSubview:imageLoader];
            [imageLoader release];
            //[building setBackgroundColor:[UIColor blackColor]];
        }

        //Is the building upgrading?
        if(building.buildUpgrading > 0) 
        {
            HJManagedImageV *constructionImage = [[HJManagedImageV alloc] init];
            constructionImage.url = [NSURL URLWithString:[NSString stringWithFormat:@"http://www.mysite.com/img/base/upgrade/%@",building.buildUpgradeImg]];
            constructionImage.greyscale = greyscale;
            constructionImage.resize = 1;
            constructionImage.reloadSuperview = 1;
            constructionImage.animateSetImage = YES;
            constructionImage.delegate = building;
            //[constructionImage showLoadingWheel];
            [appDelegate.objManager manage:constructionImage];
            constructionImage.frame = CGRectMake(0, 0, building.frame.size.width, building.frame.size.height);

            [building addSubview:constructionImage];
            [building bringSubviewToFront:constructionImage];
            [building setNeedsDisplay];
            [constructionImage release];
        }
    }

    if(building.buildID == 999) { //Road is disabled!
        building.userInteractionEnabled = FALSE;
    }

    NSLog(@"Loaded: %@", building);
    if(greyscale == 0) {
        [building addTarget:self action:@selector(playButtonSound) forControlEvents:UIControlEventTouchDown];
        [building addTarget:self action:@selector(loadWindow:) forControlEvents:UIControlEventTouchUpInside];
    }
    [sandbox addSubview:building];
}
\$\endgroup\$
3
\$\begingroup\$

You should rewrite convertImageToGrayScale: to grayScaled in a category on UIImage

 image1 = [self grayScaled];

in the following block you should add the images to array first, and then loop through it, to perform for(UIImage *image in images){image = [image grayScaled];}

 UIImage *image1 = [UIImage imageNamed:@"oil_refinery_desert01.png"];
 UIImage *image2 = [UIImage imageNamed:@"oil_refinery_desert02.png"];
 UIImage *image3 = [UIImage imageNamed:@"oil_refinery_desert03.png"];
 UIImage *image4 = [UIImage imageNamed:@"oil_refinery_desert04.png"];
 UIImage *image5 = [UIImage imageNamed:@"oil_refinery_desert05.png"];
        //Make GREY!
 if (greyscale) {
        image1 = [self convertImageToGrayScale:image1];
        image2 = [self convertImageToGrayScale:image2];
        image3 = [self convertImageToGrayScale:image3];
        image4 = [self convertImageToGrayScale:image4];
        image5 = [self convertImageToGrayScale:image5];
 }
 NSArray *images = [[NSArray alloc] initWithObjects:image1, image2, image3, image4, image5, nil];

becomes:

 UIImage *image1 = [UIImage imageNamed:@"oil_refinery_desert01.png"];
 UIImage *image2 = [UIImage imageNamed:@"oil_refinery_desert02.png"];
 UIImage *image3 = [UIImage imageNamed:@"oil_refinery_desert03.png"];
 UIImage *image4 = [UIImage imageNamed:@"oil_refinery_desert04.png"];
 UIImage *image5 = [UIImage imageNamed:@"oil_refinery_desert05.png"];

 NSArray *images = [[NSArray alloc] initWithObjects:image1, image2, image3, image4, image5, nil];

 if (greyscale) {
     for(UIImage *image in images){
         image= [image grayScaled];
     }
 }

can you confirm, that this is necessary:

 [building addSubview:animationSequence];
 [building bringSubviewToFront:animationSequence];
 [building setNeedsDisplay];

I think, [building addSubview:animationSequence]; should be sufficient.

BTW: IMHO Class names for Views should end with the word View, i.e. Building becomes BuildingView. Same for Controllers.

if(building.buildID == 999) { //Road is disabled! looks a bit suspicious to me. Adding a meta information to an object shouldn't be necessary in a OO environment. I assume a RoadModel to be a subclass of an BuildingModel (or FacilityModel,…), so the better check would be if([building isKindOfClass:[Road class]])

\$\endgroup\$
  • \$\begingroup\$ please see my addition \$\endgroup\$ – vikingosegundo Dec 30 '11 at 13:12
  • \$\begingroup\$ and another edit \$\endgroup\$ – vikingosegundo Dec 30 '11 at 13:22

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.