Cleanup L5tileFix (drlg_l1.cpp)

[ghost]

Bin exact. Thanks to the beta.

[squidcc]

I have no issue with the style used here other than the fact I'd never be able to get away with it.

[mewmew]

Some good metrics :)

+98 −271

As for whitespace issues, I personally don't care too much about white space. The importance is that the code is readable and does what it should in relation to being binary exact. In a yet to be determined future we will have the code automatically formatted using clang-format (#182). Til then, lets focus on binary exactness.

[mewmew]

Even if it is a massive undertaking, I think we should consider adding enums for the tiles and dungeon pieces, as reading and understanding the code is a lot easier once this is done.

For the tile ID enum of Cathedral, see https://github.com/sanctuary/notes/blob/master/include/enums.h#L926

typedef enum {
	L1_TILE_ID_NONE                                    =   0,
	L1_TILE_ID_WALL_SW                                 =   1,
	L1_TILE_ID_WALL_SE                                 =   2,
	L1_TILE_ID_ARCH_NE_ARCH_NW                         =   3,
	L1_TILE_ID_WALL_SW_WALL_SE = 4,
....
	L1_TILE_ID_BROKEN_ENTERANCE_SE_3                   = 206,
} l1_tile_id;

As a reference, this is the Go version of this function with added enums (ref: https://github.com/sanctuary/djavul/blob/master/d1/l1/l1.go#L1211):

// fixTiles fixes tile IDs of wall edges.
//
// PSX ref: 0x8013EA28
// PSX sig: void L5tileFix__Fv()
//
// ref: 0x40C551
func fixTiles() {
  for yy := 0; yy < 40; yy++ {
    for xx := 0; xx < 40; xx++ {
      switch TileID(gendung.TileIDMap[xx][yy]) {
      case WallSw:
        if TileID(gendung.TileIDMap[xx][yy+1]) == Dirt {
          gendung.TileIDMap[xx][yy+1] = uint8(DirtWallEndSe)
        }
      case WallSe:
        if TileID(gendung.TileIDMap[xx+1][yy]) == Dirt {
          gendung.TileIDMap[xx+1][yy] = uint8(DirtWallEndSw)
        }
      case WallEndSw:
        if TileID(gendung.TileIDMap[xx+1][yy]) == Dirt {
          gendung.TileIDMap[xx+1][yy] = uint8(DirtWallEndSe)
        }
      case Floor:
        switch TileID(gendung.TileIDMap[xx+1][yy]) {
        case WallSe:
          gendung.TileIDMap[xx+1][yy] = uint8(WallEndSe)
        case Dirt:
          gendung.TileIDMap[xx+1][yy] = uint8(DirtWallSw)
        }
        switch TileID(gendung.TileIDMap[xx][yy+1]) {
        case WallSw:
          gendung.TileIDMap[xx][yy+1] = uint8(WallEndSw)
        case Dirt:
          gendung.TileIDMap[xx][yy+1] = uint8(DirtWallSe)
        }
      }
    }
  }

  for yy := 0; yy < 40; yy++ {
    for xx := 0; xx < 40; xx++ {
      switch TileID(gendung.TileIDMap[xx][yy]) {
      case WallSw:
        switch TileID(gendung.TileIDMap[xx][yy+1]) {
        case WallSe:
          gendung.TileIDMap[xx][yy+1] = uint8(WallEndSe)
        case Floor:
          gendung.TileIDMap[xx][yy+1] = uint8(ArchEndNe)
        }
      case WallSe:
        switch TileID(gendung.TileIDMap[xx+1][yy]) {
        case WallSw:
          gendung.TileIDMap[xx+1][yy] = uint8(WallEndSw)
        case Floor:
          gendung.TileIDMap[xx+1][yy] = uint8(ArchEndNw)
        case DirtWallSe:
          gendung.TileIDMap[xx+1][yy] = uint8(DirtWallSwWallSe)
        case DirtWallEndSe:
          gendung.TileIDMap[xx+1][yy] = uint8(DirtWallSwWallSe)
        }
      case ArchNeArchNw:
        if TileID(gendung.TileIDMap[xx+1][yy]) == Dirt {
          gendung.TileIDMap[xx+1][yy] = uint8(DirtWallEndSe)
        }
      case WallSwWallSe:
        if TileID(gendung.TileIDMap[xx+1][yy]) == ArchEndNe {
          gendung.TileIDMap[xx+1][yy] = uint8(ArchEndNw)
        }
      case WallEndSw:
        switch TileID(gendung.TileIDMap[xx-1][yy]) {
        case Dirt:
          gendung.TileIDMap[xx-1][yy] = uint8(DirtWallEndSe)
        case DirtWallEndSw:
          gendung.TileIDMap[xx-1][yy] = uint8(DirtWallSwWallSe)
        }
        switch TileID(gendung.TileIDMap[xx][yy+1]) {
        case WallSe:
          gendung.TileIDMap[xx][yy+1] = uint8(WallEndSe)
        case Floor:
          gendung.TileIDMap[xx][yy+1] = uint8(ArchEndNe)
        case DirtWallSw:
          gendung.TileIDMap[xx][yy+1] = uint8(DirtWallSwWallSe)
        case Dirt:
          gendung.TileIDMap[xx][yy+1] = uint8(DirtWallEndSe)
        }
        if TileID(gendung.TileIDMap[xx][yy-1]) == Dirt {
          // NOTE: The following value is always overwritten.
          //gendung.TileIDMap[xx][yy-1] = uint8(WallEndSe)
          gendung.TileIDMap[xx][yy-1] = uint8(DirtWallEndSe)
        }
      case WallEndSe:
        switch TileID(gendung.TileIDMap[xx+1][yy]) {
        case WallSw:
          gendung.TileIDMap[xx+1][yy] = uint8(WallEndSw)
        case Floor:
          gendung.TileIDMap[xx+1][yy] = uint8(ArchEndNw)
        case DirtWallSe:
          gendung.TileIDMap[xx+1][yy] = uint8(DirtWallSwWallSe)
        case Dirt:
          gendung.TileIDMap[xx+1][yy] = uint8(DirtWallEndSw)
        case DirtWallEndSe:
          gendung.TileIDMap[xx+1][yy] = uint8(DirtWallSwWallSe)
        }
        if TileID(gendung.TileIDMap[xx][yy-1]) == DirtWallEndSe {
          gendung.TileIDMap[xx][yy-1] = uint8(DirtWallSwWallSe)
        }
      case Floor:
        switch TileID(gendung.TileIDMap[xx+1][yy]) {
        case DirtWallSe:
          gendung.TileIDMap[xx+1][yy] = uint8(DirtWallSwWallSe)
        case Dirt:
          gendung.TileIDMap[xx+1][yy] = uint8(DirtWallNeWallNw)
        case DirtWallEndSe:
          gendung.TileIDMap[xx+1][yy] = uint8(DirtWallSwWallSe)
        }
        if TileID(gendung.TileIDMap[xx][yy+1]) == ArchEndNe {
          gendung.TileIDMap[xx][yy+1] = uint8(ArchEndNw)
        }
      case DirtWallSw:
        if TileID(gendung.TileIDMap[xx][yy+1]) == WallSe {
          gendung.TileIDMap[xx][yy+1] = uint8(WallEndSe)
        }
        if TileID(gendung.TileIDMap[xx][yy-1]) == DirtWallEndSe {
          gendung.TileIDMap[xx][yy-1] = uint8(DirtWallSwWallSe)
        }
      case DirtWallSe:
        switch TileID(gendung.TileIDMap[xx+1][yy]) {
        case WallSw:
          gendung.TileIDMap[xx+1][yy] = uint8(WallEndSw)
        case Dirt:
          gendung.TileIDMap[xx+1][yy] = uint8(DirtWallNeWallNw)
        }
        if TileID(gendung.TileIDMap[xx-1][yy]) == DirtWallEndSw {
          gendung.TileIDMap[xx-1][yy] = uint8(DirtWallSwWallSe)
        }
      case DirtWallSwWallSe:
        if TileID(gendung.TileIDMap[xx+1][yy]) == WallSw {
          gendung.TileIDMap[xx+1][yy] = uint8(WallEndSw)
        }
        if TileID(gendung.TileIDMap[xx][yy+1]) == WallSe {
          gendung.TileIDMap[xx][yy+1] = uint8(WallEndSe)
        }
      case DirtWallEndSw:
        if TileID(gendung.TileIDMap[xx-1][yy]) == Dirt {
          gendung.TileIDMap[xx-1][yy] = uint8(DirtWallSe)
        }
      }
    }
  }

  for yy := 0; yy < 40; yy++ {
    for xx := 0; xx < 40; xx++ {
      switch TileID(gendung.TileIDMap[xx][yy]) {
      case WallSe:
        if TileID(gendung.TileIDMap[xx+1][yy]) == DirtWallSe {
          gendung.TileIDMap[xx+1][yy] = uint8(DirtWallSwWallSe)
        }
      case WallSwWallSe:
        if TileID(gendung.TileIDMap[xx][yy+1]) == WallSe {
          gendung.TileIDMap[xx][yy+1] = uint8(WallEndSe)
        }
      case DirtWallSw:
        if TileID(gendung.TileIDMap[xx][yy+1]) == Dirt {
          gendung.TileIDMap[xx][yy+1] = uint8(DirtWallNeWallNw)
        }
      }
    }
  }
}

[AJenbo]

I think it would be nice if you could align a bit more on the code style already at this point. The debug information we have only tells us about lines (maybe except for online assert strings), so for spacing we should either go with something that feels realistic or makes us happy.

[mewmew]

I think it would be nice if you could align a bit more on the code style already at this point.

@AJenbo was this comment for @galaxyhaxz? (and not the code example I posted above)

[AJenbo]

Wasn't specifically for him, but it was about code style and not what you posted 😊

[ghost]

This function should have 64 lines according to the PSX. The only way to do that is to put the if(tile)\r\n tile = x on the same line. It wouldn't look bad now but later on when we add the enums it would span really wide (and you have to keep in mind they likely wrote this code on a 640x480 monitor).

Not sure what else there is to complain about, it's three simple for loops using the same variables as the PSX and generates identical...