7
\$\begingroup\$

I had the idea the other day to implement an artificial intelligence that could play my strategy game. This could be useful in a variety of different ways. For example, the player could "lose contact" with one of the worlds in the game, and the AI could take over in that event. Or perhaps the player could elect a worker as governor to lead one of the worlds. Or maybe there could be opposing players that are artificial intelligence working on their own worlds competing with the player.

I've devised just such an artificial intelligence, and after watching it play my game for a little while, I think it's actually doing pretty well! I know that there are many possible problems with the code, so I hope this one will be a good one for code review.

I have tried to reduce duplicate code as much as possible, and I think I have succeeded. Where I am not happy is with the extensibility and changeability of the code. For example, at the moment the logic is hard coded to match the rules of the game, and so if major aspects of those rules change the AI will need to be changed as well. I tried hard to think of a way to better abstract this away, but it seemed like I had to pass on a large amount of information for something like that to work, and some of the logic contained in this class would have to be repeated in those classes anyway.

The other thing I am not happy about is that I have to pass the Tower itself to the AI in order for it to choose the proper move. I could package up the list of floors and the list of workers, but I also need some job validation that is contained inside the Tower object. Is this a major problem in this case? After all, for an AI to make a good decision it is always going to need a lot of information. Maybe I should have a subclass of the Tower that I instantiate and pass to the AI? But then I would need to have the Game configure this properly before passing it to the AI, and copy the values from the real tower to this mock tower object anyway.

The code is comprised of a GamePlayingAI and the GameAIJob classes. The GameAIJob has the bits of information relevant to the Game to actually send the real job to a tower. I am a bit concerned that this is a data only object, but I can't think of a better way to pass those values along.

DTGamePlayingAI.h

#import <Foundation/Foundation.h>
#import "DTTower.h"
#import "DTGamePlayingAIState.h"
#import "DTGameAIJob.h"

@interface DTGamePlayingAI : NSObject <NSCoding>

-(void) activate;

-(void) update;

@property GamePlayingAIState state;

@property DTGameAIJob *moveToPerform;

@property DTTower *tower;

@property int commonResources;
@property int rareResources;
@property int foodResources;

@end

DTGamePlayingAI.m

#import "DTGamePlayingAI.h"
#import "DTTowerFloor.h"
#import "DTJobCost.h"
#import "DTGamePlayingAIJobType.h"
#import "DTJobType.h"
#import "DTRoomType.h"

/*
    AI starts in NoMove state, which counts down until it reaches ReadyForInput
    The game then inserts the tower data and sets the state to received input
    On the next tick, the AI will choose a general needs case
    This will either immediately choose a job, or will select another case
    If another case, on the next tick the AI will select a job
    Once a job is chosen, state is changed to MoveDecided and the Game collects the move
    If the state ends up back in NoMove, the countdown starts over 
    This allows for time to pass between AI move selections
*/

static const int kMinimumResources = 200;
static const int kMinimumDwarves = 6;
static const int kMinimumFood = 325;
static const int kAgeForOld = 165;

@implementation DTGamePlayingAI {
    NSMutableArray *_revealedFloors;

    DTCountdown *_countdownBetweenJobs;
    int _countdownBetweenJobsAmount;
}

#pragma mark - Initialization
-(id) init {
    self = [super init];
    if (self) {
        _state = GamePlayingAIStateNotStarted;
        _revealedFloors = [[NSMutableArray alloc]init];

        //the reason this is a property and not a constant is because it could be
        //optionally lowered to make the AI make decisions faster or vice versa
        _countdownBetweenJobsAmount = 50;
    }
    return self;
}

#pragma mark - Activation
-(void) activate {
    _countdownBetweenJobs = [[DTCountdown alloc]initWithCount:_countdownBetweenJobsAmount];
    [_countdownBetweenJobs startCountdown];
    self.state = GamePlayingAIStateNoMove;
}

#pragma mark - Update Loop
-(void) update {
    switch (self.state) {

        //the countdown only counts down in this case
        case GamePlayingAIStateNoMove:
            [self updateCountdown];
            break;

        //immediately after the game loads with the tower data
        case GamePlayingAIStateReceivedInput:
            [self calculateBestMove];
            break;

        //big picture cases that sometimes cause supporting buildings to be built
        case GamePlayingAIStateNeedMoreDwarves:
            [self addMoveToGetDwarves];
            break;
        case GamePlayingAIStateNeedMoreFood:
            [self addMoveToGetFood];
            break;
        case GamePlayingAIStateNeedMoreResources:
            [self changeStateToGetMoreResources];
            break;
        case GamePlayingAIStateExpandTower:
            [self changeStateToExpandTower];
            break;

        //these are supporting buildings
        //arranged in the order they must be built underground
        case GamePlayingAIStatePlaceLadders:
            [self addMoveToBuildLadder];
            break;
        case GamePlayingAIStateMineFloors:
            [self addMoveToMineFloor];
            break;
        case GamePlayingAIStateBuildWalls:
            [self addMoveToBuildWalls];
            break;
        case GamePlayingAIStateBuildBottom:
            [self addMoveToBuildBottom];
            break;
        case GamePlayingAIStateBuildRoom:
            [self addMoveToBuildRoom];
            break;

        //activated by resource need case
        case GamePlayingAIStateHaulItems:
            [self addMoveToHaulItems];
            break;

        default:
            break;
    }
}

#pragma mark - Countdown Update
-(void) updateCountdown {
    [_countdownBetweenJobs update];
    if (_countdownBetweenJobs.state == CountdownFinished) {
        self.tower = nil;
        self.state = GamePlayingAIStateReadyForInput;
        [_countdownBetweenJobs restartCountdown];
    }
}

#pragma mark - Move Update
-(void) calculateBestMove {

    //this is the method that will need to be changed if the game changes significantly in any way

    [self calculateRevealedFloors];

    //first check if more resources are needed
    if ([self needMoreCommonResources:self.commonResources]) {
        self.state = GamePlayingAIStateNeedMoreResources;

    //then check if more dwarves are needed
    } else if ([self needMoreDwarves]) {
        self.state = GamePlayingAIStateNeedMoreDwarves;

    //then check if more food is needed
    } else if ([self needMoreFood:self.foodResources]) {
        self.state = GamePlayingAIStateNeedMoreFood;

    //if no needed jobs are found, set to this state
    } else {
        self.state = GamePlayingAIStateExpandTower;
    }

    //failsafe to deactivate working rooms once they are not needed
    //this will override a previously chosen job
    if (![self needMoreDwarves]) {
        if ([self floorsWithRunningRoomUpgradesOfType:RoomTypeBirther].count > 0) {
            [self endBirtherJob];
        }
    } else if (![self needMoreFood:self.foodResources]) {
        if ([self floorsWithRunningRoomUpgradesOfType:RoomTypeFarm].count > 0) {
            [self endFarmJob];
        }
    }
}
-(void) calculateRevealedFloors {
    [_revealedFloors removeAllObjects];
    for (id key in self.tower.towerDict) {
        DTTowerFloor *floor = [self.tower.towerDict objectForKey:key];
        if (floor.isRevealed) {
            [_revealedFloors addObject:floor];
        }
    }
}

#pragma mark - Needs Checking
-(BOOL) needMoreDwarves {
    //count up the number of dwarves under a certain age
    NSMutableArray *youngEnoughDwarves = [[NSMutableArray alloc]init];
    for (DTDwarf *dwarf in self.tower.dwarfListForRender) {
        if (dwarf.age < kAgeForOld) {
            [youngEnoughDwarves addObject:dwarf];
        }
    }

    if (youngEnoughDwarves.count < kMinimumDwarves) {
        //make sure that no birther rooms are already active
        if ([self floorsWithRunningRoomUpgradesOfType:RoomTypeBirther].count > 0) {
            return NO;
        }
        return YES;
    }
    return  NO;
}
-(BOOL) needMoreFood:(int)currentFood {
    if (currentFood < kMinimumFood) {
        //allows for up to three farms to be activated
        int maxActiveFarms = 2; //change this to a method that increases for the number of dwarves or revealed floors
        if ([self floorsWithRunningRoomUpgradesOfType:RoomTypeFarm].count > maxActiveFarms) {
            return NO;
        }
        return YES;
    }
    return NO;
}
-(BOOL) needMoreCommonResources:(int)commonResources {
    return commonResources < kMinimumResources;
}

#pragma mark - State Changing Moves
-(void) changeStateToGetMoreResources {
    //haul items if available, if not dig out floors if available, if not build new ladders to reveal
    if ([self floorsNeedingHauling].count > 0) {
        self.state = GamePlayingAIStateHaulItems;
    } else if ([self floorsForDigging].count > 0) {
        self.state = GamePlayingAIStateMineFloors;
    } else if ([self floorsWithoutLadders].count > 0) {
        self.state = GamePlayingAIStatePlaceLadders;
    } else {
        self.state = GamePlayingAIStateNoMove;
    }
}
-(void) changeStateToExpandTower {
    //either build a room, or a bottom, or walls, or mine, or place a ladder
    if ([self floorsWithValidPermissionsForJob:RoomBuildJob].count > 0) {
        self.state = GamePlayingAIStateBuildRoom;
    } else if ([self floorsWithValidPermissionsForJob:BottomBuildJob].count > 0) {
        self.state = GamePlayingAIStateBuildBottom;
    } else if ([self floorsWithValidPermissionsForJob:WallBuildJob].count > 0) {
        self.state = GamePlayingAIStateBuildWalls;
    } else if ([self floorsWithValidPermissionsForJob:JobTypeMining].count > 0) {
        self.state = GamePlayingAIStateMineFloors;
    } else if ([self floorsWithValidPermissionsForJob:LadderJob].count > 0) {
        self.state = GamePlayingAIStatePlaceLadders;
    } else {
        self.state = GamePlayingAIStateNoMove;
    }
}

#pragma mark - Specific Job Moves
-(void) addMoveToGetDwarves {
    //if there are birthers around that are inactive, start one
    NSMutableArray *floorsWithInactiveBirthers = [self floorsWithInactiveRoomUpgrades:RoomTypeBirther];
    if (floorsWithInactiveBirthers.count > 0) {
        DTTowerFloor *floor = [floorsWithInactiveBirthers firstObject];
        _moveToPerform = [[DTGameAIJob alloc]initWithFloor:floor.floorNumber AIJobToDo:GamePlayingAIJobTypeFlipRoomUpgradeSwitch jobToDo:0 roomType:0];

    //otherwise if there are rooms available to upgrade, upgrade one to birther
    } else if ([self floorsWithRooms].count > 0) {
        DTTowerFloor *floor = [[self floorsWithRooms] firstObject];
        _moveToPerform = [[DTGameAIJob alloc]initWithFloor:floor.floorNumber AIJobToDo:GamePlayingAIJobTypeRoomUpgrade jobToDo:0 roomType:RoomTypeBirther];

    //otherwise expand the tower
    } else {
        //eventually this will not always happen
        self.state = GamePlayingAIStateExpandTower;
    }

    [self finalizeStateForConcreteMove];
}
-(void) addMoveToGetFood {
    //if there are inactive farms, start one
    NSMutableArray *floorsWithInactiveFarms = [self floorsWithInactiveRoomUpgrades:RoomTypeFarm];
    if (floorsWithInactiveFarms.count > 0) {
        DTTowerFloor *floor = [floorsWithInactiveFarms firstObject];
        _moveToPerform = [[DTGameAIJob alloc]initWithFloor:floor.floorNumber AIJobToDo:GamePlayingAIJobTypeFlipRoomUpgradeSwitch jobToDo:0 roomType:0];

    //otherwise if one can be built, build one
    } else if ([self floorsWithRooms].count > 0) {
        DTTowerFloor *floor = [[self floorsWithRooms] firstObject];
        _moveToPerform = [[DTGameAIJob alloc]initWithFloor:floor.floorNumber AIJobToDo:GamePlayingAIJobTypeRoomUpgrade jobToDo:0 roomType:RoomTypeFarm];

    //otherwise expand the tower
    } else {
        //eventually this will not always happen
        self.state = GamePlayingAIStateExpandTower;
    }

    [self finalizeStateForConcreteMove];
}
-(void) addMoveToMineFloor {
    NSMutableArray *floorsNeedingMining = [self floorsForDigging];
    if (floorsNeedingMining.count > 0) {
        DTTowerFloor *floor = [floorsNeedingMining firstObject]; //add method to pick the closest to 0 or closest to a stockpile
        _moveToPerform = [[DTGameAIJob alloc]initWithFloor:floor.floorNumber AIJobToDo:GamePlayingAIJobTypeBasicJob jobToDo:JobTypeMining roomType:0];
    }

    [self finalizeStateForConcreteMove];
}
-(void) addMoveToBuildLadder {
    NSMutableArray *floorsNeedingLadders = [self floorsWithoutLadders];
    if (floorsNeedingLadders.count > 0) {
        DTTowerFloor *floor = [floorsNeedingLadders firstObject]; //add method to pick the deepest floor
        _moveToPerform = [[DTGameAIJob alloc]initWithFloor:floor.floorNumber AIJobToDo:GamePlayingAIJobTypeBasicJob jobToDo:LadderJob roomType:0];
    }

    [self finalizeStateForConcreteMove];
}
-(void) addMoveToHaulItems {
    NSMutableArray *floorsNeedingHauling = [self floorsNeedingHauling];
    if (floorsNeedingHauling.count > 0) {
        DTTowerFloor *floor = [floorsNeedingHauling firstObject]; //add method to choose closest to zero, gateway, or stockpile
        _moveToPerform = [[DTGameAIJob alloc]initWithFloor:floor.floorNumber AIJobToDo:GamePlayingAIJobTypeBasicJob jobToDo:JobTypeHaulItem roomType:0];
    }

    [self finalizeStateForConcreteMove];
}
-(void) addMoveToBuildRoom {
    NSMutableArray *floorsThatCanBuildRooms = [self floorsWithValidPermissionsForJob:RoomBuildJob];
    if (floorsThatCanBuildRooms.count > 0) {
        DTTowerFloor *floor = [floorsThatCanBuildRooms firstObject];
        _moveToPerform = [[DTGameAIJob alloc]initWithFloor:floor.floorNumber AIJobToDo:GamePlayingAIJobTypeBasicJob jobToDo:RoomBuildJob roomType:0];
    }

    [self finalizeStateForConcreteMove];
}
-(void) addMoveToBuildBottom {
    NSMutableArray *floorsThatCanBuildBottoms = [self floorsWithValidPermissionsForJob:BottomBuildJob];
    if (floorsThatCanBuildBottoms.count > 0) {
        DTTowerFloor *floor = [floorsThatCanBuildBottoms firstObject];
        _moveToPerform = [[DTGameAIJob alloc]initWithFloor:floor.floorNumber AIJobToDo:GamePlayingAIJobTypeBasicJob jobToDo:BottomBuildJob roomType:0];
    }

    [self finalizeStateForConcreteMove];
}
-(void) addMoveToBuildWalls {
    NSMutableArray *floorsThatCanBuildWalls = [self floorsWithValidPermissionsForJob:WallBuildJob];
    if (floorsThatCanBuildWalls.count > 0) {
        DTTowerFloor *floor = [floorsThatCanBuildWalls firstObject];
        _moveToPerform = [[DTGameAIJob alloc]initWithFloor:floor.floorNumber AIJobToDo:GamePlayingAIJobTypeBasicJob jobToDo:WallBuildJob roomType:0];
    }

    [self finalizeStateForConcreteMove];
}

#pragma mark - Failsafe Room Deactivation
-(void) endBirtherJob {
    NSMutableArray *floorsWithRunningBirthers = [self floorsWithRunningRoomUpgradesOfType:RoomTypeBirther];
    if (floorsWithRunningBirthers.count > 0) {
        DTTowerFloor *floor = [floorsWithRunningBirthers firstObject];
        _moveToPerform = [[DTGameAIJob alloc]initWithFloor:floor.floorNumber AIJobToDo:GamePlayingAIJobTypeFlipRoomUpgradeSwitch jobToDo:0 roomType:0];
    }

    [self finalizeStateForConcreteMove];
}
-(void) endFarmJob {
    NSMutableArray *floorsWithRunningFarms = [self floorsWithRunningRoomUpgradesOfType:RoomTypeFarm];
    if (floorsWithRunningFarms.count > 0) {
        DTTowerFloor *floor = [floorsWithRunningFarms firstObject];
        _moveToPerform = [[DTGameAIJob alloc]initWithFloor:floor.floorNumber AIJobToDo:GamePlayingAIJobTypeFlipRoomUpgradeSwitch jobToDo:0 roomType:0];
    }

    [self finalizeStateForConcreteMove];
}

#pragma mark - Finalize State
-(void) finalizeStateForConcreteMove {
    //if a concrete move was not decided, or not expanding, go to nomove
    if (_moveToPerform) {
        self.state = GamePlayingAIStateMoveDecided;
    } else if (self.state != GamePlayingAIStateExpandTower) {
        self.state = GamePlayingAIStateNoMove;
    }
}

#pragma mark - Complex Floor Collections
-(NSMutableArray *) floorsWithInactiveRoomUpgrades:(RoomType)roomType {
    NSMutableArray *floorsWithInactiveRoomUpgrades = [[NSMutableArray alloc]init];
    for (DTTowerFloor *floor in _revealedFloors) {
        if ((floor.floorBuildState & FloorHasRoomUpgrade) && floor.room.roomType == roomType) {
            if (![floor isRoomUpgradeJobCreationCountdownRunning]) {
                [floorsWithInactiveRoomUpgrades addObject:floor];
            }
        }
    }
    return floorsWithInactiveRoomUpgrades;
}
-(NSMutableArray *) floorsWithRunningRoomUpgradesOfType:(RoomType)roomType {
    NSMutableArray *floorsWithRunningRoomUpgrades = [[NSMutableArray alloc]init];
    for (DTTowerFloor *floor in _revealedFloors) {
        if (floor.room.roomType == roomType) {
            if ([floor isRoomUpgradeJobCreationCountdownRunning]) {
                [floorsWithRunningRoomUpgrades addObject:floor];
            }
        }
    }
    return floorsWithRunningRoomUpgrades;
}
-(NSMutableArray *) floorsWithValidPermissionsForJob:(JobType)jobType {
    NSMutableArray *validFloors = [[NSMutableArray alloc]init];
    for (DTTowerFloor *floor in _revealedFloors) {
        if ([_tower checkIfFloor:floor.floorNumber isValidForJob:jobType]) {
            [validFloors addObject:floor];
        }
    }
    return validFloors;
}

#pragma mark - Simple Floor Collections
-(NSMutableArray *) allFloorsWithInactiveRooms {
    NSMutableArray *allFloorsWithInactiveRooms = [[NSMutableArray alloc]init];
    for (DTTowerFloor *floor in _revealedFloors) {
        if ([floor hasRoomWithWorkOption]) {
            if (![floor isRoomUpgradeJobCreationCountdownRunning]) {
                [allFloorsWithInactiveRooms addObject:floor];
            }
        }
    }
    return allFloorsWithInactiveRooms;
}
-(NSMutableArray *) floorsWithRooms {
    NSMutableArray *floorsWithRooms = [[NSMutableArray alloc]init];
    for (DTTowerFloor *floor in _revealedFloors) {
        if ((floor.floorBuildState & FloorHasRoom) && ((floor.floorBuildState & FloorHasRoomUpgrade) == 0)) {
            [floorsWithRooms addObject:floor];
        }
    }
    return floorsWithRooms;
}
-(NSMutableArray *) floorsWithoutLadders {
    NSMutableArray *floorsWithoutLadders = [[NSMutableArray alloc]init];
    for (DTTowerFloor *floor in _revealedFloors) {
        if (((floor.floorBuildState & FloorHasLadder) == 0) && [floor getActiveJobType] != LadderJob && [self.tower checkIfFloor:floor.floorNumber isValidForJob:LadderJob]) {
            [floorsWithoutLadders addObject:floor];
        }
    }
    return floorsWithoutLadders;
}
-(NSMutableArray *) floorsForDigging {
    NSMutableArray *floorsForDigging = [[NSMutableArray alloc]init];
    for (DTTowerFloor *floor in _revealedFloors) {
        if (floor.groundBlocks.count > 0 && [floor getActiveJobType] != JobTypeMining && (floor.floorBuildState & FloorHasLadder)) {
            [floorsForDigging addObject:floor];
        }
    }
    return floorsForDigging;
}
-(NSMutableArray *) floorsNeedingHauling {
    NSMutableArray *floorsNeedingHauling = [[NSMutableArray alloc]init];
    for (DTTowerFloor *floor in _revealedFloors) {
        if (floor.groundBlocks.count == 0 && floor.itemsNeedingHauling.count > 0 && [floor getActiveJobType] != JobTypeHaulItem) {
            [floorsNeedingHauling addObject:floor];
        }
    }
    return floorsNeedingHauling;
}

#pragma mark - NSCoding
-(id) initWithCoder:(NSCoder *)aDecoder {
    self = [super init];
    if (self) {
        _state = [aDecoder decodeIntegerForKey:@"state"];
        _moveToPerform = [aDecoder decodeObjectForKey:@"moveToPerform"];

        _revealedFloors = [aDecoder decodeObjectForKey:@"revealedFloors"];
        _tower = [aDecoder decodeObjectForKey:@"tower"];

        _commonResources = [aDecoder decodeIntForKey:@"commonResources"];
        _rareResources = [aDecoder decodeIntForKey:@"rareResources"];
        _foodResources = [aDecoder decodeIntForKey:@"foodResources"];

        _countdownBetweenJobs = [aDecoder decodeObjectForKey:@"countdownBetweenJobs"];
        _countdownBetweenJobsAmount = [aDecoder decodeIntForKey:@"countdownBetweenJobsAmount"];
    }
    return self;
}
-(void) encodeWithCoder:(NSCoder *)aCoder {
    [aCoder encodeInteger:self.state forKey:@"state"];
    [aCoder encodeObject:self.moveToPerform forKey:@"moveToPerform"];

    [aCoder encodeObject:_revealedFloors forKey:@"revealedFloors"];
    [aCoder encodeObject:self.tower forKey:@"tower"];

    [aCoder encodeInt:self.commonResources forKey:@"commonResources"];
    [aCoder encodeInt:self.rareResources forKey:@"rareResources"];
    [aCoder encodeInt:self.foodResources forKey:@"foodResources"];

    [aCoder encodeObject:_countdownBetweenJobs forKey:@"countdownBetweenJobs"];
    [aCoder encodeInt:_countdownBetweenJobsAmount forKey:@"countdownBetweenJobsAmount"];    
}

@end

DTGameAIJob.h

#import <Foundation/Foundation.h>
#import "DTGamePlayingAIJobType.h"
#import "DTJobType.h"
#import "DTRoomType.h"

@interface DTGameAIJob : NSObject <NSCoding>

-(id) initWithFloor:(int)floorNumber AIJobToDo:(GamePlayingAIJobType)AIJobType jobToDo:(JobType)jobType roomType:(int)roomType;

@property int floorToDoMove;
@property GamePlayingAIJobType jobTypeToDo;
@property JobType jobToDo;
@property RoomType roomToBuild;

@end

DTGameAIJob.m

#import "DTGameAIJob.h"

@implementation DTGameAIJob

-(id) initWithFloor:(int)floorNumber AIJobToDo:(GamePlayingAIJobType)AIJobType jobToDo:(JobType)jobType roomType:(int)roomType {
    self = [super init];
    if (self) {
        _floorToDoMove = floorNumber;
        _jobTypeToDo = AIJobType;
        _jobToDo = jobType;
        _roomToBuild = roomType;
    }
    return self;
}

-(id) initWithCoder:(NSCoder *)aDecoder {
    self = [super init];
    if (self) {
        _floorToDoMove = [aDecoder decodeIntForKey:@"floorToDoMove"];
        _jobTypeToDo = [aDecoder decodeIntegerForKey:@"jobTypeToDo"];
        _jobToDo = [aDecoder decodeIntegerForKey:@"jobToDo"];
        _roomToBuild = [aDecoder decodeIntegerForKey:@"roomToBuild"];
    }
    return self;
}
-(void) encodeWithCoder:(NSCoder *)aCoder {
    [aCoder encodeInt:self.floorToDoMove forKey:@"floorToDoMove"];
    [aCoder encodeInteger:self.jobTypeToDo forKey:@"jobTypeToDo"];
    [aCoder encodeInteger:self.jobToDo forKey:@"jobToDo"];
    [aCoder encodeInteger:self.roomToBuild forKey:@"roomToBuild"];
}

@end

Here is where the AI is updated inside the Game class:

-(void) processAI {
    //once a second tower is added the ai will start running
    if (_towerArray.count > 1) {

        [_gameAI update];

        if (_gameAI.state == GamePlayingAIStateReadyForInput) {

            _gameAI.commonResources = self.currentCommonResources;
            _gameAI.rareResources = self.currentRareResources;
            _gameAI.foodResources = self.currentFoodResources;

            //there was a bug with doing it this way in the prototype which was fixed by instantiating a new tower and copying relevant values over, then passing it in
            //after fleshing out the real class, switched back to this way and the bug was gone
            _gameAI.tower = [_towerArray firstObject];

            _gameAI.state = GamePlayingAIStateReceivedInput;

        } else if (_gameAI.state == GamePlayingAIStateMoveDecided) {

            if (_gameAI.moveToPerform) {
                switch (_gameAI.moveToPerform.jobTypeToDo) {
                    case GamePlayingAIJobTypeBasicJob:
                        [self addJobToQueueOfType:_gameAI.moveToPerform.jobToDo forFloor:_gameAI.moveToPerform.floorToDoMove];
                        break;
                    case GamePlayingAIJobTypeFlipRoomUpgradeSwitch:
                        [self flipRoomUpgradeCountdownSwitch:_gameAI.moveToPerform.floorToDoMove];
                        break;
                    case GamePlayingAIJobTypeRoomUpgrade:
                        [self buildRoomUpgradeOfType:_gameAI.moveToPerform.roomToBuild forFloor:_gameAI.moveToPerform.floorToDoMove];
                        break;
                    default:
                        break;
                }
            }
            _gameAI.moveToPerform = nil;
            _gameAI.state = GamePlayingAIStateNoMove;
        }
    }
}
\$\endgroup\$
7
\$\begingroup\$

DTGamePlayingAI

For this post, I'm only looking at the DTGamePlayingAI class. This is a big question, and this is a big answer. I don't want the lack of commentary on other posted classes to be taken as a sign that they're flawless. And some of the comments from this post may be applicable to the other classes as well, but as a full disclaimer, I'm writing this post before having even looked at the other classes.


for (id key in self.tower.towerDict) {
    DTTowerFloor *floor = [self.tower.towerDict objectForKey:key];
    if (floor.isRevealed) {
        [_revealedFloors addObject:floor];
    }
}

When iterating over all the objects in a dictionary and the key itself is irrelevant, we can iterate over the objects as an array:

for (DTTowerFloor *floor in [self.tower.towerDict allObjects]) {
    if (floor.isRevealed) {
        [_revealedFloors addObject:floor];
    }
}

if (youngEnoughDwarves.count < kMinimumDwarves) {
    //make sure that no birther rooms are already active
    if ([self floorsWithRunningRoomUpgradesOfType:RoomTypeBirther].count > 0) {
        return NO;
    }
    return YES;
}
return  NO;

Having BOOL returns where the logic works out like this is quite common. You use it more than once. I've ended up with structures like this myself before as well.

However, I find a different pattern to be slightly more pleasing.

BOOL needMoreDwarves = (youngEnoughDwarves.count < kMinimumDwarves);
BOOL canBirthMoreDwarves = ([self floorsWithRunningRoomUpgradesOfType:RoomTypeBirther].count > 0);

return needMoreDwarves && canBirthMoreDwarves;

-(BOOL) needMoreFood:(int)currentFood;
-(BOOL) needMoreCommonResources:(int)commonResources;

These two methods are curious. They're not exposed publicly, so they're only used privately. The only argument they seem to take is a property of this class... a variable the method can already know about without it being passed. Why are we passing a variable? Just make the methods take no arguments and access the appropriate variable within the method.


You have a lot of methods which build and return a mutable array and in a lot of places, you're calling these methods only to call count on the array to get a number. And probably the worst part here is that you're not even actually concerned with the count. You're only concerned with whether or not an object exists in the array.

Maybe I've missed a spot, but it seems that your methods build these arrays to completeness, then the most that is ever done with the return value is to check whether it's count is greater than zero, and in some cases, do something with the first object in the array.

I'm suggesting we change that. These methods shouldn't build and return arrays. Instead, the should iterate through _revealedFloors and return the first object they would otherwise have added to the array they're building. If we make it through the entirety of the array and haven't found a match, we return nil.

Now, we've saved a bit of time and a bit of space. And we can still do everything we'd want to do.

In the cases where we'd just check if the count is greater than 0, instead we can just check the return to see whether it's nil or not.

In the cases where we'd check if the count is greater than 0 then grab firstObject, we can instead just use the return value (after checking it for nil).

And by the way, you don't need to check an array's count before calling firstObject or lastObject. These two convenience methods are relatively new, and Apple designed these so that they'll return nil if the array is empty, preventing the need for checking the array's size to prevent an index out of bounds exception. Just grab firstObject or lastObject and check if it's nil.

\$\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.