I challenged -> Claude 3.5 to follow -> these way to detailed instructions to create a simple 4X "play by mail" strategy game in Dart.
It spit out ~500 lines of Dart code which I had to stitch together because it wasn't able to create the code in one go. I had to fix its usage of dynamic types with JSON decoding because I always enable strict-casts in my analysis_options.yaml, but otherwise there were no syntax errors. Compared to ChatGPT, Claude's style is a bit more "archaic", though.
Because I could have created the Dart code myself, I find it difficult to rate the AI's achievement. Do you find the result impressive? Please read my specification and try to come up with an implementation on your own.
Let's analyse -> Claude's code.
It created a Planet, a Player and a GameState class for JSON serialization and a Game class for implementing the game rules.
It didn't create a Fleet class or an Instruction class or a Report class which is something I'd probably have done.
As instructed, it extracted all constants (I renamed them from GRID_SIZE to kGridSize to match the usual Dart conventions, though, to silence the linter).
const int kGridSize = 10;
const int kTotalPlanets = 50;
const int kMaxPlayers = 5;
const int kInitialFleetSize = 16;
const int kInitialHomePlanetResource = 4;
const int kMinimumPlanetResource = 1;
const int kMaximumPlanetResource = 6;
const int kSpaceYardCost = 8;
The Planet has an id, a coordinate, a resource value, an owner, a spaceyard flag and a map for fleets along with another map for tracking attacking fleets. The latter is also serialized with isn't needed (and could be called wrong).
class Planet {
int id;
int x;
int y;
int resourceValue;
String? owner;
bool hasSpaceyard;
Map<String, int> fleets;
Map<String, bool> attackingFleets;
...
}
The Player has an id, a resource value and lists for instructions and the report. Again, everything is serialized which isn't needed (and could be called wrong), because the last two fields are clearly needed only while processing a turn.
class Player {
String id;
int resources;
List<String> instructions;
List<String> report;
...
}
Neither player not planets have any business logic. They are plain data structures. A planet could deal with joining and splitting fleets, though. A player could generate their report. I'd consider this better object-oriented style.
I already see two problems with the representation. Do you too?
The GameState knows about the turn, all planets and all players. It also tracks battle reports. I think, the AI misunderstood me. Everybody gets all battle reports, not only the involved players. But I didn't specify this and just assumed that this would have been clear.
class GameState {
int turn;
List<Planet> planets;
List<Player> players;
List<String> battleReports;
...
}
Overall, those data model classes look fine, although they don't distinguish persistent and transient fields. Perhaps this is something developers also fail to do so the AI learned a bad style here.
The Game does all the work.
class Game {
GameState state;
Random random = Random();
Game(this.state);
...
}
It has a state and knows a random generator which is good. However, instead of passing a state in the constructor, which is never used as we see in just a minute, it would have been better (I'd even say more correct) to pass the random generator here. Without pinning the Random object, it would be impossible to create unit tests. Always remember, Random and DateTime are the enemy of all unit tests. But that's another common mistake, that Claude learned from weak examples, I guess.
There are two methods initializeGame and loadState to set the state and the latter calls the former in case there is no state.json file.
It does the right thing for randomizing the planet coordinates by creating a list of all possible coordinates, shuffleing it and then taking the first 50, but it didn't pick up my hint that the number of planets is half of the number of sectors. It also didn't know about tuple syntax, using a List<List<int>> instead of a List<({int x, int y})>.
When generating the home planets, it takes the first five planets which isn't wrong but makes those planets guessable so as human who tries to think along, I'd probably randomized the list again before choosing.
There's a saveState method that is called at the end of runTurn, which is eventually called in main to process a turn.
void runTurn() {
loadState();
readInstructions();
collectResources();
processInstructions();
generateReports();
state.turn++;
state.battleReports.clear();
saveState();
}
So far, so good.
The readInstructions method checks whether an instruction file is present, so it should theoretically work to run the application without any instruction file and get the first turn report. The method also deletes the files it has read – which I'm sure I didn't ask for.
void readInstructions() {
for (var player in state.players) {
var file = File('${player.id}.txt');
if (file.existsSync()) {
player.instructions = file.readAsLinesSync();
file.deleteSync();
}
}
}
I'm also sure that I said that collecting resources is the last step, not the first. Wait, no, I didn't specify this at all. My bad. But doing this at the beginning is wrong. It should be the last step. But otherwise, the code looks correct, although I'd probably iterated all planets, searching the owner instead of iterating all players, searching matching planets. A matter of taste. I like the idea of adding the number of collected resources to the repport.
void collectResources() {
for (var player in state.players) {
int collectedResources = 0;
for (var planet in state.planets.where((p) => p.owner == player.id)) {
collectedResources += planet.resourceValue;
}
player.resources += collectedResources;
player.report.add('Collected $collectedResources resources from owned planets.');
}
}
To processInstructions, it iterates the instructions of all players multiple times, always splitting the string and matching the instruction letter. That's not wrong but a bit inefficient therefore I'd have probably created an Instruction class. Also, there's no error handling at all! I didn't explicitly asked for it but it should be obvious, IMHO.
First, it searches for T (transfer starships) and G (gift resources) instructions, calling transferStarships or giveResources methods, so the code is actually quite well structured. But it will crash if an instruction has not enough arguments or doesn't use an integer where expected.
Second, it searches for M and A, calling moveStarships.
Third, it calls resolveBattles and then checkPlanetOccupations.
Last, it searches for S (build starships), Y (build spaceyard), and D (develop planet - I think, I never called it "develop" so it inferred it from the D which is impressive) instructions and calls fitting methods. It failed to understand that you cannot issue Y and S for the same planet, though. And it failed to guess my reasoning that D should be issued only once per planet. It it actually quite difficult to write a complete specification.
It then clears all instructions so that they are never saved as part of the state. Unfortunately, it later tries to report them, which doesn't work.
Looking at transferStarships we see the general problem with all methods that execute instructions. Nearly no error handling.
void transferStarships(Player player, int planetId, int count, String targetPlayerId) {
var planet = state.planets.firstWhere((p) => p.id == planetId);
if (planet.fleets[player.id] != null && planet.fleets[player.id]! >= count) {
planet.fleets[player.id] = planet.fleets[player.id]! - count;
planet.fleets[targetPlayerId] = (planet.fleets[targetPlayerId] ?? 0) + count;
player.report.add('T,$planetId,$count,$targetPlayerId - OK');
} else {
player.report.add('T,$planetId,$count,$targetPlayerId - Error: Not enough ships');
}
}
What if I try to transfer -10 ships? I could also use Z as a player id. Or 99 as a planet id.
From any human developer, I'd expect code like this:
if (!validateCount(player, instr, count)) return;
if (!validatePlayer(player, instr, targetPlayerId)) return;
final planet = state.findPlanet(planetId);
if (planet == null) return player.reportError(instr, 'no such planet');
final ships = planet.subtractShips(player.id, count);
if (shis == null) return player.reportError(instr, 'not enough ships');
planet.addShips(targetPlayerId, ships);
And if you'd have an Instruction object that has an execute method that knows the game and assuming that this knows the current player along with the state, it could look like this:
class TransferInstruction extends Instruction {
final Planet planet; // already guaranteed to exist
final String targetPlayerId; // already guaranteed to exist
final int count; // already guaranteed > 0
void execute(Game game) {
final ships = planet.subtractShips(currentPlayerId, count);
if (ships == null) return reportError('not enough ships');
planet.addShips(targetPlayerId, ships);
reportSuccess();
}
}
But back to Claude's code. It wrote a canReach method that checks whether you can move from one planet to another one, by using a breadth-first A* search. Good job.
bool canReach(int startX, int startY, int endX, int endY) {
var visited = List.generate(kGridSize, (_) => List.filled(kGridSize, false));
var queue = [
[startX, startY]
];
while (queue.isNotEmpty) {
var current = queue.removeAt(0);
var x = current[0];
var y = current[1];
if (x == endX && y == endY) return true;
// Loop adjecent sectors of (x/y) and queue them, if empty
// or if it is (endX/endY) where we could also have just
// returned with true. Also mark sector visited so that we
// don't create loops
}
return false;
}
In moveStarships it faithfully implemented my instructions that you can only move ships if you own the planet, which I now notice, is a stupid restrictions. You should be able to move ships if you have a fleet. That part of my rules was from a time when I had a simple only one player can have ships at a planet approach. But the AI didn't notice and questioned my design. Unfortunately.
The bigger problem: It didn't make sure that you cannot move a ship twice. Again, a human developer should have thought along and noticed this, even if I didn't specify this behavior.
The most complex piece of the code is resolveBattles.
It starts by searching for planets that have at fleets of at least two factions and at least one attacking fleet. The way the second attackingFleets map can be used here is kind-of clever. I would have used a Fleet class with an attacking flag and this would have been a bit more involved.
void resolveBattles() {
for (var planet in state.planets) {
if (planet.fleets.length > 1 || planet.attackingFleets.isNotEmpty) {
It will then begin a battleReport and makes a copy of all ship counts, probably to implement the way that all ships fire once and kill each other. Unfortunately, it fails to do so in the following for loop.
var battleReport = StringBuffer('Battle at planet ${planet.id}:\n');
var fleets = Map.of(planet.fleets);
Now, there's a loop which I think, I didn't ask for. I do not want a battle to the death. All ships fire once per turn. There can be an inconclusive result. So, I think, this is hallucinated. Probably from other games of this type.
while (fleets.length > 1 || (fleets.length == 1 && planet.owner != null && planet.owner != fleets.keys.first)) {
Here, all factions will do combat.
for (var entry in fleets.entries) {
var attackerId = entry.key;
var attackerShips = entry.value;
And it missunderstood me. I wanted that a die result of 1..3 means 0 casulties, and a result of 4 to 6 means 1 to 3 casulties. I made up that rule on the spot, hoping that this way, a smaller fleet can destroy a larger one, but on the average, the larger fleet will win.
The remaining code looks okayish, but I think, it's difficult to understand without further comments and using helper methods would have helped :)
for (var i = 0; i < attackerShips; i++) {
var roll = random.nextInt(6) + 1;
if (roll > 3) {
var targets = fleets.keys.where((id) => id != attackerId).toList();
if (targets.isNotEmpty) {
var targetId = targets[random.nextInt(targets.length)];
fleets[targetId] = fleets[targetId]! - 1;
if (fleets[targetId]! <= 0) {
fleets.remove(targetId);
battleReport.write('Fleet $targetId was destroyed.\n');
}
}
}
}
}
}
At this point, the battle has been resolved and the result must be applied to the planet. However, the attempt to remove ships from another list of fleets failed and therefore, this assigned the wrong result. This also only works because the code above ran the battle until only one fleet survived.
if (fleets.isNotEmpty) {
var winner = fleets.keys.first;
planet.fleets = fleets;
planet.attackingFleets.clear();
if (planet.owner != winner) {
battleReport.write('Player $winner has conquered planet ${planet.id}.\n');
planet.owner = winner;
} else {
battleReport.write('Player $winner has successfully defended planet ${planet.id}.\n');
}
} else {
battleReport.write(
'All fleets were destroyed. Planet ${planet.id} remains under control of ${planet.owner ?? "no one"}.\n');
planet.fleets.clear();
planet.attackingFleets.clear();
}
state.battleReports.add(battleReport.toString());
}
}
}
So, unfortunately, let's at least call this a missunderstanding by the AI. But frankly, it was completely wrong in the most important and more difficult part of the applcation. Which is a pitty, because in an earlier attempt where I did miss out some things in the instructions, it actually managed to get the battle right (but failed on reporting).
Apropos, generateReports iterates all players, uses a StringBuffer to record turn and resources and also manages to add a map like so, hard-coding the grid size of 10 into the separator strings, unfortunately. The List<List<String>> grid is a local variable that is populated with left-padded planet numbers and player ids earlier.
report.writeln('+---+---+---+---+---+---+---+---+---+---+');
for (var row in grid) {
report.writeln('|${row.join('|')}|');
report.writeln('+---+---+---+---+---+---+---+---+---+---+');
}
report.writeln();
Unfortunately, it forget that without an owner, there is no player id and therefore, the string is shorter, so this all is unaligned. But it's easy to fix, so I don't mind.
The list of planets, I then asked for, looks correct.
// List planets
for (var planet in state.planets.where((p) => p.owner == player.id || p.fleets.containsKey(player.id))) {
report.write(planet.id.toString().padLeft(2, '0'));
if (planet.owner != null) report.write(planet.owner!);
report.write(' (${planet.x.toString().padLeft(2, '0')}/${planet.y.toString().padLeft(2, '0')}) ');
report.write('R${planet.resourceValue}, ');
if (planet.hasSpaceyard) report.write('Y, ');
report.write('F: ');
planet.fleets.forEach((ownerId, count) => report.write('$count$ownerId, '));
report.writeln();
}
report.writeln();
But why don't people use more helper methods so the AI can learn a better style?
for (var planet in state.planetsVisibleBy(player)) {
report.write(planet.idWithOwner);
report.write(' ');
report.write(planet.coordinate);
report.write('R${planet.resourceValue}');
if (planet.hasSpaceyard) report.write(', Y');
if (planet.fleets.isNotEmpty) {
report.write(', F: ');
report.write(planet.formattedFleetList);
}
report.writeln();
}
It also tries to report all instructions but that is pointless, as instructions where cleared after processing them. Instead, it should have emitted the recorded error and success messages (player.report) but failed to do so.
At least, it adds all battle reports and here, I think, it is my fault that I didn't specify that those reports should only be seen by involved players and not by everyone.
And there you have it. 500 lines of code that at the first glance look okay, but are wrong in some very important parts. Was this helpful? I think, it took me nearly as long to write the specification than to implement the code. Automatically generating the JSON serialization code (omitted here) was helpful, but otherwise, carefully checking the generated code took me at least as long as writing it myself.
I gave Claude another chance:
You didn't correctly implement battles. Fleets should not fight to the death, but each fleet will attack only once per turn. Roll 1d6 per ship of that fleet. A result of 1-3 means no effect. A result of 4 to 6 means that 1 to 3 randomly chosen enemy ships get destroyed. Also, do only add the battle report to players that are involved in that battle.
It responded with this code (and some explanation to demonstrate understanding of the task which I omitted here and for the original task which also contained this explanations along with the code):
void resolveBattles() {
for (var planet in state.planets) {
if (planet.fleets.length > 1 || planet.attackingFleets.isNotEmpty) {
var battleReport = StringBuffer('Battle at planet ${planet.id}:\n');
var fleets = Map.of(planet.fleets);
This looks much better…
var involvedPlayers = Set<String>.from(fleets.keys);
But it still doesn't understand that it made the copy of planet.fleets so that it can savely iterate the original map which modifying the copy. This will still result in concurrent modification errors.
// Each fleet attacks once
for (var attackerId in fleets.keys) {
var attackerShips = fleets[attackerId]!;
var targets = fleets.keys.where((id) => id != attackerId).toList();
An if (targets.isEmpty) continue; style of guard statements would make things easier to read. BTW, despite the instructions, I'd have used random.nextBool() to determine a hit and then random.nextInt(3)+1 to determine the damage.
if (targets.isNotEmpty) {
var totalDamage = 0;
for (var i = 0; i < attackerShips; i++) {
var roll = random.nextInt(6) + 1;
if (roll >= 4) {
totalDamage += roll - 3;
}
}
battleReport.write('Fleet $attackerId deals $totalDamage damage.\n');
// Distribute damage randomly among enemy ships
while (totalDamage > 0 && targets.isNotEmpty) {
var targetId = targets[random.nextInt(targets.length)];
fleets[targetId] = fleets[targetId]! - 1;
totalDamage--;
if (fleets[targetId]! <= 0) {
fleets.remove(targetId);
targets.remove(targetId);
battleReport.write('Fleet $targetId was destroyed.\n');
}
}
}
}
This looks better now. I didn't ask to determining stalemate but it's nice that this gets reported. I like that.
// Determine battle outcome
if (fleets.length == 1) {
var winner = fleets.keys.first;
planet.fleets = fleets;
planet.attackingFleets.clear();
if (planet.owner != winner) {
battleReport.write('Player $winner has conquered planet ${planet.id}.\n');
planet.owner = winner;
} else {
battleReport.write('Player $winner has successfully defended planet ${planet.id}.\n');
}
} else if (fleets.isEmpty) {
battleReport.write('All fleets were destroyed. Planet ${planet.id} remains under control of ${planet.owner ?? "no one"}.\n');
planet.fleets.clear();
planet.attackingFleets.clear();
} else {
battleReport.write('The battle ends in a stalemate. Remaining fleets:\n');
for (var entry in fleets.entries) {
battleReport.write('Player ${entry.key}: ${entry.value} ships\n');
}
planet.fleets = fleets;
planet.attackingFleets.clear();
}
This is correct, too:
// Add battle report only to involved players
for (var playerId in involvedPlayers) {
state.players.firstWhere((p) => p.id == playerId).report.add(battleReport.toString());
}
}
}
}
Last but not least, I tried this:
Code can be improved if you use
Instructionsubclasses to represent all instructions. AInstruction.parsefactory constructor should create them. Those subclasses should have anexecute(Game, Player)method and throw exception on errors. The main loop that executes all instructions can then catch that exceptions and report them as errors. What do you think?
It answered "You're absolutely right. This is an excellent suggestion that will greatly improve the code structure, readability, and maintainability" and spit out this code:
abstract class Instruction {
String get command;
void execute(Game game, Player player);
static Instruction parse(String input) {
var parts = input.split(',');
switch (parts[0]) {
case 'T':
return TransferInstruction(int.parse(parts[1]), int.parse(parts[2]), parts[3]);
...
default:
throw FormatException('Unknown instruction: ${parts[0]}');
}
}
}
class TransferInstruction extends Instruction {
final int planetId;
final int count;
final String targetPlayerId;
TransferInstruction(this.planetId, this.count, this.targetPlayerId);
@override
String get command => 'T,$planetId,$count,$targetPlayerId';
@override
void execute(Game game, Player player) {
var planet = game.state.planets.firstWhere((p) => p.id == planetId);
if (planet.fleets[player.id] == null || planet.fleets[player.id]! < count) {
throw Exception("Not enough ships to transfer");
}
planet.fleets[player.id] = planet.fleets[player.id]! - count;
planet.fleets[targetPlayerId] = (planet.fleets[targetPlayerId] ?? 0) + count;
}
}
Looks like it didn't know about factory constructors. It still lacks error checking, but at least it catches all errors and doesn't crash anymore. Of course, now it forgot about executing instructions in a certain order. Parsing must be a separate step. So, this is actually worse than before. Did I reach the context window limitation?
void processInstructions() {
for (var player in state.players) {
for (var instructionString in player.instructions) {
try {
var instruction = Instruction.parse(instructionString);
instruction.execute(this, player);
player.report.add('${instruction.command} - OK');
} catch (e) {
player.report.add('$instructionString - Error: ${e.toString()}');
}
}
}
resolveBattles();
checkPlanetOccupations();
}
So, what does this all mean? I'm still fascinated by an AI able to follow detailed instructions, at least to some extend, but code quality is still poor and on a beginner level. Also, I picked a simple example. Asking for improvements works, though, and if you don't want the AI make 100% of the work and if you're a beginner and don't notice (or don't mind) the poor code quality, it helps to get a job done.
After writing this article, I asked Claude again, with slightly updated instructions because I noticed some errors as described, but now Claude failed to produce a running example and added // TODO messages, asking me to do its job. So, the whole process is very unreliable. Perhaps, because I already spent some internal budget of high quality answer or the AI was too busy or whatever.
Update: I asked Claude to update the original source code to Dart 3.5 (see game3.dart) which worked quite well. It used final for immutable parts of the data model, replaced all var of immutable local variables with final, used more functional collection methods, and records (aka tuples) and pattern matching in a few cases. It missed a chance for if case, though. However, explicitly asking for Dart 3 should give much better results.