Hel Posted April 3, 2017 Share Posted April 3, 2017 2 minutes ago, Apaec said: I would highly recommend against that. Although you might have gotten lucky in the past, using while loops like that means the risk of getting stuck is high. There's no perfect solution but it's important to design your scripts as to minimise this risk! Can you explain how it would reach an infinite loop? It tries to open the bank, and waits until the bank is open, or, sleeps for 10 * 300, 15 * 300, then tries again. I understand that if it were to somehow mess up completely and continually miss-click or whatever it would reach this stale loop, but if a bank is nearby, this handles it fine. Quote Link to comment Share on other sites More sharing options...
Reveance Posted April 3, 2017 Share Posted April 3, 2017 (edited) 11 minutes ago, Slut said: It honestly doesn't end up in an infinite loop ever, I've ran scripts for over 24 hours with that banking method and it doesn't get stuck, also I suck at utilizing the onLoop's loop. To your dismay I use those sort of loops to retry all my actions that could have a miss-click or something of the sort. But it could, because there's no timeout and that's not something you really want :p The ConditionalSleep has the same behaviour, is well tested and adds a timeout. But anyways OT: I doubt it has something to do with the conditional sleeps and stuff, since I think we can expect the API to use those aswell... there's all sorts of weird stuff happening when low CPU, but maybe Apaec has the pointed the right direction? Could it be that when low FPS, the thread that handles the conditions gets called multiple times causing multiple things to queue up of the same thing in the eventexecutor or sth? Edited April 3, 2017 by Reveance Quote Link to comment Share on other sites More sharing options...
The Undefeated Posted April 3, 2017 Author Share Posted April 3, 2017 Isn't there an option when ConditionalSleep reaches timeout it will repeat method x and checks again if the condition is true in a loop? Quote Link to comment Share on other sites More sharing options...
Reveance Posted April 3, 2017 Share Posted April 3, 2017 1 minute ago, The Undefeated said: Isn't there an option when ConditionalSleep reaches timeout it will repeat method x and checks again if the condition is true in a loop? You're already in your script loop, so I think the best thing to do would be to make solid conditions for that step to be executed, and if the timeout expires and it didn't get the sleep condition to true, you should return and the next iteration of the script loop would go to that method again since the conditions for that 'task' are still true as it didn't finish it. But for this it would also be better to add a failsave :P so it doesn't keep continuing the whole time Quote Link to comment Share on other sites More sharing options...
The Undefeated Posted April 3, 2017 Author Share Posted April 3, 2017 51 minutes ago, Reveance said: You're already in your script loop, so I think the best thing to do would be to make solid conditions for that step to be executed, and if the timeout expires and it didn't get the sleep condition to true, you should return and the next iteration of the script loop would go to that method again since the conditions for that 'task' are still true as it didn't finish it. But for this it would also be better to add a failsave :P so it doesn't keep continuing the whole time I'll work on that. Here is a Gyazo of the problem accuring. https://gyazo.com/10b863bc29ea09ea472314a23114a1b1 1 Quote Link to comment Share on other sites More sharing options...
harrypotter Posted April 3, 2017 Share Posted April 3, 2017 20 minutes ago, The Undefeated said: I'll work on that. Here is a Gyazo of the problem accuring. https://gyazo.com/10b863bc29ea09ea472314a23114a1b1 This is 100% caused by the way the code is executed. You need to structure it in a way that if it does fail it will simply try again on the next loop. Here is an example of one of my banking methods: if (!script.inventory.contains("Coins")) { if (!Banks.LUMBRIDGE_UPPER.contains(script.myPosition())) { script.getWalking().webWalk(Banks.LUMBRIDGE_UPPER); } else { if (!script.getBank().isOpen()) { if (script.getBank().open()) { Sleep.sleepUntil(() -> script.getBank().isOpen(), Script.random(5000, 10000)); } } else { if (script.getBank().contains("Coins")) { if (script.getBank().withdrawAll("Coins")) { Sleep.sleepUntil(() -> script.inventory.contains("Coins"), Script.random(5000, 10000)); } } else { script.stop(false); } } } } 1 Quote Link to comment Share on other sites More sharing options...
Apaec Posted April 3, 2017 Share Posted April 3, 2017 1 hour ago, Slut said: Can you explain how it would reach an infinite loop? It tries to open the bank, and waits until the bank is open, or, sleeps for 10 * 300, 15 * 300, then tries again. I understand that if it were to somehow mess up completely and continually miss-click or whatever it would reach this stale loop, but if a bank is nearby, this handles it fine. What if you're not at a bank Quote Link to comment Share on other sites More sharing options...
The Undefeated Posted April 3, 2017 Author Share Posted April 3, 2017 30 minutes ago, harrypotter said: This is 100% caused by the way the code is executed. You need to structure it in a way that if it does fail it will simply try again on the next loop. Here is an example of one of my banking methods: if (!script.inventory.contains("Coins")) { if (!Banks.LUMBRIDGE_UPPER.contains(script.myPosition())) { script.getWalking().webWalk(Banks.LUMBRIDGE_UPPER); } else { if (!script.getBank().isOpen()) { if (script.getBank().open()) { Sleep.sleepUntil(() -> script.getBank().isOpen(), Script.random(5000, 10000)); } } else { if (script.getBank().contains("Coins")) { if (script.getBank().withdrawAll("Coins")) { Sleep.sleepUntil(() -> script.inventory.contains("Coins"), Script.random(5000, 10000)); } } else { script.stop(false); } } } } I'll try on improving that. But I'm pretty sure it will take too many if else statements in my script. Quote Link to comment Share on other sites More sharing options...
Apaec Posted April 3, 2017 Share Posted April 3, 2017 4 minutes ago, The Undefeated said: I'll try on improving that. But I'm pretty sure it will take too many if else statements in my script. Perhaps try using some classes instead 2 Quote Link to comment Share on other sites More sharing options...
Reveance Posted April 3, 2017 Share Posted April 3, 2017 (edited) 1 hour ago, The Undefeated said: I'll try on improving that. But I'm pretty sure it will take too many if else statements in my script. What Apaec said :p the way I do this is by making some state based classes that have a condition and an execute method which are evaluated and if true, run, in the script loop, then doing it like pretty much like this: BankWalker condition -> !inventory.contains("Coins") && !Banks.LUMBRIDGE_UPPER.contains(script.myPosition()) execute -> script.getWalking().webWalk(Banks.LUMBRIDGE_UPPER); BankOpener condition -> !inventory.contains("Coins") && Banks.LUMBRIDGE_UPPER.contains(script.myPosition()) && !bank.isOpen() execute -> bank.open(); sleep until open BankHandler condition -> bank.isOpen() && bank.contains("Coins") execute -> bank.withdrawAll("Coins"); ScriptStopper -> bank.isOpen() && !bank.contains("Coins") && !inventory.contains("Coins") execute -> script.stop(false); That way you can also easily add other 'States' that take priority over others and I believe it's generally less error prone But even though this might be the cause in this case, the GrandExchange#buyItem does also have some issues that are almost identical to this in terms of repetition and weird behaviour :p Edited April 3, 2017 by Reveance Quote Link to comment Share on other sites More sharing options...
The Undefeated Posted April 3, 2017 Author Share Posted April 3, 2017 10 minutes ago, Reveance said: What Apaec said :p the way I do this is by making some state based classes that have a condition and an execute method which are evaluated and if true, run, in the script loop, then doing it like pretty much like this: BankWalker condition -> !inventory.contains("Coins") && !Banks.LUMBRIDGE_UPPER.contains(script.myPosition()) execute -> script.getWalking().webWalk(Banks.LUMBRIDGE_UPPER); BankOpener condition -> !inventory.contains("Coins") && Banks.LUMBRIDGE_UPPER.contains(script.myPosition()) && !bank.isOpen() execute -> bank.open(); sleep until open BankHandler -> bank.isOpen() && bank.contains("Coins") execute -> bank.withdrawAll("Coins"); ScriptStopper -> bank.isOpen() && !bank.contains("Coins") && !inventory.contains("Coins") execute -> script.stop(false); That way you can also easily add other 'States' that take priority over others and I believe it's generally less error prone But even though this might be the cause in this case, the GrandExchange#buyItem does also have some issues that are almost identical to this in terms of repetition and weird behaviour :p That's actually Task-based, not States right? I made my own ghetto buy Method for temp use, is not that hard to do. 1 hour ago, Apaec said: Perhaps try using some classes instead Script is working so gonna take my time to clean up all. Maybe I'll switch to Task-based instead of States. 1 Quote Link to comment Share on other sites More sharing options...
Reveance Posted April 3, 2017 Share Posted April 3, 2017 6 minutes ago, The Undefeated said: That's actually Task-based, not States right? I made my own ghetto buy Method for temp use, is not that hard to do. Script is working so gonna take my time to clean up all. Maybe I'll switch to Task-based instead of States. Sorry yeah I think Task-based is correct. I believe they also used to call it Node-based or sth a while back Quote Link to comment Share on other sites More sharing options...
Final Posted April 3, 2017 Share Posted April 3, 2017 This is more of a logic issue if anything, it seems you guys may be over complicating it. Never had an issue with the low CPU mode. 1 Quote Link to comment Share on other sites More sharing options...
Reveance Posted April 3, 2017 Share Posted April 3, 2017 12 minutes ago, Final said: This is more of a logic issue if anything, it seems you guys may be over complicating it. Never had an issue with the low CPU mode. Idk, it might be in this case...but That is literally one method call to the API displaying the same behaviour. Is there a way to execute something on the game thread? I don't think Script#paint runs on the game thread does it? Quote Link to comment Share on other sites More sharing options...
Final Posted April 3, 2017 Share Posted April 3, 2017 (edited) 1 hour ago, Reveance said: Idk, it might be in this case...but That is literally one method call to the API displaying the same behaviour. Is there a way to execute something on the game thread? I don't think Script#paint runs on the game thread does it? That's because that API method is more of a 'helper method', it's a method containing specific logic for a specific task, that method more than likely has overlooked logic. In addition, it's not going to be repeatedly checked by looping through the same conditions because you only call it once and that'd just add more complexity to the method itself. Chances are it relies upon each action actually occurring such as successfully opening the Grand Exchange or not. You should design your logic in such a way that if something was to fail, it wouldn't matter since it'd just attempt this task again without it conflicting with future tasks which rely on it's success. Obviously low-cpu means clicking methods are likely to fail, because destinations are dynamically changing in real-time, let's say you are running and your target NPC is moving, you are going to have a hard time interacting with it because the screen location of that NPC is constantly changing. Some methods such as this rely on Runescape visually, as opposed to a stored value within Runescape. Edited April 3, 2017 by Final Quote Link to comment Share on other sites More sharing options...