Terrain generation#79
Conversation
|
14 consecutive generations with fn7 calibrated layer integrated through yaml file |
dbde94d to
ac0c6c8
Compare
sasmith
left a comment
There was a problem hiding this comment.
I started by reviewing mettagrid/map/scenes/simplex_sampler.py, since it seems like it's at the heart of this diff. The code needs to be more understandable before I think I'll be able to provide the review at the level I want to. I've left comments that hopefully point you in the right direction, and I've asked Slava if he can help you here as well.
I'm excited about the noise generators you're using, and the speed of map generation you've achieved, so I hope we'll be able to get this to a place where we'll be able to use it.
| player/node_modules | ||
| /multirun | ||
| .coverage | ||
| poetry.lock |
There was a problem hiding this comment.
Can you say more? By default, I don't think we want to ignore this -- I don't think we're using poetry.lock, but on principle, we should be.
| The functions take in parameters such as x and y coordinates, width and height of the terrain, and various other parameters that control the noise generation process. | ||
| ''' | ||
|
|
||
| def fn0(x,y, width, height, |
There was a problem hiding this comment.
These functions should be less mysterious. This would include:
- Giving them more descriptive names
- Providing some sort of documentation as to what they add to the output, and how their parameters impact this. (e.g., "this makes the noise more 'swirly'. The direction and tightness is controlled by foo. Bar controls how consistent the swirliness is each rotation.")
Generally I'd prefer for defaults to be in configuration rather than function definitions (defaults values like None are typically better in code than values like 0.1, but it's fuzzy). It's hard for me to be sure what makes sense in this case, but if we're going to have constants in code, we should have some documentation around why these are appropriate defaults.
There was a problem hiding this comment.
default values sort of represent the least distorted version of a sampling function, I've added them for myself to not forget what parameters generate the "normal" result, I've added description for those reasons in now separate file with all sampling functions
| import time | ||
| ''' | ||
| This file contains a set of functions that can be used to generate different types of noise patterns for terrain generation. | ||
| The functions are designed to be used with the OpenSimplex noise generator, and they can be combined in various ways to create complex terrain features. |
There was a problem hiding this comment.
(My opinion here may change based on documentation changes elsewhere in the file)
This should come with examples of how the functions can be used / composed. Pointing to example yaml files may suffice.
| wall_size: int = 1, | ||
| seed: MaybeSeed = None, | ||
| children: list[Any] = [], | ||
| layers: list[Layer] = [], # layers dictate how generated noise is sampled and how the end result will look |
There was a problem hiding this comment.
This is a helpful breadcrumb, but (as per all my other comments), I think this should be expanded. Like, how do they dictate how the noise is generated? How does this interaction with children?
berekuk
left a comment
There was a problem hiding this comment.
I'm still figuring out your fix_map algorithm, will comment on it separately.
|
|
||
| children: | ||
| - scene: | ||
| _target_: mettagrid.map.scenes.simplex_sampler.SimplexSampler |
There was a problem hiding this comment.
I'd recommend moving these scenes to separate files. When children[*].scene is a string, it's interpreted as a filename, e.g. ./configs/scenes/simplex/arbitrarily_tiled_lattice.yaml.
Then it would be decoupled from this specific 3x3 RoomGrid layout.
…pling functions + 2 scenarios for cognitive evals: maze and haul + small ASCII mazedrawing script
…older version and connects map closer to the terrain form and structure
| if out_of_bound(next_pos[0], next_pos[1]): | ||
| continue | ||
| if terrain[next_pos[0]][next_pos[1]] <= self.cutoff: | ||
| current_energy = ( |
There was a problem hiding this comment.
This would be more readable if you modified energy instead of introducing the new variable.
There was a problem hiding this comment.
I renamed them to be more readable, I think you can use one variable, but it will make it less intuitive
| distances[current][0] = 0 | ||
| terrain[int(current[0])][int(current[1])] = 255 # type: ignore I don't know why it shows warnings there, but it works | ||
| front.put((current_energy, [next_pos, current])) | ||
| front.put((current_energy, [current, previous])) |
There was a problem hiding this comment.
I very strongly suspect that the previous 3-4 lines are not necessary because you repeat the same thing in the loop below. Can you find how to get rid of the duplicate code?
|
|
||
| distances[next_pos] = [current_energy, current] # type: ignore | ||
| front.put((current_energy, [next_pos, current])) | ||
| seen.add(next_pos) |
There was a problem hiding this comment.
General thoughts on this algorithm:
-
The idea is very good, I like it.
-
The implementation could be clearer, see comments above.
- I've been thinking how to adapt it to
MakeConnected(which has several issues with it).
I'm pretty sure that the integer terrain could be adapted somehow (and could even be useful - right now MakeConnected treats walls and previously placed objects as having the same weight, and it could be good to prioritize one or another for removal).
But another concern is that MakeConnected has randomness, and that's a nice thing to have.
If want to compare them for yourself, you can check out configs/game/map_builder/mapgen_bsp_plus_connected.yaml as an example, or make your own config that calls RoomGrid with a very fine grid of isolated 1x1 cells, and then call MakeConnected on top of it. MakeConnected works decently on top of BSP; it deletes almost everything from 1x1 RoomGrid because it connects everything to a single area.
OTOH, your algorithm removes the minimal amount of walls, but it turns 1x1 RoomGrid into a boring comb-like shape (I tried it - copy-pasted your code and converted the grid to the terrain and then back). The perfect approach, I think, would produce something very maze-like. Maybe shuffling the directions in your code would be enough, but I'm not sure.
Anyway, this part is not necessary for this PR to go in, just something to keep in mind for the future.



































Terrain generation based on simplex noise and guided by arbitrary sampling functions + 2 scenarios for cognitive evals: maze and haul + small ASCII mazedrawing script