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.33k stars 254 forks source link

ROT.Map.Cellular and the "connected" option #86

Closed atomdmac closed 8 years ago

atomdmac commented 8 years ago

Hi there!

I was recently looking at some old 7DRL submissions and came across the code for Beware of the Space Bears (which uses the ROT lib). I noticed that the author was using an undocumented option for the Cellular dungeon generator called "connected". When I looked at the Cellular generator code, I did see evidence that this has been implemented (though is not documented).

My assumption (based on the comments in the ROT code) is that this is meant to connect all parts of the generated "cave". However, I can't seem to get it to work...

Can someone tell me more about this feature and why it's currently undocumented? Is it known to be broken or might I be misusing it?

Thanks!

kosinaz commented 8 years ago

I was also experiencing with it after its addition, and if I remember correctly it works only with the first generation. And unfortunately it works the opposite way I wanted, because it connects the "dead" not the "alive", so it creates walls in the caves, not tunnels between them. So without modification of the code I had to create the caves the way I like through some generation, then load the inverse of the generated data into a second generator to connect the caves. If you need more help, and the author is not around, I can check it for you.

ondras commented 8 years ago

Hi,

this is a contributed code: https://github.com/ondras/rot.js/pull/34

I have no idea how it works; it is missing documentation, tests and a proper section in the interactive manual :/

But looking at https://github.com/ondras/rot.js/blob/master/src/map/cellular.js#L266, the algo apparently considers "free" those cells whose value is not 1, which is consistent with the "0 = all empty, 1 = all full" comment at https://github.com/ondras/rot.js/blob/master/src/map/cellular.js#L28.

Also, the "connect all spaces" feature is applied at the end of the create function, which implies connecting after each iteration. You can selectively {en,dis}able this via the setOptions call, though.

atomdmac commented 8 years ago

Paging @uzudil

Have you experienced this issue? Are we using the code wrong?

uzudil commented 8 years ago

Hi,

Ondras is correct, changing the _freeSpace function to look for value != 0 does create connected maps, however it fails to generate in some conditions. I will see if I can create a patch to fix this (and maybe add documentation this time.)

Thanks, --Gabor

atomdmac commented 8 years ago

@uzudil Thank you very much for taking a look! This feature would be extremely useful for some of my current projects.

uzudil commented 8 years ago

Actually after some more testing - this feature appears to be working for me. For example:

        var w = 80, h = 40;
        var map = new ROT.Map.Cellular(w, h, { connected: true });

        /* cells with 1/2 probability */
        map.randomize(0.5);

        /* generate and show four generations */
        for (var i=0; i<4; i++) {
            map.create();
        }

        var display = new ROT.Display({width:w, height:h, fontSize:4});
        document.body.appendChild(display.getContainer());
        map.create(display.DEBUG);

Produces a map whose black (empty, ie. 0) values can all be reached.

atomdmac commented 8 years ago

Perhaps I misunderstood... The examples for the other map generators appear to show the white cells as passable/empty. @ondras Is the Cellular generator different for some reason?

uzudil commented 8 years ago

Ah that's probably the issue. Let me know if that is the case and I will submit a pull request with the fix.

ondras commented 8 years ago

Hmm, @suicidepills, can you please provide a particular example? A quick look into the features.js reveals that 0 is supposed to represent an empty space while 1 is supposed to represent a wall. This sounds consistent with the cellular generator... perhaps I am overlooking something.

atomdmac commented 8 years ago

@uzudil @ondras D'oh! You guys are both right. I just went back to look at the examples for the other generators in the interactive manual and sure enough, the tiles displayed in black are empty spaces.

I had subconsciously associated "black" with walls/void in my head. My bad!

With that said, it looks like the "connected" feature works as expected (huzzah!) I was just using the map generators backwards! I think we can close this.

Thanks again to everyone for their help (and my apologies for the confusion)!

kosinaz commented 8 years ago

I think I know what caused the confusion.

The interactive manual says that the default born and survive options are set according to this Roguebasin article. This is why the default fourth generation looks like a cave. But if I understand correctly, in the original article the inner parts are the floor tiles, and the outer ones are the walls. And I think in this way it is more useful.

But in rot.js it is the complete opposite. So you can use the original method not to generate caves, but lakes, because you can walk around them on the edge of the map. Sure you can make a create callback to swap the floor and wall tiles, but you can't do that with the connected option. That means you can use the connected option to build bridges into the islands of the lakes, but can't dig tunnels between the separated caves.

So an option would be nice to decide if we want to connect 0 or 1 tiles. I hope I was understandable.

atomdmac commented 8 years ago

@kosinaz Both the "cavern" and "lake" results seem like they could be pretty useful.

@ondras and @uzudil What do you guys think about the merits of being able to choose between connecting 0s vs 1s with a parameter/option? If you think it would be useful (and if I can get some time this weekend), I could try my hand at it.

ondras commented 8 years ago

I agree that both approaches are reasonable, so having a way to specify 0/1 is necessary.

But in general, this feature ("connect them!") might be moved away from the creation step, being a separate function (of the Cellular object, or something else). This would have the advantage of

  1. being able to create -> swap -> connect
  2. being able to connect non-cellular maps

What do you think?

uzudil commented 8 years ago

Yes I agree with making this a post-creation step. It will speed up map generation. Making it accept a wall-vs-lake parameter is also a good idea.

suicidepills, let me know if you want me to do this, or if you feel like it, go for it.

kosinaz commented 8 years ago

I agree with the decoupling of the room-connector tool.

By the way I had an other idea when I first used the connected option. Usually the result of the cellular generation is some caves connected with some tight tunnels. So the caves work like rooms, and this way the result is a kind of dungeon. But for the dungeons tunnels and rooms are defined, stored and can be used separately and you can generate doors as well. And this is what I would like to use if a connector tool is made:

Get the endpoints of the generated tunnels, so I can put doors on them.

What do you think? Is it possible to return the connection points?

atomdmac commented 8 years ago

@uzudil I'm not currently very familiar with your code so you may be able to bang it out faster than I could. You may also have a better idea of how to facilitate the adding of doors (as @kosinaz describes above). Either way, I'm happy to help in whatever capacity I can.

uzudil commented 8 years ago

Sounds good, I will write a patch for this in the next few days. I'll add some way (possibly a callback) to be notified when an extra connection is made.

@kosinaz I think just looking at the final map for good places for doors may be a better way to go. There could be generated areas suitable for doors already, even without making connections. Anyway, I'll let you know when I have the code for this.

uzudil commented 8 years ago

A new connect() method has been merged in. You'd call it after the last create(). It runs with no arguments, or optionally, you can send it:

See the manual for examples. Enjoy!

atomdmac commented 8 years ago

Thanks, @uzudil ! I apologize for the recent radio silence. Life has gotten pretty noisy within the last week so I haven't had a chance to play around with this. Hopefully I'll be able to take a closer look within the next few days. Thank you for setting aside some time to work on this :)

atomdmac commented 8 years ago

I've finally had some time to play around with this and it works very well for my purposes. I've put together a quick CodePen for anyone else that would like to toy around with it, too.

http://codepen.io/atommac/pen/EKyJGw

uzudil commented 8 years ago

Great thanks! I'm glad it worked out for you.