# Map generator sometimes doesn't work

I am making a dungeon map generator and am currently trying to make it place empty rooms. It first checks if the space is empty and then places a room. The problem is that most of the times it compiles and I get a map/2d array printed, but sometimes it compiles but doesn't print the map. I don't get an error.

public class Mapa {

public static void main(String[] args) {
// Create a 2D array map filled with floors

char map1[][] = new char[100][100];
for (int i = 0; i < map1.length; i++) {
for (int j = 0; j < map1[i].length; j++) {
map1[i][j] = tiles.wall();

}
}
char mapFinal[][] = roomPlacement(map1);
for (int i = 0; i < mapFinal.length; i++) {
for (int j = 0; j < mapFinal[i].length; j++) {
System.out.print(mapFinal[i][j] + " ");
}
System.out.println();
}

}

public static char[][] roomPlacement(char map1[][]) {
// this method places rooms into the 2D array map

char mapReplacement[][] = map1;

int roomNumber = (int) (10 + (Math.random() * 10));
int roomLength = (int) (10 + (Math.random() * 10));
int roomWidth = (int) (10 + (Math.random() * 10));

int startPositionX = (int) (Math.random() * (mapReplacement.length - roomLength));
int startPositionY = (int) (Math.random() * (mapReplacement.length - roomWidth));

int counter = 0;

while (roomNumber > counter  ) {
if (position(startPositionX, startPositionY, roomLength, roomWidth, mapReplacement)) {

if (roomSpace(mapReplacement, startPositionX, startPositionY, roomLength, roomWidth) == true) {
for (int i = startPositionX; i < (startPositionX + roomLength); i++) {
for (int j = startPositionY; j < (startPositionY + roomWidth); j++) {
mapReplacement[i][j] = tiles.floor();
}
}
}
if (roomSpace(mapReplacement, startPositionX, startPositionY, roomLength, roomWidth) == false) {
startPositionX = (int) (Math.random() * (mapReplacement.length - roomLength));
startPositionY = (int) (Math.random() * (mapReplacement.length - roomWidth));

}

counter++;
}

roomLength = (int) (10 + (Math.random() * 10));
roomWidth = (int) (10 + (Math.random() * 10));

}

return mapReplacement;
}

public static boolean roomSpace(char map1[][], int startPositionX, int startPositionY, int roomLength,
int roomWidth) {
// this method checks if there is enough free space

boolean isFreeSpace = false;
char mapCheck[][] = map1;

int counter = 0;
for (int i = startPositionX; i < (startPositionX + roomLength); i++) {
for (int j = startPositionY; j < (startPositionY + roomWidth); j++) {
if (mapCheck[i][j] == tiles.wall()) {
counter++;
}
}
}

if (counter == (roomLength * roomWidth)) {
isFreeSpace = true;
}

if (isFreeSpace == true) {
System.out.println("true");
} else if (isFreeSpace == false) {
System.out.println("false");
}

return isFreeSpace;

}

public static boolean position(int startPositionX, int startPositionY, int roomLength, int roomWidth,
char mapReplacement[][]) {
boolean position = false;
// checks if the room is out of bounds
if (startPositionX > 0 && startPositionX < mapReplacement.length && startPositionX + roomLength > 0
&& startPositionX + roomLength < mapReplacement.length && startPositionY > 0
&& startPositionY < mapReplacement.length && startPositionY + roomWidth > 0
&& startPositionY + roomWidth < mapReplacement.length) {
position = true;
}

return position;
}

}


Sorry for the long code but wanted to let you see the whole code to better understand it

You get an infinite loop. The statement that increases your counter in the while loop is inside an if statement that checks if the position is in bounds. Else it just sets roomLength and roomWidth to new values. To fix this you could move the counter++ statement at the end of the while block so that it is guaranteed to be executed. Note that this means that you could sometimes get 0 rooms if you're unlucky. You probably also want to set startPositionX and startPositionY to fresh random values at that point instead of only if roomSpace is false. You can see that this is happening, since your program never actually finishes ("returns"). That means it's still computing, so for simple programs an infinite loop is the likely culprit.

Some other minor stuff: You don't need an == true or == false check. if (a == true) is the same as if (a) (as long as a is a boolean or can automatically be casted to one, im not sure ad hoc what the rules are for this in Java), and if (a == false) is equal to if (!a). Similarly you can change your position method to simply be

return startPositionX > 0
&& startPositionX < mapReplacement.length
&& startPositionX + roomLength > 0
&& startPositionX + roomLength < mapReplacement.length
&& startPositionY > 0
&& startPositionY < mapReplacement.length
&& startPositionY + roomWidth > 0
&& startPositionY + roomWidth < mapReplacement.length;


Lastly, I'd recommend you try reading up on some development practices, like SOLID - your code is a bit hard to read. One easy thing that I mean is simply renaming methods: Methods are actions, so their name should be a verb. If I see public static boolean position(), I have no idea what this method does. If I instead see public static boolean isValidPosition() I know that I have to give it a position and it will check if it is valid in some sense.

• thanks for the answer. I fixed the counter, but like you said, sometimes I get a map with no room or just one room. What is the best way to fix that? – NikolaJ Jan 5 at 12:49
• Like I suggested, try out different starting positions each iteration. Other options include counting how many rooms you generated and if you didn't generate a minimum number of rooms, to start over again. Or you could change the algorithm: For example, roll a random center of the room and a random size and try growing the room to that size without bumping into other rooms. That would work better. You might be interested in literature about procedural generation, but it always depends a bit on the game you're making. – Gaze Jan 5 at 14:18
• With regard to == true or especially == false, some style guides recommend keeping them because they make the condition more explicitly visible (it's easy to miss a single ! when scanning quickly) – DMGregory Jan 5 at 14:24
• @DMGregory That is the reason I use that. It is much easier to read and think about it that way – NikolaJ Jan 5 at 15:17
• @Gaze Thanks, will try. Do you know any good books on procedural generation or are you just suggesting it? – NikolaJ Jan 5 at 15:18