Quote: Original post by Decrius
But could you explain the following a bit further?

Quote: Original post by NotAYakk
You keep on working on two dimensional coordinates: stop treating them as two unrelated integers. At any of two dozen points you could have mixed up the pairs of coordinates.

struct Coord {  int x;  int y;  Coord(int x_, int y_):x(x_), y(y_){}  Coord():x(), y(){}};

The x and y coordinates are part of one concept. Treat them that way. Passing around an x and a y seperately is begging for bugs and makes your life harder.

While you are at it, you can implement Coord::operator+(const Coord& other), allowing you to add two coordinates together using one line.

struct Coord {  int x;  int y;  Coord(int x_, int y_):x(x_), y(y_){}  Coord():x(), y(){}  Coord operator+(Coord const& other) const {    Coord retval = *this;    retval.x+=other.x;    retval.y+=other.y;    return retval;  }};

Bundle together things that are conceptually bundled together.

Quote:

Quote: Right here:

            Nodes nodes_list;            nodes.push_back(nodes_list);            nodes[nodes.size() - 1].parent = y;            nodes[nodes.size() - 1].parent_node = nodes[y].node;

you just stuck a Nodes object into nodes with an array of uninitialized node objects in it.

Your Nodes object contains an array of Node, whose data isn't initialized in the either constructor of Nodes or in the constructor of Node.

Every object you use should, in general, have a constructor that initializes the memory to zero (well, actually, initialize each object to it's default value). (There are exceptions, but they are exceptions).

C++ will quite happily let you have uninitialized memory in your structs.

Quote:

Quote: Have one canonial map between "index of surrounding node" to "x and y offset to surrounding node". Don't hand-code it.

Here:

Coord GetNeighbourDelta(int num) {  // sort of like:  // 012  // 3x4  // 567  assert(num >= 0);  assert(num <= 7);  if (num>=4)++num;  // now it is like:  // 012  // 345  // 678  // but we skip 4.  Coord retval;  retval.x = (num%3)-1;  retval.y = (num/3)-1;  return retval;}Coord GetNeighbour(int num, Coord base) {  return base + GetNeighbourDelta(num);}

You could even write the reverse map.

Of course, if you only use it in one place, you don't have to turn it into a function.

Quote:

Quote: Other asides: don't store std::vectors in std::vectors until C++09, we don't have efficient move semantics, and vectors are expensive to copy. Use a std::deque of std::vectors.

Do I put vectors in vectors?

Your comments say you do.

Quote:
// when a path is found, this will put the path into 'std::vector<std::vector<Path> > paths' with the differences after each step (so it says: go one left, go one up-right)

Quote: The others ones I've already implented, and it looks a lot better :) (also less function calls -> speed! xD)

First, write it so it works. Pay attention to fundamental slow down issues that you are relying on, but get it working first. Then make it fast.

If you have a slow, working version, at least you can test your attempt at a blazing-fast version by running one then the other and comparing them. :)