What this code is supposed to do?

[Krice]

It's from Abura Tan which I'm trying to rewrite to SDL. I'm puzzled by this:

Code: [Select]

for (y = 0; y < map_height; ++y)
{
for (x = 0; x < map_width; ++x)
{
      if
(
(x < map_width-1 && mapArr[y][x+1] == river) &&
(
(y < map_height-1 && mapArr[y+1][x] == river) &&
(x < map_width-1 && y < map_height-1 && mapArr[y+1][x+1] != river) ||
(y > 0 && mapArr[y-1][x] == river) &&
(x < map_width-1 && y > 0 && mapArr[y-1][x+1] != river)
)
)
      mapArr[y][x] = river;
}
}

Why check dimensions, why not use a loop with x=1 to map_width-1? What's the purpose of this generally, it's used after the actual river generation. I guess it does something to rivers.

[Perdurabo]

At first glance, it looks like some sort of smoothing function, to fill in additional river tiles?

At second glance, its atrociously written and gives me a headache just looking at it.

[purestrain]

Hmm; there are just checks for tiles with content "river" and map border checks... could be rewritten

[Krice]

Hmm; there are just checks for tiles with content "river" and map border checks...

I know that! Look at the routine. I can't figure out the order of checking and what kind of result it makes.

[Perdurabo]

Assuming the origin is at top left.

C is the cell bring checked. r is river tile, L is non-river tile (call it Land)

It checks for either of these cases:


     C r
     r L

     r L
     C r



and if found sets the Cell C to river

I think. My C is a tad rusty.

It looks as if it has been written to ensure rivers are more than 1 cell wide on the diagonal?

[Krice]

It looks as if it has been written to ensure rivers are more than 1 cell wide on the diagonal?

Something like that. Maybe I should try to leave it out and see what happens. But first I have to make a map viewing routine for SDL using large pixels, so I can really see what the level generation is doing.

[Krice]

Here is another one. That return part. get()->getTile(). What is that?

Code: [Select]

Target Map::getTile( const Point3d &world_loc ) {

  assert( limits.contains( world_loc.toPoint() ) );

  return get( Point3d( world_loc.getx() / lev_width,
                       world_loc.gety() / lev_height,
                       world_loc.getz() )
             )->getTile(world_loc);
  }


[Ex]

Here is another one. That return part. get()->getTile(). What is that?

Code: [Select]

Target Map::getTile( const Point3d &world_loc ) {

  assert( limits.contains( world_loc.toPoint() ) );

  return get( Point3d( world_loc.getx() / lev_width,
                       world_loc.gety() / lev_height,
                       world_loc.getz() )
             )->getTile(world_loc);
  }


Returns the map tile at the given location. Could return a struct, class, or perhaps just a good ol' char. For instance, this may return '.' or '#' if it returns characters. Specifically, the function get seems to return a pointer to a class which has another function inside of it (getTile), which should return the map tile at the given location.

[purestrain]

I don't know you problems, krice, the source code is so simple...

  The methods calls for a LevelHandle ( i think the map is composed of several levels, aligned in a grid). The LevelHandle has a overriden -> operator which returns the actual level where you get a specifc target...  LevelHandle is supposed as a proxy which provides some sort of streaming functionality, so i could on the fly load a concrete level (save/load)

ok, to be honest: the source is not that good and simple... even overriding the "->" operator is the hellish (of course not if you want to implement some kind of smart pointer) if you don't take care. Of course in this case levelhandle is almost transparent.

but some architectural decisions look quite good 

but method naming....
instead of "getTile" it should be called "getTargetAt".... nice looking at that good, i think you'll have lots of fun

[purestrain]

After some more inspecting: it seems like the original author knows his c++ skills :-) Some parts are actually really good designed, but not easy to understand at a first glance.

[Ex]

overriding the "->" operator

It looks much more like a pointer to a class/struct with a function inside to me. For instance:

Code: [Select]

class A
{
   A();
  ~A();
  int getTile(int InputAndStuff);
};

A *get(int OriginalInputAndStuff);

...

return get(0)->getTile(1);

...


[purestrain]

Yes, at the first sight, but actually the returned object of get() has the pointer operator overriden. <surpriiiise>

[Krice]

Some parts are actually really good designed, but not easy to understand at a first glance.

I think it's badly written source code. Why make everything such a complex way?

[purestrain]

Its not that bad. The map class is the entire world and it passes the actual request for a target at a specific (world) coordinate to the proper level and its levelcoordinates. Between level and map is another layer which seems to do some manager stuff. To get the right levelhandle the world coordinates have to be divided by level-size. Easy and nice, almost what i would do.

[corremn]

Why mess around with code that has nothing to do with screen output?  You are just converting it to sdl.  And besides that code is pretty logical.