whoknowshonestly Posted June 18, 2018 Share Posted June 18, 2018 (edited) Hi, I've just recently decided to try my hand at some basic scripting. I've been following a few tutorials on the site but I can't get the Conditional Sleep to work no matter where I place it. It's an extremely basic woodcutter, here's the code: import org.osbot.rs07.api.map.Area; import org.osbot.rs07.api.model.Entity; import org.osbot.rs07.script.Script; import org.osbot.rs07.utility.ConditionalSleep; import org.osbot.rs07.script.ScriptManifest; import java.util.function.BooleanSupplier; @ScriptManifest(author = "whoknowshonestly", name = "Trial Woodcutter", info = "First script - will chop trees and drop the logs", version = 0.1, logo = "https://i.imgur.com/7m1RYof.png") public final class Woodcutter extends Script { private final Area oakChoppingArea = new Area(3129, 3329, 3148, 3317); @Override public final int onLoop() throws InterruptedException { if (!oakChoppingArea.contains(myPosition())){ getWalking().webWalk(oakChoppingArea);} if (canChopTree()) { chopTree(); } else { dropWood(); } return random(1000, 2000); } private boolean canChopTree(){ Entity tree = getObjects().closest("Oak"); if (tree != null && tree.interact("Chop down") && !myPlayer().isAnimating()) { return true; } return false; } private boolean isCurrentlyChopping(){ return myPlayer().isAnimating(); } private boolean isInventoryEmpty() { return getInventory().contains(""); } private boolean inventoryContainsLogs(){ return getInventory().contains("Logs", "Oak logs", "Willow logs"); } private void chopTree(){ if (isInventoryEmpty() && !isCurrentlyChopping()){ if (getObjects().closest("Oak").interact("Chop down")) { new ConditionalSleep(10000, 10000) { @Override public boolean condition() { return inventoryContainsLogs(); } }.sleep(); } } } private void dropWood(){ inventory.dropAll(); } } class Sleep extends ConditionalSleep { private final BooleanSupplier condition; public Sleep(final BooleanSupplier condition, final int timeout) { super(timeout); this.condition = condition; } @Override public final boolean condition() throws InterruptedException { return condition.getAsBoolean(); } public static boolean sleepUntil(final BooleanSupplier condition, final int timeout) { return new Sleep(condition, timeout).sleep(); } } I have tried placing the ConditionalSleep into the OnLoop(), and moved the conditions with it but it still will always click when the random timeout expires at the end of the OnLoop(). I could get the regular sleep method to work but obviously the conditional sleep is preferable and so would appreciate it if someone could point me in the right direction. Edited June 18, 2018 by whoknowshonestly Quote Link to comment Share on other sites More sharing options...
Chris Posted June 18, 2018 Share Posted June 18, 2018 16 minutes ago, whoknowshonestly said: Hi, I've just recently decided to try my hand at some basic scripting. I've been following a few tutorials on the site but I can't get the Conditional Sleep to work no matter where I place it. It's an extremely basic woodcutter, here's the code: import org.osbot.rs07.api.map.Area; import org.osbot.rs07.api.model.Entity; import org.osbot.rs07.script.Script; import org.osbot.rs07.utility.ConditionalSleep; import org.osbot.rs07.script.ScriptManifest; import java.util.function.BooleanSupplier; @ScriptManifest(author = "whoknowshonestly", name = "Trial Woodcutter", info = "First script - will chop trees and drop the logs", version = 0.1, logo = "https://i.imgur.com/7m1RYof.png") public final class Woodcutter extends Script { private final Area oakChoppingArea = new Area(3129, 3329, 3148, 3317); @Override public final int onLoop() throws InterruptedException { if (!oakChoppingArea.contains(myPosition())){ getWalking().webWalk(oakChoppingArea);} if (canChopTree()) { chopTree(); } else { dropWood(); } return random(1000, 2000); } private boolean canChopTree(){ Entity tree = getObjects().closest("Oak"); if (tree != null && tree.interact("Chop down") && !myPlayer().isAnimating()) { return true; } return false; } private boolean isCurrentlyChopping(){ return myPlayer().isAnimating(); } private boolean isInventoryEmpty() { return getInventory().contains(""); } private boolean inventoryContainsLogs(){ return getInventory().contains("Logs", "Oak logs", "Willow logs"); } private void chopTree(){ if (isInventoryEmpty() && !isCurrentlyChopping()){ if (getObjects().closest("Oak").interact("Chop down")) { new ConditionalSleep(10000, 10000) { @Override public boolean condition() { return inventoryContainsLogs(); } }.sleep(); } } } private void dropWood(){ inventory.dropAll(); } } class Sleep extends ConditionalSleep { private final BooleanSupplier condition; public Sleep(final BooleanSupplier condition, final int timeout) { super(timeout); this.condition = condition; } @Override public final boolean condition() throws InterruptedException { return condition.getAsBoolean(); } public static boolean sleepUntil(final BooleanSupplier condition, final int timeout) { return new Sleep(condition, timeout).sleep(); } } I have tried placing the ConditionalSleep into the OnLoop(), and moved the conditions with it but it still will always click when the random timeout expires at the end of the OnLoop(). I could get the regular sleep method to work but obviously the conditional sleep is preferable and so would appreciate it if someone could point me in the right direction. new ConditionalSleep(int timeout, int sleep_per_condition_check) { @Override public boolean condition() { return condition; // return true = break loop; return false = keep checking every int sleep per condition check until timeout ends } }.sleep(); //Sleep for timeout until condition is reached Quote Link to comment Share on other sites More sharing options...
Apaec Posted June 18, 2018 Share Posted June 18, 2018 Looks like you're using it right - although you don't need your 'Sleep' class (perhaps avoid this until you understand functional interfaces or lambdas in general - better stick to the anonymous conditional sleep instantiation for now as you are doing already) For some reason you're interacting with the tree twice, once in your 'canChopTree' method and once in your 'chopTree' method - perhaps this is a source of confusion. Best put a number of console logs around the script to see exactly where the script gets (woo, debugging!). Finally, it's worth considering the arguments to the ConditionalSleep class constructor. You've put 10000 for both the timeout AND the sleep time. This means that it will always sleep for ~10000ms exactly! Oops! -Apa 3 Quote Link to comment Share on other sites More sharing options...
whoknowshonestly Posted June 18, 2018 Author Share Posted June 18, 2018 2 hours ago, Apaec said: Looks like you're using it right - although you don't need your 'Sleep' class (perhaps avoid this until you understand functional interfaces or lambdas in general - better stick to the anonymous conditional sleep instantiation for now as you are doing already) For some reason you're interacting with the tree twice, once in your 'canChopTree' method and once in your 'chopTree' method - perhaps this is a source of confusion. Best put a number of console logs around the script to see exactly where the script gets (woo, debugging!). Finally, it's worth considering the arguments to the ConditionalSleep class constructor. You've put 10000 for both the timeout AND the sleep time. This means that it will always sleep for ~10000ms exactly! Oops! -Apa Just thought I should let you know I've got it working now thanks to your comment and I can see why it was getting confused. Thanks very much for the help, appreciate it. Quote Link to comment Share on other sites More sharing options...
Apaec Posted June 19, 2018 Share Posted June 19, 2018 8 hours ago, whoknowshonestly said: Just thought I should let you know I've got it working now thanks to your comment and I can see why it was getting confused. Thanks very much for the help, appreciate it. No worries - let me know if you have any further problems! -Apa Quote Link to comment Share on other sites More sharing options...
Alek Posted June 20, 2018 Share Posted June 20, 2018 Some bonus criticisms: if (getObjects().closest("Oak").interact("Chop down")) { This is a null pointer waiting to happen private boolean isInventoryEmpty() { return getInventory().contains(""); } Just use the API methods, getInventory().isEmpty() - what you're using is not good return random(1000, 2000); You're not being clever, literally hasn't worked in 15 years of botting. Just return 1500. If you still think this is even worth your time, read up on some distributions and how they work: https://www.mathsisfun.com/data/standard-normal-distribution.html private boolean isCurrentlyChopping(){ return myPlayer().isAnimating(); } private void dropWood(){ inventory.dropAll(); } These methods may be okay for now if you plan on expanding them. However don't create wrapped methods, this is a very bad style of programming. Entity tree = getObjects().closest("Oak"); Don't use Entity, this is a super class so you're object slicing important information. Use RS2Object instead. Quote Link to comment Share on other sites More sharing options...
dogetrix Posted June 20, 2018 Share Posted June 20, 2018 3 hours ago, Alek said: return random(1000, 2000); You're not being clever, literally hasn't worked in 15 years of botting. Just return 1500. If you still think this is even worth your time, read up on some distributions and how they work: https://www.mathsisfun.com/data/standard-normal-distribution.html What if he just applies a gaussian distribution instead of a uniform one, does that actually change anything? I always thought randomizing sleep times along a gaussian distribution was safer. Quote Link to comment Share on other sites More sharing options...
whoknowshonestly Posted June 20, 2018 Author Share Posted June 20, 2018 7 hours ago, Alek said: Some bonus criticisms: if (getObjects().closest("Oak").interact("Chop down")) { This is a null pointer waiting to happen private boolean isInventoryEmpty() { return getInventory().contains(""); } Just use the API methods, getInventory().isEmpty() - what you're using is not good Hey, thanks for taking a look. I've been trying to refactor it a bit over the last couple of days so these two bits I identified as poor and fixed them already. 8 hours ago, Alek said: return random(1000, 2000); You're not being clever, literally hasn't worked in 15 years of botting. Just return 1500. If you still think this is even worth your time, read up on some distributions and how they work: https://www.mathsisfun.com/data/standard-normal-distribution.html Can you expand on this just slightly? I definitely wasn't trying to be clever - it's my first script and I was following a tutorial where they had entered 2 values in their return. The values themselves were not even chosen for a reason other than I was playing around with them trying to get the script to stop spam clicking. I tried to find some more information on what a reasonable amount of time was for setting the OnLoop() but i've just been playing around with it really. It's currently set to (100, 200) after playing around with it last night - again not for any particular reason other than to test it. 8 hours ago, Alek said: private boolean isCurrentlyChopping(){ return myPlayer().isAnimating(); } private void dropWood(){ inventory.dropAll(); } These methods may be okay for now if you plan on expanding them. However don't create wrapped methods, this is a very bad style of programming. Thanks for the advice here - i'll try and avoid it in the future. I would say that I was actually encountering some issues with the inventory.dropAll() function not dropping all of the logs as it was tending to use logs on other logs and therefore skip a random amount of logs in the inventory. At the moment I have this which seems to have fixed it but again it's probably a plaster on the wound: private void dropWood(){ if (inventory.dropAllExcept("Bronze axe", "Steel axe", "Rune axe")){ new ConditionalSleep(100, 1000) { @Override public boolean condition() { return isInventoryEmpty(); } }.sleep(); } } 8 hours ago, Alek said: Entity tree = getObjects().closest("Oak"); Don't use Entity, this is a super class so you're object slicing important information. Use RS2Object instead. I will fix this right now. Again thanks very much for the help, appreciate it. The code is improving every day. Quote Link to comment Share on other sites More sharing options...