Advertisement

Pathfinding, need feedback

Started by April 30, 2020 05:52 PM
109 comments, last by Calin 4 years, 6 months ago

Calin said:
let me finish the algorithm first, then I`ll see how I can reduce the amount the code.

This is like saying: ‘Let me drive the kids to school first, then i'll take the first lesson for my driving license. I know i asked for your help and advise, and now i ignore it all. But need to get the job done first - i know what i'm doing!’ :/

JoeJ said:
i asked for your help and advise, and now i ignore it all

I asked for general supervision. The approach to make less code is optional. I can make the difference between crucial/mandatory and optional code.

My project`s facebook page is “DreamLand Page”

Advertisement

Calin said:
I asked for general supervision.

Trying to read your code gives me eyestrain, but it looks what you try to do is similar to this:

https://www.geeksforgeeks.org/boundary-fill-algorithm/

When drawing a new pixel, add to that pixel the information which pixel has drawn it. Then, when the first pixel hits the goal, track back this information to get a path to the initial source.

But this won't be the shortest path. To get that, you must ensure you draw pixels in order of increasing path length from the source. Priority queue is one option to achieve this (but not a very good one):

https://www.geeksforgeeks.org/dijkstras-shortest-path-algorithm-using-priority_queue-stl/?ref=rp​​

Calin said:
The approach to make less code is optional.

If you work on something you do not yet know how it should work, badly cluttered code will slow you down much more than if you knew.

I wrote some shortest paths code in a grid for another thread here a while back, but not sure if it's free of bugs or good at all:


			int data [8*8] = {
				1,1,1,1,1,1,1,1,
				1,0,0,0,0,7,0,1,
				1,0,0,0,0,0,0,1,
				1,0,0,1,1,1,1,1,
				1,0,0,1,0,9,0,1,
				1,0,0,1,0,0,0,1,
				1,0,0,0,0,0,0,1,
				1,1,1,1,1,1,1,1};

			int start = 0, target = 0;
			for (int i=0; i<64; i++)
			{
				int x = data[i];
				if (x==7) start = i;
				if (x==9) target = i;
				data[i] = (x == 1 ? 1000 : 999);
			}

			int previous[64] = {};
			int stack[64];
			stack[0] = target;
			data[target] = 0;
			int tail = 1;

			for (int i=0; i<tail; i++)
			{
				int c = stack[i];
				int distC = data[c];
				int distN = distC + 1;

				//if (c==start) break; // optional early out if we want only one path

				int x = c&7;
				int y = (c>>3);
				for (int j=0; j<4; j++)
				{
					int n = c;
					switch (j)
					{
						case 0: n++; break;
						case 1: n--; break;
						case 2: n+=8; break;
						default: n-=8;
					}
					if (data[n] == 1000) continue;
					
					if (distN < data[n])
					{
						data[n] = distN;
						previous[n] = c;
						stack[tail++] = n; 
					}
				}
			}

			char output[8][9] = {};

			for (int y=0; y<8; y++) for (int x=0; x<8; x++) 
			{
				output[y][x] = data[y*8+x] == 1000 ? 'X' : ' ';
			}

			int path = start;
			while (path)
			{
				output[path>>3][path&7] = '@';
				path = previous[path];
			}

			for (int y=0; y<8; y++) 
			{
				SystemTools::Log(output[y]);
				SystemTools::Log("\n");
			}
ouput:

XXXXXXXX
X    @ X
X @@@@ X
X @XXXXX
X @X@@ X
X @X@  X
X @@@  X
XXXXXXXX

JoeJ said:
what you try to do is similar to this

No it`s all good, tomorrow I`ll update my code, I need a notice when I go offcourse. The thread was going well just that I`m slow with updates.
I guess I need someone who can read my code and see the pitfalls

My project`s facebook page is “DreamLand Page”

Yes. Not only others but yourself too must read easy your own code.
And, @calin , you should not think that it is about length of the code.
It has nothing to do with the length of the code.
It has to do with maintainability as @sillycow suggested.

What is more, you need to “shape the functionality” of the code.
A loop could represent a gear inside your machine. A function could represent a spring in your mechanism.
Basic programming tools, like functions, arrays, for and while loops, things that became an universal standard in the past 60 years are the gears in your machinery.(Then it comes OOP, FP and others)

If you can not “shape the functionality” of the code, then you probably have no idea what your own algorithm is doing. That's why i didn't read your code. It is meaningless for me to figure out something that you yourself might be not knowing how it works. We all aim for productivity. It is not productive for me to read your code.

I know you are interested in ASM, but your code is bad even for ASM. Even in ASM, the code would look more like the code of @alberth than your “stream of statements”-like code.

And finally, do your homework if you want people to help you.

Calin said:
I guess I need someone who can read my code and see the pitfalls

Calin said:
I need a notice when I go offcourse.

Sure man, we have time for this and are just waiting to give a helping hand when you're ready. And until then we'll study your code so we can assist, while you are unable (or just too lazy?) to explain how your algorithm is intended to work, or what's wrong with it.
Probably you are oh so clever and never need to study known solutions to ancient problems - you just reinvent that wheel from scratch with ease, while we are only good enough to fix your tiny bugs because we did our homework only to help you out.

… that's how it feels. You should try to ask specifically, or just commit you have no idea at all. Speaking in one liner metaphors rarely gives us any feedback about if you understood what we tried to say or not, or if you are interested at all.
I can imagine why this isn't that easy and i don't mean it bad.
So, increasing your average count of lines per post might be more important than decreasing lines of code ; )

Advertisement

Calin said:
I need a notice when I go offcourse. The thread was going well just that I`m slow with updates. I guess I need someone who can read my code and see the pitfalls

You may be thinking that writing code is what programming is about, but it is not. Writing code is the simplest and to me the most boring part of programming.

The real thing we're doing in programming is to flesh out a thorny problem like “path-finding”, and flesh it out in so much detail that something stupid like a box of silicon can do the work.

In programming, you define wanted properties of the solution to the problem, figure out data structures and rules about those data structures, and steps to perform in the computation, so anyone (ie not just you) can understand that the solution is indeed solving the original problem.

So far you are just dumping code (the solution, it's “42”!), and not telling anything about real or desired properties or about data structure rules or about high-level view of computation steps ("what is the question about life, the universe, and everything else"?).

That's fine, except any high-level problem has infinitely many possible solutions, so for us there is no way to understand which solution you are exactly aiming for, and thus no way to see you going in the wrong direction. In other words, you're moving, but no idea where you want to go, so any path you take may be good or bad, no way to tell the difference.

If you want feedback, just dumping code is not going to do much good. Reverse engineering properties from code is not efficient, in particular as you didn't even tell us what properties you actually want to have. Tell properties, tell how those properties are ensured by code, and then we can verify that the code is actually doing what you claim it is doing.

The above also turns the goal of code upside down. Code is not for computers, code is for fellow human beings. Write code that is easy to read for humans, so others can easily verify the code is doing what it is supposed to be doing.

Trying to read your code gives me eyestrain

You don`t see everyday this kind of code so you could make an exception this time.

while loop updated

while(!exit)
{
 
 if((BestNode.x != Endx)||(BestNode.z != Endz))
 {
  int x = BestNode.x;
  int z = BestNode.z;
   
   
  if(NodeCoord(x,z+1).access)//n
  {
   exists = false;
   for(int i =0; i < TrunkListCount; i++)
   {
    if((TrunkList[i].x == NodeCoord(x,z+1).x)&&(TrunkList[i].z == NodeCoord(x,z+1).z))
    {
     exists = true;
    }
     
   }
   if(!exists)
   {
    TrunkListCount++;
    TrunkList[TrunkListCount].x =  NodeCoord(x,z+1).x;
    TrunkList[TrunkListCount].z =  NodeCoord(x,z+1).z;
    TrunkList[TrunkListCount].index = TrunkListCount;
    NodeCoord(x,z+1,TrunkListCount);
    TrunkList[TrunkListCount].Parent = NodeCoord(x,z).tileindex;  
     
   }
  }
  if(NodeCoord(x+1,z+1).access)//ne
  {
   exists = false;
   for(int i =0; i < TrunkListCount; i++)
   {
    if((TrunkList[i].x == NodeCoord(x+1,z+1).x)&&(TrunkList[i].z == NodeCoord(x+1,z+1).z))
    {
     exists = true;
    }
     
   }
   if(!exists)
   {
    TrunkListCount++;
    TrunkList[TrunkListCount].x =  NodeCoord(x+1,z+1).x;
    TrunkList[TrunkListCount].z =  NodeCoord(x+1,z+1).z;
    TrunkList[TrunkListCount].index = TrunkListCount;
    NodeCoord(x+1,z+1,TrunkListCount);
    TrunkList[TrunkListCount].Parent = NodeCoord(x,z).tileindex;  
     
   }    
  }
  if(NodeCoord(x+1,z).access)//e
  {
   exists = false;
   for(int i =0; i < TrunkListCount; i++)
   {
    if((TrunkList[i].x == NodeCoord(x+1,z).x)&&(TrunkList[i].z == NodeCoord(x+1,z).z))
    {
     exists = true;
    }
     
   }
   if(!exists)
   {
    TrunkListCount++;
    TrunkList[TrunkListCount].x =  NodeCoord(x+1,z).x;
    TrunkList[TrunkListCount].z =  NodeCoord(x+1,z).z;
    TrunkList[TrunkListCount].index = TrunkListCount;
    NodeCoord(x+1,z,TrunkListCount);
    TrunkList[TrunkListCount].Parent = NodeCoord(x,z).tileindex;  
     
   }  
  }
  if(NodeCoord(x+1,z-1).access)//se
  {
   exists = false;
   for(int i =0; i < TrunkListCount; i++)
   {
    if((TrunkList[i].x == NodeCoord(x+1,z-1).x)&&(TrunkList[i].z == NodeCoord(x+1,z-1).z))
    {
     exists = true;
    }
     
   }
   if(!exists)
   {
    TrunkListCount++;
    TrunkList[TrunkListCount].x =  NodeCoord(x+1,z-1).x;
    TrunkList[TrunkListCount].z =  NodeCoord(x+1,z-1).z;
    TrunkList[TrunkListCount].index = TrunkListCount;
    NodeCoord(x+1,z-1,TrunkListCount);
    TrunkList[TrunkListCount].Parent = NodeCoord(x,z).tileindex;  
   }   }
  if(NodeCoord(x,z-1).access)//s
  {
   exists = false;
   for(int i =0; i < TrunkListCount; i++)
   {
    if((TrunkList[i].x == NodeCoord(x,z-1).x)&&(TrunkList[i].z == NodeCoord(x,z-1).z))
    {
     exists = true;
    }
     
   }
   if(!exists)
   {
    TrunkListCount++;
    TrunkList[TrunkListCount].x =  NodeCoord(x,z-1).x;
    TrunkList[TrunkListCount].z =  NodeCoord(x,z-1).z;  
    TrunkList[TrunkListCount].index = TrunkListCount;
    NodeCoord(x,z-1,TrunkListCount);
    TrunkList[TrunkListCount].Parent = NodeCoord(x,z).tileindex;  
   }    
  }
  if(NodeCoord(x-1,z-1).access)//sw
  {
   exists = false;
   for(int i =0; i < TrunkListCount; i++)
   {
    if((TrunkList[i].x == NodeCoord(x-1,z-1).x)&&(TrunkList[i].z == NodeCoord(x-1,z-1).z))
    {
     exists = true;
    }
     
   }
   if(!exists)
   {
    TrunkListCount++;
    TrunkList[TrunkListCount].x =  NodeCoord(x-1,z-1).x;
    TrunkList[TrunkListCount].z =  NodeCoord(x-1,z-1).z;
    TrunkList[TrunkListCount].index = TrunkListCount;
    NodeCoord(x-1,z-1,TrunkListCount);
    TrunkList[TrunkListCount].Parent = NodeCoord(x,z).tileindex;  
   } }
  if(NodeCoord(x-1,z).access)//w
  {
   exists = false;
   for(int i =0; i < TrunkListCount; i++)
   {
    if((TrunkList[i].x == NodeCoord(x+1,z-1).x)&&(TrunkList[i].z == NodeCoord(x+1,z-1).z))
    {
     exists = true;
    }
     
   }
   if(!exists)
   {
    TrunkListCount++;
    TrunkList[TrunkListCount].x =  NodeCoord(x+1,z-1).x;
    TrunkList[TrunkListCount].z =  NodeCoord(x+1,z-1).z;
    TrunkList[TrunkListCount].index = TrunkListCount;
    NodeCoord(x+1,z-1,TrunkListCount);
    TrunkList[TrunkListCount].Parent = NodeCoord(x,z).tileindex;  
   }}
  if(NodeCoord(x-1,z+1).access)//nw
  {
   exists = false;
   for(int i =0; i < TrunkListCount; i++)
   {
    if((TrunkList[i].x == NodeCoord(x-1,z+1).x)&&(TrunkList[i].z == NodeCoord(x-1,z+1).z))
    {
     exists = true;
    }
     
   }
   if(!exists)
   {
    TrunkListCount++;
    TrunkList[TrunkListCount].x =  NodeCoord(x-1,z+1).x;
    TrunkList[TrunkListCount].z =  NodeCoord(x-1,z+1).z;
    TrunkList[TrunkListCount].index = TrunkListCount;
    NodeCoord(x-1,z+1,TrunkListCount);
    TrunkList[TrunkListCount].Parent = NodeCoord(x,z).tileindex;  
   }   }
 }
 else
 {
  exit = true;
 }
}

My project`s facebook page is “DreamLand Page”

Calin said:
You don`t see everyday this kind of code so you could make an exception this time.

You never update ‘BestNode’, so the code would just loop until it crashes because TrunkListCount gets larger than array size.

Why do we need to convert coordinates to NodeCoord coordinates? Is this indierection necessary assuming you use a regular grid?

Overall i miss tracking path path length.

Do you develop this only in your head? Why don't compile and run it, to see what happens? Additionally you could visualize the sate after given iteration. I often use a slider to set this, then i can see visually each step and how things change.
Easy without much effort and thinking.

Calin said:
You don`t see everyday this kind of code so you could make an exception this time.

Make a %$#@! function already. All you need are arguments to pass into NodeCoord(param1, param2). Why would you put this off?

🙂🙂🙂🙂🙂<←The tone posse, ready for action.

This topic is closed to new replies.

Advertisement