Way to relieve the CPU with nested while loops

Started by Rocco, Tue 26/06/2012 15:45:14

Previous topic - Next topic

Rocco

well my construct looks like this:
(the critical section is commented right now, so it works smooth) 

Code: AGS

function room_RepExec()
{
  int i = 0;
  
  while(i < Game.CharacterCount)
  {
    if(Guard[i].alerted)
    {
      
      /*
      int j = 0;
      while(j < Game.CharacterCount)
      {
        if(i != j)
          if(Guard[j].alive)
             if(character[i].ROC_FUNC_Get_Distance_From(character[j]) < 300)
                   character[j].ROC_FUNC_Guard_Alerted();
            
     */

       if(character[i].IsCollidingWithChar(player))
         if(thief.guards_alerted > 1)
         player.ChangeRoom(5);        
         else
         {

            character[i].ROC_FUNC_JB_Startfight(); 
         } 
         
        // j++;
         
      //}
       
    } 

    i++;
  } 
}



but its obivious when the other section isnt commented, it brings the PC down to his knees.
i thaught about a gamecycle counter, only execute this code every 3rd frame or so, but i guess this isnt a convenient solution either..
And suggestions how to solve this?

DoorKnobHandle

1.) You're looping through all the characters in your game. Maybe it would be better to check if the character is actually in the same room as the player first.

2.) What's wrong with the commented code? It should work well in most cases and be a LOT faster.

Khris

I don't think it's that obvious that the code should slow down the game that much.

I'm wondering: shouldn't the entire inner loop skip the i == j case?
Or rather, shouldn't the part below the commented out part be outside the inner loop? Since it doesn't use j.

Here's my suggestion:
Code: ags
function room_RepExec()
{
  int i = 0, j;
  while(i < Game.CharacterCount)
  {
    if (i != player.ID && Guard[i].alerted)
    {
      // alert other guards close to current one
      j = 0;
      while(j < Game.CharacterCount)
      {
        if (i != j && Guard[j].alive && !Guard[j].alerted) {
          if (character[i].ROC_FUNC_Get_Distance_From(character[j]) < 300)
            character[j].ROC_FUNC_Guard_Alerted();
        }
        j++;
      }
    }
    
    if(character[i].IsCollidingWithChar(player)) {
      if(thief.guards_alerted > 1) player.ChangeRoom(5);        
      else character[i].ROC_FUNC_JB_Startfight();
    }
    i++;
  }
}

Crimson Wizard

#3
First, are all the other characters except players - guards?
You practically are checking through all the characters, and through all the characters for every character all the time. This makes (Character count * Character count) checks. Perhaps you could start inner loop with outer loop's index, like
Code: cpp
int j = i;


Are ALL of the characters in the same room? Perhaps you could check their rooms first.

Then, I am also curious what's in functions ROC_FUNC_Get_Distance_From().

Heh, nice, 3 comments at once :)

EDIT: dang, by comment on j = i does not make sense, since you check only alerted guards in outer loop. So ignore that.

Rocco

#4
Quote from: dkh on Tue 26/06/2012 16:00:53
1.) You're looping through all the characters in your game. Maybe it would be better to check if the character is actually in the same room as the player first.

2.) What's wrong with the commented code? It should work well in most cases and be a LOT faster.
Thx, checking for the npcs in the current room, brings a lot of performance, i think it could work this way.  :)

@Khris: Your modules are also running in the back (POV and NoMouse) you know ;), so i guess there is too much going on with the innefficient while loops..
Thx for your suggestion, i will merge all the ideas and see if i can make this construct working.

@Crimson:
Code: AGS
function ROC_FUNC_Get_Distance_From(this Character*, Character*c) {

  float dx = IntToFloat(c.x - this.x);
  float dy = IntToFloat(c.y - this.y);
  float dis = Maths.Sqrt(dx*dx + dy*dy);
  return FloatToInt(dis, eRoundNearest);
}


edit:THX, it works now, without stucking.  :)

Ryan Timothy B

#5
I would suggest keeping a squared distance and checking against that. Thus removing the square root call because it's very taxing on the processor and not needed if you're only checking if something is within a radius.

Also to avoid yet another taxing calculation, do this instead on your Real distance function call:
Code: ags

function ROC_FUNC_Get_Distance_From(this Character*, Character*c) {
      int dx = c.x - this.x;
      int dy = c.y - this.y;
      return FloatToInt(Maths.Sqrt(IntToFloat(dx*dx + dy*dy)), eRoundNearest);
    }

The reason for this is that int calculations are faster than float calculations, so try to do all your int calculations first before converting to float. Even if it's just a few microseconds but they add up overall.

And for your squared distance:
Code: ags

function ROC_FUNC_Get_Distance_From2(this Character*, Character*c) {
      int dx = c.x - this.x;
      int dy = c.y - this.y;
      return dx*dx + dy*dy;
    }

Rocco

Thx, i will use that and keep it in my toolbox.  :)

Shane 'ProgZmax' Stevens

You could also avoid that outer while by using a global variable as your COUNTER and incrementing it in the repeatedly_execute section of the room (or if this happens in a lot of areas, in the repeatedly_execute_always function of your global script). 

I untie a lot of hangups just by getting rid of while loops altogether since the main game is essentially one big while loop.


Ryan Timothy B

#8
I forgot to mention. Another method to reduce calculations is to check the literal square radius the characters are between each other. Then if they're within the square it'll check the distance squared against your squared radius.

Code: ags
if (character[i].x > character[j].x - 300 && character[i].x < character[j].x + 300
    && character[i].y > character[j].y - 300 && character[i].y < character[j].y + 300
    && character[i].ROC_FUNC_Get_Distance_From2(character[j]) < 90000) {
  // etc
}


I check the X before I check the Y if the room is larger horizontally than vertically. Because once the engine hits a condition that is false it skips the rest of that conditional statement. The majority of the time your character won't even be within the square radius of your guards, so there is no point in constantly running the squared distance call.

Since you're using characters and I know there is a screen limit of something like 40 characters, you'll likely not notice these micromanagement changes. But you should notice it on something with a larger scale. Another method is to first check if either character is actually moving via character.Moving. Because they won't collide into each other if they're both standing still. Or if you're not actually using AGS's Walk command, you'd need to store the Moving boolean yourself. But this could lead into issues if not scripted properly. For instance if you change rooms, or if you manually change the X or Y values of the character. Etc.

Also I'm going to nitpick here for a second and say you should avoid magical numbers like you have. The 300 or 90000 (300 squared). You should use Define for these constant values so it's easier to maintain or change and helps readability of your code later. GUARD_RADIUS vs 300, or GUARD_RADIUS2 vs 90000 is much friendlier for you and others looking or editing your code.

Edit:
QuoteI untie a lot of hangups just by getting rid of while loops altogether since the main game is essentially one big while loop.
I'm confused by this. So you only increment once each game loop? Because if that's the case, if you had 40 items to iterate through, it could take a whole second to run through them all? Some games it likely wouldn't matter, but not all games.

SMF spam blocked by CleanTalk