For loop doesn't break when it should?

Started by LostTrainDude, Sun 24/06/2018 11:48:37

Previous topic - Next topic

LostTrainDude

As I am trying to work my way around missing equivalents of functions such as push(), pop() and empty() for arrays, I created my own Pop() function.

The idea is that: iterating through an array of already initalized managed structs, as soon as the loop finds an element that has its valid variable value set to true, it should:
  • switch it to false;
  • break out of the loop and exit the function.
Problem is: it exits, but for some reason doesn't stop at the first element it finds.
I know it exits because I tried creating breakpoints in the loop and debugging it step by step and apparently the loop breaks\returns when it should.
Still, the results are still not what I think I coded them to be (unless I made some silly mistake, and that could be possible).

This is the code I'm currently using:

Code: ags
managed struct PathNode
{
   bool valid;
};

PathNode* frontier[TOTAL_NODES];

PathNode* current;

function Pop(String array)
{
   for (int n = 0; n < TOTAL_NODES; n++)
   {
      if (array == "frontier")
      {
         if (frontier[n].valid)
         {
            frontier[n].valid = false;
            return; // also tried using break
         }
      }
   }
}

function Push(String array, PathNode* to_add)
{
   for (int n = TOTAL_NODES-1; n >= 0; n--)
   {
      if (array == "frontier")
      {
         if (frontier[n].valid)
         {
            frontier[n+1].valid = true;
            frontier[n+1] = to_add;
         }
         else if (n == 0)
         {
            frontier[n].valid = true;
            frontier[n] = to_add;
         }
      }
   }
}

function game_start()
{
   current = new PathNode;
   current.valid = true;
   
   for (int n = 0; n < TOTAL_NODES; n++)
   {
      frontier[n] = new PathNode;
      frontier[n].valid = false;
   }
   
   if (Empty("frontier")) Display("Frontier is EMPTY");
   else Display("Frontier is NOT EMPTY");
   
   Push("frontier", current);
   Push("frontier", current);
   Push("frontier", current);
   
   for (int n = 0; n < 5; n++)
   {
      Display("frontier\[%d]: %d", n, frontier[n].valid);
   }
   
   // Outputs:
   // Frontier[0]: 1
   // Frontier[1]: 1
   // Frontier[2]: 1
   // Frontier[3]: 0
   // Frontier[4]: 0
   
   
   Pop("frontier");
   
   for (int n = 0; n < 5; n++)
   {
      Display("frontier\[%d]: %d", n, frontier[n].valid);
   }
   
   // Outputs:
   // Frontier[0]: 0 
   // Frontier[1]: 0 -- SHOULD BE 1
   // Frontier[2]: 0 -- SHOULD BE 1
   // Frontier[3]: 0 
   // Frontier[4]: 0
}


I also tried writing the loop like this, but it still produces the same results:

Code: ags

function Pop(String array)
{
   bool found = false;
   for (int n = 0; n < TOTAL_NODES; n++)
   {
      if (!found)
      {
         if (array == "frontier")
         {
            if (frontier[n].valid)
            {
               frontier[n].valid = false;
               found = true;
            }
         }
      }
   }
}


I even tried not doing everything altogether in the game_start()

Code: ags
function on_key_press(eKeyCode keycode) 
{ 
   if (keycode == eKeyU)
   {
      Pop("frontier");
      for (int n = 0; n < 5; n++)
      {
         Display("frontier\[%d]: %d", n, frontier[n].valid);
      }
   }
}


Thanks in advance to anyone who can shed some light.
"We do not stop playing because we grow old, we grow old because we stop playing."

Gurok

You are pushing current to the array 3 times. It's the same instance and so all three are being overwritten when the first item in the array is overwritten.
[img]http://7d4iqnx.gif;rWRLUuw.gi

LostTrainDude

#2
Ohh, I see! Thanks Gurok! This means I should assign a new PathNode every time?

I tried that but apparently I'm getting another kind of error and that is: it only works once.
The first time it turns the first found valid element to false, the second time all of them.

But now I guess it's maybe something missing in the Pop() function.

(New code in the game_start() function)
Code: ags
for (int c = 0; c < 4; c++)
{
    PathNode* pn = new PathNode;
    pn.valid = true;
    Push("frontier", pn);
}
"We do not stop playing because we grow old, we grow old because we stop playing."

Gurok

Either that or copy the new PathNode's values when it's pushed instead of assigning it.
[img]http://7d4iqnx.gif;rWRLUuw.gi

Gurok

#4
Well, your push function is a little strange. Ignore my previous comments, it's late here.

EDIT: Here is something a little closer to standard for a fixed-capacity stack:

Code: ags
    function Push(String array, PathNode* to_add)
    {
       for (int n = TOTAL_NODES - 2; n >= 0; n--)
       {
          if (array == "frontier")
          {
             if (frontier[n].valid == true)
             {
                frontier[n+1].valid = true;
                frontier[n+1] = frontier[n];
             }
          }
       }
	if (array == "frontier")
	    frontier[0] = to_add;
    }


Haven't checked it thoroughly, but I think the Pop function should be fine though it would normally return a value.
[img]http://7d4iqnx.gif;rWRLUuw.gi

LostTrainDude

Thanks a lot, Gurok. I'll give it a spin and see where does it take me to :)
"We do not stop playing because we grow old, we grow old because we stop playing."

Gurok

I will add that normally for this sort of limited-sized stack structure, you use a variable to keep track of the cursor and let it wrap around when it reaches the end of the array. Saves shifting N variables across whenever you push a value.
[img]http://7d4iqnx.gif;rWRLUuw.gi

fernewelten

#7
If it's a stack in the classic sense, then you "push" something on top, and you "pop" something from the top. That means, at any point of time, the elements a[0], a[1] ... a[mx] for some mx are valid, the elements a[mx+1], a[mx+2] ... are invalid (free). So you don't need to keep track of "valid" for each element, but you manage an int mx as the fill level. It increments at each push and decrements at each pop.

LostTrainDude

Quote from: fernewelten on Sun 24/06/2018 14:25:12
If it's a stack in the classic sense, then you "push" something on top, and you "pop" something from the top. That means, at any point of time, the elements a[0], a[1] ... a[mx] for some mx are valid, the elements a[n+1], a[n+2] ... are invalid (free). So you don't need to keep track of "valid" for each element, but you manage an int mx as the fill level. It increments at each push and decrements at each pop.

I am trying to reproduce what queue::push() and queue::pop() do which, I suppose, is to add\remove elements to\from the back of the queue.
I think I have more or less grasped the idea of how to do that, but let's not put the cart before the horse (laugh)
"We do not stop playing because we grow old, we grow old because we stop playing."

Gurok

A queue? I thought it was a stack! Change the direction of your Pop method's loop. Better yet, use a variable as I suggested to track the cursor (circular queue).
[img]http://7d4iqnx.gif;rWRLUuw.gi

SMF spam blocked by CleanTalk