I'd suggest writing at least one cellposition iterator for the Dungeon. Something like Dungeon.PositionsInViewOrder(), that would yield the cell positions in the right order for display.
(Note: if Cell objects knew their own Position, I'd suggest just writing a Cell iterator and skipping the Position. But as things are, this is the way to go.)
I'd suggest writing at least one cell iterator for the Dungeon. Something like Dungeon.PositionsInViewOrder(), that would yield the cell positions in the right order for display.
I'd suggest writing at least one position iterator for the Dungeon. Something like Dungeon.PositionsInViewOrder(), that would yield the cell positions in the right order for display.
(Note: if Cell objects knew their own Position, I'd suggest just writing a Cell iterator and skipping the Position. But as things are, this is the way to go.)
Some general comments about how your code "reads":
Usage
You wrote: "To use this code, call:"
dungeonGenerator = new DungeonGenerator();
dungeonGenerator.CreateDungeonRoom(115, 32);
dungeonGenerator.CellsToGenerate = 1500;
dungeonGenerator.CreateDungeonScenery();
dungeonGenerator.LogDungeon();
First, let me say that I support the idea that the dungeon generator and the dungeon object should be different. The code to randomly create rooms has nothing to do with the code that decides whether a user has stepped on a trap, etc. So I like the idea of a separate class/object/function.
That said, this is a very awkward set of calls. It doesn't "scan" well (to use a term from literature). I suspect that all these fine details need to be wrapped up into a single factory function that a user can call:
cavern = Dungeon.Generate(width: 115, height: 32, min_cells: 1500);
cavern.WriteTo(Console.Out);
CellType
Your cell type is an enumeration, which seems obvious but actually isn't very useful. The thing you mainly do with the cell type is display it, and sadly C# enums cannot have char as their underlying type. But you can work around that using an int and just assigning char values:
public enum CellType
{
WALL = '█',
GROUND = 'O',
START = 'H'
};
Just be careful about what size characters you use (16 bit only). If you have to use a full-out string, consider the solution in this answer.
I suggest this because your output code would benefit from having "concrete" values. (The rest of your code is happy with the CellType being an abstract enum.)
switch (DungeonCell[x, y].CellType)
{
case CellType.GROUND:
line.Append("O"); break;
Note: As a rule of thumb, if you find yourself executing a switch on internal data it's a good indicator that you should consider a new type. Switching on external data might be a correct way to deal with user input. But switching on internal data frequently means "these things should be different objects, or different classes". In this case, the enum values are serving as proxies for objects that have their own output string representation.
It would be better, IMO, to simply emit the value, or a method of the value (as in the linked answer):
line.Append(DungeonCell[x, y].cellType);
// -or-
line.Append(DungeonCell[x, y].cellType.ToFriendlyString());
(but see below about that x, y thing!)
Direction
Direction is another enum that doesn't quite do what you need. Currently, you write code like this (code modified for size):
//Choose a direction at random
Direction direction = (Direction)values.GetValue(random.Next(values.Length));
if (direction == Direction.UP)
if (startY > minValueXY)
--startY;
else if (direction == Direction.DOWN)
if (startY < maxValueY)
++startY;
else if (direction == Direction.LEFT)
if (startX > minValueXY)
--startX;
else if (direction == Direction.RIGHT)
if (startX < maxValueX)
++startX;
//From the position chosen, mark it as ground, if possible
Instead of making Direction an enum, what if you make it a class? You can then have Direction.AtRandom() return what you want without having to cast it.
What's more, consider what you are doing: you immediately decode the direction into an X or a Y offset. You then either adjust the variable startX or startY.
Why are those two variables different? What is startX? What is startY? Aren't they part of a greater whole, called Position (or Location or Coords something)?
If you had a Position type, you could just have start instead of startX and startY. You might have to adjust start.X and start.Y, but probably not, because Direction should be a vector.
If Direction.North is a Vector defined as { δx = 0, δy = -1 } then you can define addition of a Position and a Vector in the obvious way (x+δx, y+δy) and then simplify your code:
public static Position operator +(Position pos, Vector v)
{
return new Position(pos.x + v.δx, pos.y + v.δy);
}
Then:
// Choose a direction at random (but don't go out-of-bounds)
dir = Direction.AtRandom();
if (dungeon.Contains(start + dir))
start += dir;
// From the position chosen, mark it as ground, if possible
Note: .Contains may be the wrong name, since there is that implicit rule
about not modifying the outer edges. Maybe "CanBeRoom()"?
Position
If you define an indexer for your Dungeon, you can use the Position directly instead of reaching in for the x and y values. This will cost some performance, but that likely doesn't matter during generation. Alternatively, you might just write a Dungeon.At(Position) method.
Dungeon Rules
I saw one "implicit" rule in your code: the dungeon's outside walls are inviolable. I'd suggest that you make those rules explicit, and encode the operations you are currently implementing with just a paragraph or two of code:
//Make it so that the outside walls are untouched
int minValueXY = 1;
int maxValueX = Width - 2;
int maxValueY = Height - 2;
//Choose a random position in the room
Random random = new Random();
int startX = random.Next(minValueXY, maxValueX);
int startY = random.Next(minValueXY, maxValueY);
This can be replaced by defining methods:
topLeft = dungeon.PlayerTopLeft(); // returns a Position, (1,1)?
botRight = dungeon.PlayerBottomRight(); // returns a Position.
start = Position.AtRandom(min: topLeft, max: botRight);
Iterators
I'd suggest writing at least one cell iterator for the Dungeon. Something like Dungeon.PositionsInViewOrder(), that would yield the cell positions in the right order for display.
You could rewrite your LogDungeon function as a Dungeon method:
public void WriteTo(System.IO.TextWriter out)
{
// Only care about Y
int last_pos = TopLeft() + Vector(δx:0, δy:-1);
for (Position pos in PositionsInViewOrder())
{
if (last_pos.y != pos.y)
out.WriteLine();
last_pos = pos;
out.Write(dungeon[pos].cellType);
}
out.WriteLine();
}
You might find that many dungeon creation tasks are made easier by defining Position iterators. For example, if you want to randomly generate rooms and hallways you could define Position.WithinRect(topLeft:Position, bottomRight:Position) and Position.AlongLine(from:Position, to:Position). (Be consistent on how you handle the end position: inclusive or exclusive!)