ondras / rot.js

ROguelike Toolkit in JavaScript. Cool dungeon-related stuff, interactive manual, documentation, tests!
https://ondras.github.io/rot.js/hp/
BSD 3-Clause "New" or "Revised" License
2.32k stars 254 forks source link

What is the minimum "safe" size for Rogue map? #202

Closed DmitryShashkov closed 2 years ago

DmitryShashkov commented 2 years ago

Hi! I'm using Rot.js Rogue map generator in my project, and sometimes the output is a little bit strange.

Here's the sample of the code responsible for map generation:

import RogueMap from "rot-js/lib/map/rogue";

// these are usually rolled randomly, I only hardcoded them to demonstrate the issue
const MAP_WIDTH = 42;
const MAP_HEIGHT = 16;

const rotMap = new RogueMap(MAP_WIDTH, MAP_HEIGHT, {});
const rotFreeCells = new Set<string>();
const rotWalls = new Set<string>();

const contentsArray: number[][] = Array(MAP_WIDTH)
  .fill(0)
  .map(() => Array(MAP_HEIGHT).fill(0));

rotMap.create((x, y, contents) => {
  contentsArray[x][y] = contents;
  if (!contents) {
    // I save free cells to Set to ease the check whether cell is free or not
    rotFreeCells.add(coordsToStringKey(x, y));
  }
});

// I do not fill all the space with walls, 
// I only put them aroung free cells
rotFreeCells.forEach((cell) => {
  const [x, y] = stringKeyToCoords(cell);
  const surroundingCells = getSurroundingCells(x, y);
  surroundingCells.forEach(([nearX, nearY]) => {
    if (contentsArray[nearX][nearY]) {
      rotWalls.add(coordsToStringKey(nearX, nearY));
    }
  });
});

Utils used in the code above:

import { RNG } from "rot-js";

export const coordsToStringKey = (x: number, y: number) => `${x},${y}`;
export const stringKeyToCoords = (key: string) =>
  key.split(",").map((part) => parseInt(part, 10)) as [number, number];

export const getSurroundingCells = (x: number, y: number): number[][] => [
  [x - 1, y - 1],
  [x, y - 1],
  [x + 1, y - 1],
  [x - 1, y],
  [x + 1, y],
  [x - 1, y + 1],
  [x, y + 1],
  [x + 1, y + 1],
];

export const rollInRange = (from: number, to: number) =>
  RNG.getUniformInt(from, to);

This works perfectly fine most of the time, but in some rare occasions (as I assume, when map dimensions are too small), it generates unreachable areas. For example, here's rotMap object that was produced by the code above:

{
  "_width": 42,
  "_height": 16,
  "map": [
    [1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1], [1,1,0,0,0,0,1,1,1,1,1,1,1,1,1,1], [1,1,0,0,0,0,0,0,1,1,1,1,1,1,1,1], [1,1,0,0,0,0,1,0,0,1,1,1,1,1,1,1],
    [1,1,0,0,0,0,1,1,0,1,1,1,1,1,1,1], [1,1,0,0,0,0,1,1,0,1,0,0,0,1,1,1], [1,1,0,0,0,0,1,1,0,1,0,0,0,1,1,1], [1,1,1,1,1,1,1,1,0,0,0,0,0,1,1,1],
    [1,1,1,1,1,1,1,1,1,1,0,0,0,1,1,1], [1,1,1,1,1,1,1,1,1,1,0,0,0,1,1,1,0], [1,1,1,1,1,1,1,1,1,1,1,0,1,1,1,1,0], [1,1,1,1,1,1,1,1,1,1,1,0,1,1,1,1,0],
    [1,1,1,1,1,1,1,1,1,1,1,0,1,1,1,1,0], [1,1,1,1,1,1,1,1,1,1,1,0,1,1,1,0,0], [1,0,0,0,0,1,1,1,1,1,1,0,1,1,1,0], [1,0,0,0,0,0,0,1,1,1,1,0,1,1,1,0],
    [1,0,0,0,0,1,0,1,1,1,1,0,1,1,1,0], [1,0,0,0,0,1,0,0,0,1,0,0,1,1,1,1], [1,0,0,0,0,1,1,1,0,0,0,0,1,1,1,1], [1,0,0,0,0,1,1,1,1,1,0,0,1,1,1,1],
    [1,0,0,0,0,1,1,1,1,1,0,0,1,1,1,1], [1,1,1,0,1,1,1,1,1,1,1,0,1,1,1,0], [1,1,1,0,1,1,1,1,1,1,1,0,1,1,1,0], [1,1,1,0,1,1,1,1,1,1,1,0,1,1,1,0],
    [1,1,1,0,1,1,1,1,1,1,1,0,1,0,0,0], [1,1,1,0,1,1,1,1,1,1,1,0,1,0,1,1], [1,1,1,0,1,1,1,1,0,0,0,0,1,0,1,1], [1,1,1,0,1,1,1,1,0,1,1,1,1,0,1,1],
    [1,1,1,0,1,1,1,1,0,1,1,1,1,0,1,1], [1,1,1,0,1,1,1,1,0,0,1,1,1,0,0,1], [1,1,0,0,0,1,1,1,0,0,1,1,1,0,0,1], [1,1,0,0,0,1,1,1,0,0,0,0,1,0,0,1],
    [1,1,0,0,0,1,1,1,0,0,1,0,1,0,0,1], [1,1,0,0,0,1,1,1,0,0,1,0,1,0,0,1], [1,1,0,0,0,1,1,1,1,1,1,0,1,0,0,1], [1,1,0,0,0,1,1,1,1,1,1,0,0,0,0,1],
    [1,1,0,0,0,1,1,1,1,1,1,1,1,0,0,1], [1,1,0,0,0,1,1,1,1,1,1,1,1,1,1,1], [1,1,0,0,0,1,1,1,1,1,1,1,1,1,1,1], [1,1,0,0,0,1,1,1,1,1,1,1,1,1,1,1],
    [1,1,0,0,0,1,1,1,1,1,1,1,1,1,1,1], [1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1]
  ],
  "rooms": [
    [
      { "x": 1, "y": 2, "width": 6, "height": 4, "connections": [[0,1]], "cellx": 0, "celly": 0 },
      { "x": 5, "y": 10, "width": 5, "height": 3, "connections": [[1,1]], "cellx": 0, "celly": 1 },
      { "x": 1, "y": 16, "width": 8, "height": -1, "connections": [[1,2]], "cellx": 0, "celly": 2 }
    ],
    [
      { "x": 14, "y": 1, "width": 7, "height": 4, "connections": [[1,1]], "cellx": 1, "celly": 0 },
      { "x": 17, "y": 10, "width": 4, "height": 2, "connections": [[2,1]], "cellx": 1, "celly": 1 },
      { "x": 17, "y": 15, "width": 4, "height": 0, "connections": [[2,2]], "cellx": 1, "celly": 2 }
    ],
    [
      { "x": 30, "y": 2, "width": 11, "height": 3, "connections": [[1,0]], "cellx": 2, "celly": 0 },
      { "x": 29, "y": 8, "width": 5, "height": 2, "connections": [[2,2]], "cellx": 2, "celly": 1 },
      { "x": 29, "y": 13, "width": 8, "height": 2, "connections": [], "cellx": 2, "celly": 2 }
    ]
  ],
  "connectedCells": [[2,1], [2,0], [1,0], [1,1]],
  "_options": {
    "cellWidth": 3,
    "cellHeight": 3,
    "roomWidth": [3,11],
    "roomHeight": [2,4]
  }
}

This is map array visually: OnPaste 20220314-090948

And this is how it ends up looking in the game: OnPaste 20220314-090919

Please notice:

I assume it happens because I set map height too low; I did not find any mentions of minimum map dimensions in docs though. So, can you please tell me if there's some "safe" threshold that I can use to avoid such issues? Or should I just "post-process" the map, detect areas that are not connected to others and remove them?

ondras commented 2 years ago

Hi @DmitryShashkov,

thanks for reporting the issue. The main problem with the Rogue map generator is that the code is completely contributed and I have zero ideas how does it work.

This is how it all happened: https://github.com/ondras/rot.js/issues/5

Perhaps @hyakugei can give us some insights?

DmitryShashkov commented 2 years ago

Okay, so, from what I've discovered, the issue occurs when rooms have zero or negative height (in my case it was height, but it can also occur with width). This happens here: https://github.com/ondras/rot.js/blob/master/src/map/rogue.ts#L267 (it tries to adjust either offset or room dimensions for room to fit into the map, and there are no checks if roomw / roomh are positive).

So, instead of trying to figure out which dimensions are "safe" to use, I ended up doing this:

This is the code I'm using for checks (in case if someone finds it helpful):

// use `flatten` instead of native `Array.flat()`, cause I'm targeting `es5`
import { flatten } from "lodash";

// I don't describe all room and map fields, only those that are
// actually needed for checks
export interface RotRogueMapRoom {
  width: number;
  height: number;
}

export interface RotRogueMapWithRooms {
  rooms: RotRogueMapRoom[][];
}

// this is kinda "hacky": `rooms` are private in RogueMap, but they are accessible 
// in runtime, because Javascript; I use type guards to be sure object has 
// all the required fields and they are of proper types
export const isRotRogueMapRoom = (room: any): room is RotRogueMapRoom =>
  !!room && typeof room.width === "number" && typeof room.height === "number";

export const isRotRogueMapWithRooms = (map: any): map is RotRogueMapWithRooms =>
  !!map &&
  Array.isArray(map.rooms) &&
  flatten(map.rooms as any[]).reduce(
    (acc, curr) => acc && isRotRogueMapRoom(curr),
    true
  );

// in the end, just pass the map generated by Rot here
export const isMapValid = (map: unknown) =>
  isRotRogueMapWithRooms(map) &&
  flatten(map.rooms).reduce(
    (acc, curr) => acc && curr.width > 0 && curr.height > 0,
    true
  );

Obviously, this is not the perfect solution, but it does the job. I was hoping to update map generation code to avoid such situations completely, but had no success. Also, as I understood, the algorithm is not guaranteed to work properly on any map dimensions, so maybe trying to alter it doesn't make sense at all.

From OOP perspective, it might be proper to put validation method into Rogue class to avoid hacks with reaching private properties. I would propose creating PR with that, but my issue doesn't seem to be common enough to justify that.

Sooo, I guess the problem is solved, I'm closing this ticket now. Thanks @ondras for your time and for the great lib!

hyakugei commented 2 years ago

I know this has been closed, but i just wanted to chime in. Taking a quick look at the code, there is an options object that can be sent in, and if you don't define the roomWidth and/or roomHeight values, they are auto-generated. That auto-generation happens in the _calculateRoomSize() method. You can see that with a height of 16, the "min" value comes out to -- (16/3) * 0.25 == 1.333 -- which is then floored to 1. But the min is never allowed to be less then 2, which means that the system will probably (likely) break where the height is less then 24.

Apologies for the delay in replying, and also for this code -- i wrote it a long time ago, and it could use more edge-case fixes, and better documentation about minimum sizes for map height/width.

Also! the URL for the original "algorithm" is now for a radio station (!?), so here is the wayback machine's text of the page (very short):

By Mark Damon Hughes kamikaze@kuoi.asui.uidaho.edu

The original Rogue algorithm is pretty nifty. Any time you need a random dungeon, give this a try:

  1. Divide the map into a grid (Rogue uses 3x3, but any size will work).
  2. Give each grid a flag indicating if it's "connected" or not, and an array of which grid numbers it's connected to.
  3. Pick a random room to start with, and mark it "connected".
  4. While there are unconnected neighbor rooms, connect to one of them, make that the current room, mark it "connected", and repeat.
  5. While there are unconnected rooms, try to connect them to a random connected neighbor (if a room has no connected neighbors yet, just keep cycling, you'll fill out to it eventually).
  6. All rooms are now connected at least once.
  7. Make 0 or more random connections to taste; I find rnd(grid_width) random connections looks good.
  8. Draw the rooms onto the map, and draw a corridor from the center of each room to the center of each connected room, changing wall blocks into corridors. If your rooms fill most or all of the space of the grid, your corridors will very short - just holes in the wall.
  9. Scan the map for corridor squares with 2 bordering walls, 1-2 bordering rooms, and 0-1 bordering corridor, and change those to doors.
  10. Place your stairs up in the first room you chose, and your stairs down in the last room chosen in step 5. This will almost always be a LONG way away.
  11. All done!

Rogue also has "gone rooms", which just put a corridor space instead of the room, and draws L-shaped corridors instead of straight lines, but those are just flavor.

This algorithm also has the virtues of being extremely fast (even on MUCH bigger grid arrays than 3x3), and guaranteed to succeed.

DmitryShashkov commented 2 years ago

Thanks @hyakugei , this update is quite helpful too!