While (Loops Bad Practice? and Unexpected Result)

[Ace99]

Discussion:

I think I have read from time to time that having a while loop in the main loop is a bad idea/bad practice.  Can someone explain why this is?  Loops are nested in loops all the time (small applications), what is the issue here that makes it different?

Issue:

I loop though the keyset of a hashmap and run a set of tasks for each key, once complete I set the key value to true and move on.  Part of my task is to get to a specific area.  If I use this:

// initial bank for items

for(Area in Areas){
      if(!Area.contains(myPlayer){
        walkTo(Area); // This is a boolean only returning true when Area.contains(myPlayer)
      }
       //  Do actions at area
     }

Sometimes the bot will not be at the Area and all the sudden move to //  Do action at area.  I assume the walkTo(Area) hit a false return.  So instead I can do:

// initial bank for items

for(Area in Areas){
      while(!Area.contains(myPlayer){
        walkTo(Area); // This is a boolean only returning true when Area.contains(myPlayer)
      }
       //  Do actions at area
     }

And when a false is hit on walkTo(Area) it will try it again and move the screen and find to door/portal is is not finding currently.

Is there anything wrong with this or a better yet, a cleaner way to accomplish this?

[Impensus]

1 hour ago, Ace99 said:

Discussion:

I think I have read from time to time that having a while loop in the main loop is a bad idea/bad practice.  Can someone explain why this is?  Loops are nested in loops all the time (small applications), what is the issue here that makes it different?

Issue:

I loop though the keyset of a hashmap and run a set of tasks for each key, once complete I set the key value to true and move on.  Part of my task is to get to a specific area.  If I use this:

// initial bank for items

for(Area in Areas){
      if(!Area.contains(myPlayer){
        walkTo(Area); // This is a boolean only returning true when Area.contains(myPlayer)
      }
       //  Do actions at area
     }

Sometimes the bot will not be at the Area and all the sudden move to //  Do action at area.  I assume the walkTo(Area) hit a false return.  So instead I can do:

// initial bank for items

for(Area in Areas){
      while(!Area.contains(myPlayer){
        walkTo(Area); // This is a boolean only returning true when Area.contains(myPlayer)
      }
       //  Do actions at area
     }

And when a false is hit on walkTo(Area) it will try it again and move the screen and find to door/portal is is not finding currently.

Is there anything wrong with this or a better yet, a cleaner way to accomplish this?

  

While loops within your onLoop or subsequently called functions are bad practice. The reason for this is that in itself your onLoop is a while loop that will constantly run. By nesting while loops within, you can cause script performance issues and issues with other Osbot functions not being reachable (I remember when I did this my scripts always took ages to stop, leading me to believe that the Osbot functions called upon exiting were not called until the while loop exited). The best case is to just structure your code so that the correct functions/if statements will be reached when the onLoop iterates over everything repeatedly.

As for your specific issue, the .webWalk should handle objects like doors? I assume your issue when using the first snippet is as follows:

1. Loops through the areas

2. False return in one of the areas

3. Carries onto next area in the loop after returning. False return causes rest of the tasks in the previous area not to be executed.

- Using the while loop mitigates this as it will be stuck within the the navigating block until reaching the area to conduct tasks, although it isn't best practice. 

 

What I would personally do (unsure if it is what other scripters would suggest).

1. Create an execution chain of functions that carry out the task in each area, they should be called after navigating to the area. Part of these tasks is to webwalk to the next area

2. In my onLoop, instead of iterating over each area, have an if statement for each area starting at the first. When the script completes these tasks then it will webwalk to the next area and reach the next areas if statement and conduct the tasks within there.

3. This execution chain continues until every area has been navigated to and all the tasks completed for each.

By doing this, you have lots more code (and I agree it is less elegant of a solution than a for loop). You however, are certain that if your script has another strange/erroneous return whilst navigating, the next iteration of the onLoop will resume in the correct place and carry on with the execution of the tasks at hand.

[azuresuN]

Regarding the efficiency, from what i know - for and while are equal. It really matters only by preference during the task. The only problem with while is, that you will have to really make sure, that you will match your break conditions perfectly, otherwise you are risking endless loop. In this case .. what if Jagex add some object in the center of area? What if the area, you are trying to walkTo is not reachable? Then you will get stuck in the endless loop.

For example if you will try to use dialogue api to conversate with NPCs in tutorial island, you will end-up with endless loop, etc.

Edited June 24, 2020 by azuresuN

[Wolk]

I have never used a while loop, ever. Never saw/had the need of using them. 

Edited June 24, 2020 by Wolk

[BravoTaco]

While loops have their place, but with bots you really want to only execute one task/function per loop for safety and performance. If you want to use a while loop I would add a counter to set the maximum number of attempts the code can run. That way the while loop can never get stuck.

Also I think Mirror Mode is single threaded, so it could cause issues depending on how and when the values used by the API calls are updated. 

Edited June 24, 2020 by BravoTaco

[Ace99]

6 hours ago, Impensus said:

As for your specific issue, the .webWalk should handle objects like doors?

I am using a home teleport and nexus/wall mounted things.  I get a false return when the nexus, for example, is not in the screen view so it has to rotate the camera. Then after that else catch it moves onto the false return and I am stuck somewhere like that. Maybe I can adjust that 'if' functionality specially... thanks for the feedback I am going to look into the list of if statements after I try @BravoTaco idea to add a counter in the while loop to catch endless loops.  So far I have not had a mirror mode issue either.

6 hours ago, Wolk said:

I have never used a while loop, ever. Never saw/had the need of using them. 

Never say never

6 hours ago, azuresuN said:

you will have to really make sure, that you will match your break conditions perfectly

I definitely do and will use the count method above as a fail-safe.  These areas are pretty much staples so I do not foresee this as an issue but who knows. I will keep tabs on the updates they make to the game for sure.

[Nbacon]

Hello Ace and others,

I agree with Impensus. That code should be abill to stop and start.  I personaly program like a state machine or a binary tree were nodes are decision and leafs are actions. Maybe if you used a Queue stack? Thre would be no no need to write so many if else trees. On the talks of efficiency I realy don't think as programers we should care about efficiency on our side because most 95% of my codes computation is done by the api and hiden to us users. We just accept that it is writen to the best of the osbot devs abilty. I have a quest bot that is over 6000 if else statments and it seems to be abil to do all 6000 for a base case in under 1/10 of a second.

 

Im a dumb ass,

Queue stack idea:

Quote

        Event walkandtalk = scemeBot.execute(new Event() {
          
          
          
            Queue<Area> areas = new LinkedList<>(listofareas);
 			@Override
            public void onStart() throws InterruptedException {
              
              //somecode to find out what area your in ?
              
            }
            @Override
            public int execute() throws InterruptedException {
             ItemReq area = areas.peek();
              
             
            if(Area.contains(myPlayer){
                areas.poll();
            //  Do actions at area
            }else{
                walkTo(Area);
            }   
                return 600;
            }
        });

 

 

 

 

 

Edited June 25, 2020 by Nbacon

[Ace99]

@Nbacon That is a cool idea - but being honest with myself that is out of my ability right now.  Crazy but good that your 6000 if statements don't effect performance lol

[Ace99]

As many mentioned above you were right on the following:

• My while loop condition was met and was logging that it was met but the loop continued - mirror mode thread issue i guess
• The Area loop itself was an issue as well because when the code is in the foreach loop it does not hit the return on the onLoop() until the whole Area loop is done - allowing the false return on a boolean inside the Area loop to continue the code rather than loop back through until it is true

I have resorted to the list of if statements - will let you know if the outcome is better after I get the correct logic on all this restructuring.

[Nbacon]

 

30 minutes ago, Ace99 said:

but being honest with myself that is out of my ability right now.

Learn to lie to yourself. 

31 minutes ago, Ace99 said:

Crazy but good that your 6000 if statements don't effect performance lol

1/10 of second is a life time in computer time but I personaly think anything under 600ms or 1 tic to think of an action is acceptable. 

[Slick]

On 6/24/2020 at 4:40 PM, BravoTaco said:

While loops have their place, but with bots you really want to only execute one task/function per loop for safety and performance. If you want to use a while loop I would add a counter to set the maximum number of attempts the code can run. That way the while loop can never get stuck.

Also I think Mirror Mode is single threaded, so it could cause issues depending on how and when the values used by the API calls are updated. 

Wasn't aware Mirror Mode is single threaded, thanks for the tip and would love to read more the difference between stealth and mirror mode.

I was having issues terminating my script properly in osbot with an endless while loop. Putting a maximum loop count seems to fix the issue.