t0r3 Posted March 24, 2019 Share Posted March 24, 2019 (edited) Hey Title says it all Anything I could improve or change/ problems I might run into? This is the important part of the code private void walk(){ int wcLvl = getSkills().getStatic(Skill.WOODCUTTING); if (getInventory().isEmptyExcept("Bronze axe", "Iron axe", "Steel axe", "Mithril axe", "Adamant axe", "Rune axe") && !treeArea.contains(myPosition()) && wcLvl < 15){ log("Walking to treeArea"); getWalking().webWalk(treeArea); } else if (getInventory().isEmptyExcept("Bronze axe", "Iron axe", "Steel axe", "Mithril axe", "Adamant axe", "Rune axe") && !oakArea.contains(myPosition()) && wcLvl >= 15){ log("Walking to oakArea"); getWalking().webWalk(oakArea); } if (getInventory().isFull() && !Banks.LUMBRIDGE_UPPER.contains(myPosition())){ log("Walking to Bank"); getWalking().webWalk(Banks.LUMBRIDGE_UPPER); } } private void chop() throws InterruptedException{ RS2Object tree = getObjects().closest("Tree"); RS2Object oak = getObjects().closest("Oak"); int wcLvl = getSkills().getStatic(Skill.WOODCUTTING); if (tree != null && !myPlayer().isAnimating() && !myPlayer().isMoving() && wcLvl < 15){ log("Chopping down tree"); tree.interact("Chop down"); } else if (oak != null && !myPlayer().isAnimating() && !myPlayer().isMoving() && wcLvl >=15){ log("Chopping down oak"); oak.interact("Chop down"); sleep(random(600,1500)); getMouse().moveOutsideScreen(); } } private void bank(){ RS2Object bankBooth = getObjects().closest("Bank booth"); if (Banks.LUMBRIDGE_UPPER.contains(myPosition())&& bankBooth != null && !getBank().isOpen()){ log("Interacting with bank"); bankBooth.interact("Bank"); } else if (getBank().isOpen()){ int wcLvl = getSkills().getStatic(Skill.WOODCUTTING); log("Depositing all logs"); getBank().depositAllExcept("Bronze axe", "Iron axe", "Steel axe", "Mithril axe", "Adamant axe", "Rune axe"); if (wcLvl >= 6 && getInventory().contains("Bronze axe", "Iron Axe") && getBank().contains("Steel axe")){ log("Withdrawing Steel axe"); getBank().deposit("Bronze axe",1); getBank().withdraw("Steel axe",1); } if (wcLvl >= 21 && getInventory().contains("Steel axe") && getBank().contains("Mithril axe")){ log("Withdrawing Mithril axe"); getBank().deposit("Steel axe",1); getBank().withdraw("Mithril axe",1); } if (wcLvl >= 31 && getInventory().contains("Mithril axe") && getBank().contains("Adamant axe")){ log("Withdrawing Adamant axe"); getBank().deposit("Mithril axe",1); getBank().withdraw("Adamant axe",1); } } } @Override public int onLoop() throws InterruptedException{ walk(); chop(); bank(); return 1000; } } Edited March 24, 2019 by t0r3 Quote Link to comment Share on other sites More sharing options...
Chris Posted March 24, 2019 Share Posted March 24, 2019 can be rewritten to Make sure conditions are checked before proceeding. (e.g, Making sure boolean methods returned true before doing something else.) Conditional sleeps (Read Documentation) Better your logic flow (e.g, Write a getter function to get the best axe based on level, replace at bank; Change your logic if needToBank -> do this, else ....) Use the already made API functions in Bank (e.g, getBank().open, ...) import org.osbot.rs07.api.ui.RS2Widget; import org.osbot.rs07.script.Script; import org.osbot.rs07.script.ScriptManifest; import org.osbot.rs07.utility.ConditionalSleep; // OLD TUTORIAL CODE @ScriptManifest(version = 1.0, author = "", info = "", logo = "", name = "Testing2222") public class Main extends Script{ @Override public int onLoop() throws InterruptedException { /* Config & Widget Basics */ /* Basic Config checking. getConfigs().get(int id) returns a value set from the parameter id In our case 173 = run game setting. if we toggled run on or off. returns 0 value if the setting is off and we are on walking mode. returns 1 value if the setting is on and we are on running mode. */ switch (getConfigs().get(173)){ case 0: log("It's off!"); break; case 1: log("It's on!"); break; } /* For your widget check the color type. RWG: RED, WHITE, GREEN WHatever is visible will display its widget ids CLIENT DEBUGGER MENU R -= PARENT/ROOT ID -= CHILD ID --= GRANDCHILD/SUBCHILD ID */ /* Part 1 finding widget by static ids */ RS2Widget tutorial = getWidgets().get(261, 4); if (tutorial != null && tutorial.isVisible()){ log("It's visible"); } /* Part 2 finding widget by text */ RS2Widget tutorial = getWidgets().getWidgetContainingText(446, "Xeric's Heart"); if (tutorial != null && tutorial.isVisible()){ log("It's visible"); } /* Part 3 Crafting mould gold ring interaction */ RS2Widget tutorial = getWidgets().singleFilter(446, w -> w != null && w.isVisible() && w.getItemId() == 1635 && w.getInteractActions().length > 0); if (tutorial != null && tutorial.isVisible()){ log("It's visible"); if (tutorial.interact("Make-All")){ log("We clicked it!"); //Sleep for 30 seconds and check condition every 5 seconds or if the condition is true, break the sleep. new ConditionalSleep(30000, 5000) { @Override public boolean condition() throws InterruptedException { return getDialogues().isPendingContinuation() || !getInventory().contains("Gold bar"); } }.sleep(); } } else log("We skipped widget interaction"); return 3000; } } Quote Link to comment Share on other sites More sharing options...
Imthabawse Posted March 24, 2019 Share Posted March 24, 2019 (edited) For your chop method I would add a ConditionalSleep after you interact(Chop) tree's So after tree.interact ("Chop"); new ConditionalSleep (2500) { Sleeps for 2 1/2 seconds or untill the conditions met return myPlayer ().isAnimating; Will continue to chop trees once your players not animating. How does script run? Curious. Also as @Chris said no need for creating RS2object for bank API contains Bank.methods Edited March 24, 2019 by Imthabawse Quote Link to comment Share on other sites More sharing options...
t0r3 Posted March 24, 2019 Author Share Posted March 24, 2019 @Chris Quote Make sure conditions are checked before proceeding. (e.g, Making sure boolean methods returned true before doing something else.) Conditional sleeps (Read Documentation) Better your logic flow (e.g, Write a getter function to get the best axe based on level, replace at bank; Change your logic if needToBank -> do this, else ....) Use the already made API functions in Bank (e.g, getBank().open, ...) Where am I not checking my conditions before proceeding? How do I write a getter function? and what do you mean by changing my logic to " needToBank -> do this, else .... " ? Can you elaborate/make an example? @Imthabawse What do you mean? When I run the script it interacts with the tree and then doesn't interact again till player is not animating and it won't interact again as the player is not moving. It's only the important part of the code, script runs fine Quote Link to comment Share on other sites More sharing options...
Imthabawse Posted March 24, 2019 Share Posted March 24, 2019 @t0r3 ConditionalSleeps are a better practice also if you plan on banking logs I would suggest something like so.. Public int onloop() { If(getInventory().isFull() { getWalking.webWalk.bankArea; Bank.open.depositAllexcept (blah blah blah) }else { Chop (); There's 2 options here either your inventory is full and you must bank otherwise you can chop Quote Link to comment Share on other sites More sharing options...
Naked Posted March 24, 2019 Share Posted March 24, 2019 Could drastically increase readability with making a getAxe() function. 1 Quote Link to comment Share on other sites More sharing options...
Imthabawse Posted March 24, 2019 Share Posted March 24, 2019 21 minutes ago, Naked said: Could drastically increase readability with making a getAxe() function. Bank () Chop () Getaxe () If (inv is full) { Bank () }else { Chop () Getaxe () Perhaps that would work Quote Link to comment Share on other sites More sharing options...
Elixar Posted March 24, 2019 Share Posted March 24, 2019 You could look into using itemContainer() Making your own list of items so you don't have to keep doing dropAllExcept("blah, blah, blah,") https://osbot.org/api/org/osbot/rs07/api/util/ItemContainer.html Quote Link to comment Share on other sites More sharing options...
Imthabawse Posted March 25, 2019 Share Posted March 25, 2019 137 lines into a progressive wc'er based around Draynor later... and I'm bout over it lol. Fun little project while being a pain in the ass at the same time. Kudos to you for taking on such a task haha. Quote Link to comment Share on other sites More sharing options...
Naked Posted March 25, 2019 Share Posted March 25, 2019 6 hours ago, Imthabawse said: 137 lines Lol 1 Quote Link to comment Share on other sites More sharing options...
Imthabawse Posted March 25, 2019 Share Posted March 25, 2019 2 hours ago, Naked said: Lol XD Quote Link to comment Share on other sites More sharing options...
t0r3 Posted March 25, 2019 Author Share Posted March 25, 2019 (edited) 14 hours ago, Imthabawse said: 137 lines into a progressive wc'er based around Draynor later... and I'm bout over it lol. Fun little project while being a pain in the ass at the same time. Kudos to you for taking on such a task haha. Hahaha yeah, I'm just changing up the area, the banks, and using lvls for conditions as to what I'm doing not that hard, just takes a bit more time than creating a script for a single task. Edited March 25, 2019 by t0r3 Quote Link to comment Share on other sites More sharing options...
t0r3 Posted March 25, 2019 Author Share Posted March 25, 2019 14 hours ago, Elixar said: You could look into using itemContainer() Making your own list of items so you don't have to keep doing dropAllExcept("blah, blah, blah,") https://osbot.org/api/org/osbot/rs07/api/util/ItemContainer.html Hey How do I use the itemcontainer class to make a list of the axes? Might be not having enough knowledge of java haha, though I am learning from codeacademy.. Quote Link to comment Share on other sites More sharing options...
zwaffel Posted March 25, 2019 Share Posted March 25, 2019 I would suggest you to split up your methods more, so that they do just one task. Quote Link to comment Share on other sites More sharing options...
t0r3 Posted March 25, 2019 Author Share Posted March 25, 2019 25 minutes ago, zwaffel said: I would suggest you to split up your methods more, so that they do just one task. Ok, so u mean one method for oaks, one for tree's and so on? chopOaks(); chopTrees(); Yeah that would be better down the line, adding willows and yews thanks! Quote Link to comment Share on other sites More sharing options...