6
\$\begingroup\$

I've mostly completed the prototype for my city building simulation, and now I have added the meta-game mission system to the game. You see these mission systems in a great many games these days. The basic idea is that the player is assigned a mission at the start of the game, such as Build 50 Industrial buildings, and once completed gains a permanent bonus that will apply in all future games.

Programming this turned out to be pretty complicated (at least as far as the number of classes goes) so I think a review of the code will be helpful. I have an abstract MayorMission class, and then a few different implementations of that depending on the mission requirements. Some of them take a value for the type of building required to complete the mission, or for example the type of zone, or number of people or money required. I feel like multiple classes here is a good idea because it simplifies the code for checking whether missions are completed, but I wonder if I have taken things too far.

MayorMission.java

public abstract class MayorMission {

    public final int targetNumber;
    public final World world;
    public final String name;
    public final PlayerAbility reward;

    public boolean isComplete;

    public MayorMission(int targetNumber, World world, String name, PlayerAbility reward) {
        this.targetNumber = targetNumber;
        this.world = world;
        this.name = name;
        this.reward = reward;
    }

    public abstract boolean checkIfComplete();
}

IsoTileTypeMission.java

public class IsoTileTypeMission extends MayorMission {

    private final IsoTileType typeRequired;

    public IsoTileTypeMission(int targetNumber, World world, IsoTileType typeRequired, String name, PlayerAbility reward) {
        super(targetNumber, world, name, reward);
        this.typeRequired = typeRequired;
    }

    @Override
    public boolean checkIfComplete() {
        int count = 0;
        for (Tile[] row : this.world.levelData) {
            for (Tile tile : row) {
                if (tile.getBuildingType() == typeRequired) {
                    count++;
                }
            }
        }
        return count >= this.targetNumber;
    }
}

ZoneTypeLevelMission.java

public class ZoneTypeLevelMission extends MayorMission {

    private final int levelRequired;

    public ZoneTypeLevelMission(int targetNumber, World world, int levelRequired, String name, PlayerAbility reward) {
        super(targetNumber, world, name, reward);
        this.levelRequired = levelRequired;
    }

    @Override
    public boolean checkIfComplete() {
        int count = 0;
        for (Tile[] row : this.world.levelData) {
            for (Tile tile : row) {
                if (tile.zoneType.getLevel() >= levelRequired) {
                    count++;
                }
            }
        }
        return count >= this.targetNumber;
    }
}

ZoneTypeMission

public class ZoneTypeMission extends MayorMission {

    List<ZoneType> targetTypes;

    public ZoneTypeMission(int targetNumber, World world, List<ZoneType> targetTypes, String name, PlayerAbility reward) {
        super(targetNumber, world, name, reward);
        this.targetTypes = targetTypes;
    }

    @Override
    public boolean checkIfComplete() {
        int count = 0;
        for (Tile[] row : this.world.levelData) {
            for (Tile tile : row) {
                if (this.targetTypes.contains(tile.zoneType)) {
                    count++;
                }
            }
        }
        return count >= this.targetNumber;
    }
}

WorldPopMission.java

public class WorldPopMission extends MayorMission {

    private final int targetPop;

    public WorldPopMission(World world, int targetPop, String name, PlayerAbility reward) {
        super(0, world, name, reward);
        this.targetPop = targetPop;
    }

    @Override
    public boolean checkIfComplete() {
        int currentPop = this.world.getPopulation();
        for (Tile[] row : this.world.levelData) {
            for (Tile tile : row) {
                currentPop += tile.getPopulation();
            }
        }
        return currentPop >= this.targetPop;
    }
}

MoneyMission.java

public class MoneyMission extends MayorMission {

    public MoneyMission(int targetNumber, World world, String name,
            PlayerAbility reward) {
        super(targetNumber, world, name, reward);
    }

    @Override
    public boolean checkIfComplete() {
        return this.world.currentMoney >= this.targetNumber;
    }
}

MaxPopMission.java

public class MaxPopMission extends MayorMission{

    public MaxPopMission(int targetNumber, World world, String name, PlayerAbility reward) {
        super(targetNumber, world, name, reward);
    }

    @Override
    public boolean checkIfComplete() {
        int count = 0;
        for (Tile[] row : this.world.levelData) {
            for (Tile tile : row) {
                if (tile.getPopulation() >= tile.getMaxPopulation()) {
                    count++;
                }
            }
        }
        return count >= this.targetNumber;
    }
}

DesireMission.java

public class DesireMission extends MayorMission {

    private final int targetDesire;

    public DesireMission(int targetNumber, World world, int targetDesire, String name, PlayerAbility reward) {
        super(targetNumber, world, name, reward);
        this.targetDesire = targetDesire;
    }

    @Override
    public boolean checkIfComplete() {
        int count = 0;
        for (Tile[] row : this.world.levelData) {
            for (Tile tile : row) {
                if (tile.desire >= this.targetDesire) {
                    count++;
                }
            }
        }
        return count >= this.targetNumber;
    }
}

Here is the example creation of missions:

public List<MayorMission> getAllMissions() {
    List<MayorMission> missions = new ArrayList<MayorMission>();
    missions.add(new ZoneTypeMission(50, this.world, ZoneType.getResidentialZones(), "Build 50 Residential", PlayerAbility.UNLOCK_RES_1));
    missions.add(new ZoneTypeMission(50, this.world, ZoneType.getCommercialZones(), "Build 50 Commercial", PlayerAbility.UNLOCK_COM_1));
    missions.add(new ZoneTypeMission(50, this.world, ZoneType.getIndustrialZones(), "Build 50 Industrial", PlayerAbility.UNLOCK_IND_1));

    missions.add(new ZoneTypeLevelMission(50, this.world, 1, "Build 50 Light Zones", PlayerAbility.UNLOCK_RES_2));
    missions.add(new ZoneTypeLevelMission(100, this.world, 1, "Build 100 Light Zones", PlayerAbility.UNLOCK_COM_2));
    missions.add(new ZoneTypeLevelMission(50, this.world, 2, "Build 50 Medium Zones", PlayerAbility.UNLOCK_IND_2));
    missions.add(new ZoneTypeLevelMission(100, this.world, 2, "Build 100 Medium Zones", PlayerAbility.UNLOCK_RES_3));
    missions.add(new ZoneTypeLevelMission(50, this.world, 3, "Build 50 Heavy Zones", PlayerAbility.UNLOCK_COM_3));
    missions.add(new ZoneTypeLevelMission(100, this.world, 3, "Build 100 Heavy Zones", PlayerAbility.UNLOCK_IND_3));

    missions.add(new MoneyMission(100000, this.world, "Save 100000 dollars", PlayerAbility.HIGHER_TAXES));

    missions.add(new WorldPopMission(this.world, 7500, "Have 7500 people", PlayerAbility.BONUS_POPULATION));

    missions.add(new DesireMission(50, this.world, 75, "Have 50 tiles with 75 desire", PlayerAbility.CHEAPER_HEAVY_ZONE));

    //missions.add(new ZoneTypeMission(50, this.world, ZoneType.getUtilityZones(), "Build 50 Utility", null));
    //missions.add(new ZoneTypeMission(15, this.world, ZoneType.getSpecialZones())); //arcos
    //missions.add(new IsoTileTypeMission(50, this.world, IsoTileType.PARK, "Build 50 parks", null));
    //missions.add(new IsoTileTypeMission(20, this.world, IsoTileType.FUN, "Build 20 casinos", null));
    //missions.add(new MaxPopMission(20, this.world, "Have 20 max population tiles", null));
    //missions.add(new ZoneTypeLevelMission(World.WORLD_HEIGHT * World.WORLD_WIDTH, this.world, 1, "Cover map in light zones", null));

    return missions;
}

And here is the simple check to see whether they are completed. This runs every cycle because not all missions are completed under the same type of conditions.

private void checkIfMissionComplete() {
    if (this.game.getPlayer().assignedMission.checkIfComplete()) {
        this.game.getPlayer().assignedMission.isComplete = true;
    }
}

If you would like to try the prototype, here it is: VoxelCity

\$\endgroup\$
7
\$\begingroup\$

There's several things to address here, so let's get to it!

Classes

Too many.

Speed

Can be better.

Declaring missions

Perfect for a config file.

checkIfMissionComplete

Slight code duplication.


Classes

BEEP BEEP BEEP Strategy pattern!

This bothers me a bit:

super(targetNumber, world, name, reward);

Instead of having that, I would use a MayorMission constructor like this:

public MayorMission(int targetNumber, World world, String name,
    PlayerAbility reward, GameGoal goal)

Where GameGoal is something like:

public interface GameGoal {
     boolean checkIfComplete();
}

This reduces the need to subclass MayorMission and let's you use composition over inheritance. This might also lead to the possibility of removing some of the arguments from the constructor.

The different GameGoal objects can then either be implemented as pure classes, or constructed using factory methods and anonymous inner classes.

So instead of:

missions.add(new ZoneTypeMission(50, this.world, 
     ZoneType.getResidentialZones(), "Build 50 Residential", 
     PlayerAbility.UNLOCK_RES_1));

We might end up with:

missions.add(new MayorMission(this.world, "Build 50 Residential", 
     PlayerAbility.UNLOCK_RES_1,
     zoneTypeGoal(50, ZoneType.getResidentialZones())));

Or how about:

missions.add(world.createMission("Build 50 Residential",
     PlayerAbility.UNLOCK_RES_1, 
     zoneTypeGoal(50, ZoneType.getResidentialZones()));

Generic tile scanning

Many of your mission goals depends on a sum of all tiles (when purely counting, the value for each tile matching the predicate is 1)

This leads to a possibilty of:

public class CountingGoal implements GameGoal {
    private final ToIntFunction<Tile> tileValue;

    @Override
    public boolean checkIfComplete() {
        int sum = 0;
        for (Tile[] row : world.levelData) {
            for (Tile tile : row) {
                sum += tileValue.applyAsInt(tile);
            }
        }
        return sum >= targetNumber;
    }
}

ToIntFunction is an interface that exists in Java 8, but as you're not using Java 8 you have to declare it yourself:

public interface ToIntFunction<T> {
     int applyAsInt(T obj);
}

This will allow you to extract implementations, such as:

new ToIntFunction<Tile>() {
    @Override
    public int applyAsInt(Tile tile) {
        return tile.getBuildingType() == typeRequired ? 1 : 0;
    }
}

Or, the class version of that:

class BuildingTypeCounting implements ToIntFunction<Tile> {
    private final BuildingType typeRequired;

    @Override
    public int applyAsInt(Tile tile) {
        return tile.getBuildingType() == typeRequired ? 1 : 0;
    }
}

Speed

Looping over each and every tile in the game is a bit slow. You might want to consider keeping track of the zones that actually have some content on them. You can do this by using a List<Tile> residential, or perhaps even Map<ZoneType, List<Tile>> tilesByType, or if there are generally a lot of empty tiles, perhaps List<Tile> tilesWithBuildings would be enough.

You might want to consider splitting your world data structure so that general tile data is kept as a two-dimensional array, and objects that do something are kept in a List.

checkIfMissionComplete

private void checkIfMissionComplete() {
    if (this.game.getPlayer().assignedMission.checkIfComplete()) {
        this.game.getPlayer().assignedMission.isComplete = true;
    }
}

It feels like it is the mission's job to mark itself as completed, and then it may return true if it did mark it as completed.

private void checkIfMissionComplete() {
    this.game.getPlayer().assignedMission.markIfCompleted();
}

And in the MayorMission:

public boolean markIfCompleted() {
    boolean completed = this.checkIfComplete();
    if (completed) {
        this.isComplete = true;
    }
    return completed;
}
\$\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.