Imthabawse Posted March 18, 2019 Share Posted March 18, 2019 (edited) Made a simple barb village fisher that catches fish and drops and repeats for some easy levels on one of my pures. Figured id share. If anyone has tips on how to use IF and ELSE please give feedback below. Seems to be something I struggle with when scripting when and where to implement. Code: Edited * rivate void fish() throws InterruptedException { if(!getInventory().isFull() && (!myPlayer().isAnimating())) { NPC fishSpot = getNpcs().closest("Rod Fishing spot"); if(fishSpot != null) { fishSpot.interact("Lure"); log("Fishing.."); sleep(random(1500,2000)); getMouse().moveOutsideScreen(); } } } private void drop() throws InterruptedException { if(getInventory().isFull()) { log("Inventory full dropping fish.."); sleep(random(300,600)); getInventory().dropAllExcept("Feather","Fly fishing rod","Energy potion(3)"); } } private void levelUp() throws InterruptedException { if(getDialogues().isPendingContinuation()) { sleep(random(300,600)); getDialogues().clickContinue(); } } @Override public void onStart() { log("Welcome to my free fly fishing script.. enjoy the exp."); } @Override public int onLoop() throws InterruptedException { if(!area.contains(myPlayer())) { getWalking().webWalk(area); log("Not in fishing area.. walking there."); }else{ if(area.contains(myPlayer())) { fish(); drop(); levelUp(); } } return 1500; } } Edited March 22, 2019 by Imthabawse Quote Link to comment Share on other sites More sharing options...
redeems Posted March 18, 2019 Share Posted March 18, 2019 if(!myPlayer().isAnimating()) { if(!myPlayer().isMoving()) { if(!getInventory().isFull()) { if(fishSpot != null) { log("Fishing.."); you can just use and instead of doing each if. if(!myPlayer().isAnimating() && !getInventory().isFull() && !myPlayer().isMoving() && fishSpot != null) { log("Fishing.."); fishSpot.interact("Lure"); sleep(random(300,600)); getMouse().moveOutsideScreen(); } 1 Quote Link to comment Share on other sites More sharing options...
Imthabawse Posted March 18, 2019 Author Share Posted March 18, 2019 (edited) @redeems Good point would be a lot less clutter. Also added getWalking().webWalk(area); So if it can't see the area it'll run there. Edit: Added @redeems suggestion looks cleaner Edited March 18, 2019 by Imthabawse Quote Link to comment Share on other sites More sharing options...
redeems Posted March 18, 2019 Share Posted March 18, 2019 (edited) I've written this into a better structure using states. I commented everything so please take it and learn from it. Look up some java tutorials, and keep scripting. import org.osbot.rs07.script.Script; import org.osbot.rs07.api.map.Area; import org.osbot.rs07.script.ScriptManifest; import java.awt.*; import org.osbot.rs07.api.model.NPC; @ScriptManifest(name = "Fishing for States", author = "Redeems", version = 1.0, info = "", logo = "") //manifesto public class Main extends Script { private States currentState = States.FISHING; //setting starting state private Area area = new Area(3109, 3434, 3103, 3423); // fishing area public enum States {FISHING, DROPPING} //the two states private void startfishing() throws InterruptedException { //function to run on loop if (currentState == States.FISHING) { //if the state is "FISHING" then NPC fishSpot = getNpcs().closest("Rod Fishing spot"); //get closest rod fishing spot int openslots = (getInventory().getEmptySlotCount()); //variable for open inventory spots if (!myPlayer().isAnimating() && !getInventory().isFull() && !myPlayer().isMoving() && fishSpot != null) { //conditions for action log("Fishing.."); //log we've passed conditions fishSpot.interact("Lure"); //interact with fishing spot (fish) sleep(random(300, 600)); //sleep to give time to interact getMouse().moveOutsideScreen(); //move mouse off screen "said to be good? i dont do this..." if (openslots == 0) { //if no open inventory spaces....then currentState = States.DROPPING; //switch state to dropping } } } else if (currentState == States.DROPPING) { //if the state is dropping... log("Inventory full dropping fish.."); //log that we are in this state and executing getInventory().dropAllExcept("Feather","Fly fishing rod"); //drop everything besides feathers and rod } } @Override public void onStart() { } @Override public void onExit() { } @Override public int onLoop() { if (!area.contains(myPlayer())) { // if the area doesnt contain yourself log("Not in fishing area.. walking there."); //log that to be the case getWalking().walk(area); //walk to fishing spot } else { // otherwise.... if (area.contains(myPlayer())) { //if the area does include you try { //try catch statement in case something goes wrong we have an exception. if (myPlayer().isVisible()) { //something thats always true. log("about to start fishing"); //log entering function startfishing(); //enter function } } catch (InterruptedException e) { //catch log("got interrupted!"); //useless exception. } } } return random(1000, 1500); //how fast we loop. not needed to be random, i like it just for fun. } @Override public void onPaint (Graphics2D g) { } } Edited March 18, 2019 by redeems Quote Link to comment Share on other sites More sharing options...
HeyImJamie Posted March 18, 2019 Share Posted March 18, 2019 3 minutes ago, redeems said: I've written this into a better structure using states. I commented everything so please take it and learn from it. Look up some java tutorials, and keep scripting. import org.osbot.rs07.script.Script; import org.osbot.rs07.api.map.Area; import org.osbot.rs07.script.ScriptManifest; import java.awt.*; import org.osbot.rs07.api.model.NPC; @ScriptManifest(name = "Fishing for States", author = "Redeems", version = 1.0, info = "", logo = "") //manifesto public class Main extends Script { private States currentState = States.FISHING; //setting starting state private Area area = new Area(3109, 3434, 3103, 3423); // fishing area public enum States {FISHING, DROPPING} //the two states private void startfishing() throws InterruptedException { //function to run on loop if (currentState == States.FISHING) { //if the state is "FISHING" then NPC fishSpot = getNpcs().closest("Rod Fishing spot"); //get closest rod fishing spot int openslots = (getInventory().getEmptySlotCount()); //variable for open inventory spots if (!myPlayer().isAnimating() && !getInventory().isFull() && !myPlayer().isMoving() && fishSpot != null) { //conditions for action log("Fishing.."); //log we've passed conditions fishSpot.interact("Lure"); //interact with fishing spot (fish) sleep(random(300, 600)); //sleep to give time to interact getMouse().moveOutsideScreen(); //move mouse off screen "said to be good? i dont do this..." if (openslots == 0) { //if no open inventory spaces....then currentState = States.DROPPING; //switch state to dropping } } } else if (currentState == States.DROPPING) { //if the state is dropping... log("Inventory full dropping fish.."); //log that we are in this state and executing getInventory().dropAllExcept("Feather","Fly fishing rod"); //drop everything besides feathers and rod } } @Override public void onStart() { } @Override public void onExit() { } @Override public int onLoop() { if (!area.contains(myPlayer())) { // if the area doesnt contain yourself log("Not in fishing area.. walking there."); //log that to be the case getWalking().walk(area); //walk to fishing spot } else { // otherwise.... if (area.contains(myPlayer())) { //if the area does include you try { //try catch statement in case something goes wrong we have an exception. if (myPlayer().isVisible()) { //something thats always true. log("about to start fishing"); //log entering function startfishing(); //enter function } } catch (InterruptedException e) { //catch log("got interrupted!"); //useless exception. } } } return random(1000, 1500); //how fast we loop. not needed to be random, i like it just for fun. } @Override public void onPaint (Graphics2D g) { } } Where are you setting your states? Also this may be preference, but wouldn't it be easier just to make use of a switch statement. Something like this perhaps. private int onLoop() [ switch (getBotState()) { case WALK_TO_AREA: // walk code break; case DROP: // dropping code break; case FISH: // fishing code break; } return 50; } private enum BotState { WALK_TO_AREA, DROP, FISH } private BotState getBotState() { if (!area.contains(Player)) { return BotState.WALK_TO_AREA; return Inventory.isFull ? BotState.DROP : BotState.FISH; } Quote Link to comment Share on other sites More sharing options...
Imthabawse Posted March 18, 2019 Author Share Posted March 18, 2019 @redeems Thanks for the input but.. I fail to see how what you wrote is better. Seems to be just about the same as what I wrote just in a different way. Either way script is doing it's intended tasks. Quote Link to comment Share on other sites More sharing options...
redeems Posted March 18, 2019 Share Posted March 18, 2019 1 minute ago, HeyImJamie said: Where are you setting your states? Also this may be preference, but wouldn't it be easier just to make use of a switch statement. Something like this perhaps. private int onLoop() [ switch (getBotState()) { case WALK_TO_AREA: // walk code break; case DROP: // dropping code break; case FISH: // fishing code break; } return 50; } private enum BotState { WALK_TO_AREA, DROP, FISH } private BotState getBotState() { if (!area.contains(Player)) { return BotState.WALK_TO_AREA; return Inventory.isFull ? BotState.DROP : BotState.FISH; } the states are an enumeration. using states allows you to be dynamic. for this, case/switch works just fine, but what happens when you have a lot of cases, gets confusing. Your way is not incorrect. 1 minute ago, Imthabawse said: @redeems Thanks for the input but.. I fail to see how what you wrote is better. Seems to be just about the same as what I wrote just in a different way. Either way script is doing it's intended tasks. It's never wrong if the outcome is successful. yours is correct. It's just good practices. 1 Quote Link to comment Share on other sites More sharing options...
Imthabawse Posted March 18, 2019 Author Share Posted March 18, 2019 (edited) @redeems Well I appreciate the feedback nonetheless and i'll have to give you're method a try. Also I use getMouse().moveOutsideScreen(); with Afk tasks like fishing cause most people click on the fishing spot then scroll over to Youtube or w\e else. Just seems more realistic to me. Edited March 18, 2019 by Imthabawse 1 Quote Link to comment Share on other sites More sharing options...
redeems Posted March 18, 2019 Share Posted March 18, 2019 2 minutes ago, Imthabawse said: @redeems Well I appreciate the feedback nonetheless and i'll have to give you're method a try. excellent. Quote Link to comment Share on other sites More sharing options...
HeyImJamie Posted March 18, 2019 Share Posted March 18, 2019 (edited) 12 minutes ago, redeems said: the states are an enumeration. using states allows you to be dynamic. for this, case/switch works just fine, but what happens when you have a lot of cases, gets confusing. Your way is not incorrect. It's never wrong if the outcome is successful. yours is correct. It's just good practices. You do not set your states anywhere, so the script you have posted will not work. Edit: Upon re-reading it, I've spotted where you set your state, but I still can't see it working. The script will inevitably get stuck dropping forever, as the state is never set back to fishing. Edited March 18, 2019 by HeyImJamie 1 Quote Link to comment Share on other sites More sharing options...
Imthabawse Posted March 18, 2019 Author Share Posted March 18, 2019 @HeyImJamie and @redeems now now children this is for educational purposes not pointing out each others wrong doings Also me: Quote Link to comment Share on other sites More sharing options...
HeyImJamie Posted March 18, 2019 Share Posted March 18, 2019 1 minute ago, Imthabawse said: @HeyImJamie and @redeems now now children this is for educational purposes not pointing out each others wrong doings Also me: It's a learning lesson for all. No hatred, just discussing. 1 Quote Link to comment Share on other sites More sharing options...
redeems Posted March 18, 2019 Share Posted March 18, 2019 Just now, HeyImJamie said: You do not set your states anywhere, so the script you have posted will not work. ? https://gyazo.com/134f41b8e4a867b03c372fb0e6a70020 Quote Link to comment Share on other sites More sharing options...
HeyImJamie Posted March 18, 2019 Share Posted March 18, 2019 1 minute ago, redeems said: ? https://gyazo.com/134f41b8e4a867b03c372fb0e6a70020 Like I stated, it will work once. Let it fish a full inventory, drop it, and get back to me. Quote Link to comment Share on other sites More sharing options...
redeems Posted March 18, 2019 Share Posted March 18, 2019 7 minutes ago, HeyImJamie said: You do not set your states anywhere, so the script you have posted will not work. private States currentState = States.FISHING; //setting starting state private Area area = new Area(3109, 3434, 3103, 3423); // fishing area public enum States {FISHING, DROPPING} //the two states 1 minute ago, redeems said: private States currentState = States.FISHING; //setting starting state private Area area = new Area(3109, 3434, 3103, 3423); // fishing area public enum States {FISHING, DROPPING} //the two states LUL just realized i never ....okay you win this time LUL Quote Link to comment Share on other sites More sharing options...