Implementing the restructure changes

[PowerWyrm]

Lol... I compile the code, start a caster, play a bit around to see if the cave generation changes work and everything goes fine... until I try to cast a spell, and nothing happens. Seems that the projection code at this point doesn't work... Need to check how this was fixed later on

[PowerWyrm]

In the original code, I've counted over 1000 occurences of "cave_get" calls. After a day of reviewing, I'm down to 383 occurences, and I've still half the code to review. Hopefully, this will help a ton when levels get dynamically allocated.

Down to 197 occurences. I've left aside the spell code, because I don't want to rewrite the whole code (and it's never called during dungeon generation), but this is still impressive.

[PowerWyrm]

Yeah I'm gonna use that array instead of the cave array, which should simplify things a bit. However at this point in the code (around 03/14), the "cave" variable is still used broadly in V, and this global variable is statically allocated, so V behaves like the old code in my variant. If the "cave" variable was dynamically allocated, all the functions that use it would also crash like in my variant. That's why I'm going to review my code and pass a "cave" pointer every time I can instead of getting the pointer from the player's depth and the cave array. This should not only fix the crashes, but also make the code more readable.

In the original code, I've counted over 1000 occurences of "cave_get" calls. After a day of reviewing, I'm down to 383 occurences, and I've still half the code to review. Hopefully, this will help a ton when levels get dynamically allocated.

[MattB]

I'm absolutely hooked on this thread - even though there hasn't been a full sentence that I've actually understood. I love Angband.

[PowerWyrm]

Have you seen yet that V now has an array of generated levels too? It's called the chunk_list, and so far it only saves the town and restores it, but I imagine you could use it.

I don't think that helps with your current problem, though.

Yeah I'm gonna use that array instead of the cave array, which should simplify things a bit. However at this point in the code (around 03/14), the "cave" variable is still used broadly in V, and this global variable is statically allocated, so V behaves like the old code in my variant. If the "cave" variable was dynamically allocated, all the functions that use it would also crash like in my variant. That's why I'm going to review my code and pass a "cave" pointer every time I can instead of getting the pointer from the player's depth and the cave array. This should not only fix the crashes, but also make the code more readable.

[Nick]

Currently implementing the new dynamic cave generation... and at this point I'm suffering from the fact that my variant has an array of generated levels, and all generation routines have been modified to take a player pointer parameter instead of a cave pointer parameter.

Have you seen yet that V now has an array of generated levels too? It's called the chunk_list, and so far it only saves the town and restores it, but I imagine you could use it.

I don't think that helps with your current problem, though.

[PowerWyrm]

Currently implementing the new dynamic cave generation... and at this point I'm suffering from the fact that my variant has an array of generated levels, and all generation routines have been modified to take a player pointer parameter instead of a cave pointer parameter.

Old code:

function foo_inner(player)
{
  cave = cave_get(player)
  play_with(player, cave)
}

function foo_generate(player)
{
  foo_inner(player)
}

With the new system, the "cave" pointers are generated "on the fly", so most of the generation functions take a "cave" parameter:

function foo_inner(player)
{
  cave = cave_get(player)
  play_with(player, cave)
}

function foo_generate(player, new_cave)
{
  foo_inner(player)
}

The problem here is that "new_cave", the newly generated cave, is the only pointer defined during cave generation. So the code in foo_inner() crashes because at this point, the "cave" pointer is NULL. So I need to browse all the code and add a "cave" parameter to all functions that need to access the cave array and that are called during cave generation:

function foo_inner(player, cave)
{
  play_with(player, cave)
}

function foo_generate(player, new_cave)
{
  foo_inner(player, new_cave)
}

I bet this is going to be fun...

[AnonymousHero]

Sorry. It gets better...

QFT. (Lollerskates.) PowerWyrm, I'm sorry about the pain, but... yeah, it doesn't get much better from here.

[Nick]

This is really going slowly. Still more than a year of restructure changes to implement...

Sorry. It gets better...

[PowerWyrm]

I've implemented everything up to the command framework rewrite. Now everything crashes during dungeon generation/deallocation.

1) Dungeon generation crashes when creating a trap due to bad comparison with z_info->l_max instead of cave->trap_max and using a non-standard way of allocating a new trap. Temporary fix: code a trap_pop() method similar to o_pop and mon_pop and use that instead. Fixed in recent V with linked list.

2) Dungeon deallocation crashes when there's a mimic (undiscovered) on the level due to the fact that the object linked to the mimic has already been wiped and we try to delete it again with a bad index. Temporary fix: call wipe_mon_list before wipe_o_list. Fixed in recent V (dunno how, but summoning a mimic and leaving the level didn't crash).

3) Dungeon generation crashes with labyrinths and caverns due to an assertion on cave->height/width because level is filled with permawalls after setting height and width. Temporary fix: fill the level before setting dimensions. Fixed in recent V with cave_new having height/width parameters.

This is really going slowly. Still more than a year of restructure changes to implement...

[PowerWyrm]

Nevermind. I've made more tests and the dungeon generator seems fine. There is just a small problem with some vaults that have no "outer" walls such as #6 (turnabout) and #8 (central), which are now disconnected from the rest of the level. Adding outer walls like for all other vaults fixes the problem.

It would be probably a good thing to redraw all the rooms and vaults to allow outer walls only near the entrances of the vaults/rooms. Sometimes a huge vault has only one entrance made of granite and you have to dig 50+ squares to reach it because the tunnel was generated on the other side of the vault.

[PowerWyrm]

Looking at the code, I think there's a problem with how the old PERM_INNER walls are now handled. PERM_INNER (permanent walls inside vaults) are treated like all other permanent walls and skipped when building a tunnel; this makes escaping a huge vault a challenge for the dungeon generator; fix: add a SQUARE_WALL_INNER flag to the wall and accept SQUARE_WALL_INNER permanent walls when building a tunnel.

[PowerWyrm]

Nevermind. I've made more tests and the dungeon generator seems fine. There is just a small problem with some vaults that have no "outer" walls such as #6 (turnabout) and #8 (central), which are now disconnected from the rest of the level. Adding outer walls like for all other vaults fixes the problem.

[Nick]

Finished implementing the SQUARE_WALL_INNER/OUTER/SOLID flags for dungeon generation. I need to check vs the old dungeon generator, but I think the method that builds the tunnels is broken with the new system. I've seen many times tunnels "eating" part of rooms, especially corners/outer walls. No idea what is the cause though...

Have you got all the updates? IIRC I played with some different tunnel and layout generation before settling back to the original.

[PowerWyrm]

Finished implementing the SQUARE_WALL_INNER/OUTER/SOLID flags for dungeon generation. I need to check vs the old dungeon generator, but I think the method that builds the tunnels is broken with the new system. I've seen many times tunnels "eating" part of rooms, especially corners/outer walls. No idea what is the cause though...