GaetanoH Posted March 16, 2016 Share Posted March 16, 2016 Hello, I've been working on this for a while now and I still want to add these features. Able to cook all fishes Fish cooked TTL Add antiban But I still want some feedback, what should you guys've done better? import org.osbot.rs07.api.model.NPC; import org.osbot.rs07.api.model.RS2Object; import org.osbot.rs07.api.ui.RS2Widget; import org.osbot.rs07.api.ui.Skill; import org.osbot.rs07.script.Script; import org.osbot.rs07.script.ScriptManifest; import org.osbot.rs07.utility.ConditionalSleep; import java.awt.*; import java.text.DecimalFormat; @ScriptManifest(name = "RogueDenCooker", author = "GaetanoH", version = 1.0, info = "", logo = "") public class RogueDenCooker extends Script { private int cookingXP = 0; private long startTime; public enum State { COOKING, BANKING; } public State getState(){ if(!bank.isOpen() && inventory.contains("Raw trout")){ return State.COOKING; } if(!inventory.contains("Raw trout")){ return State.BANKING; } return null; } @Override public void onStart() { log("Thanks for using my Rogue's Den Cooker, please start with enough food in the bank"); startTime = System.currentTimeMillis(); getExperienceTracker().start(Skill.COOKING); } @Override public void onExit() { } @Override public int onLoop() throws InterruptedException { switch(getState()){ case COOKING: RS2Object fire = getObjects().closest("Fire"); RS2Widget widget = getWidgets().get(307, 2); if(!isCooking()){ if(widget == null){ getInventory().getItem("Raw trout").interact("Use"); if(fire.isVisible() && myPlayer().isVisible()){ fire.interact("Use"); new ConditionalSleep(1500){ @Override public boolean condition() throws InterruptedException { return widget != null; } }.sleep(); } } else { widget.interact("Cook All"); } } break; case BANKING: NPC emeraldBenedict = getNpcs().closest("Emerald Benedict"); if(emeraldBenedict != null){ if(!bank.isOpen()){ emeraldBenedict.interact("Bank"); sleep(1500); if(bank.isOpen()){ bank.depositAll(); bank.withdraw("Raw trout", 28); bank.close(); sleep(random(300,400)); } } } break; } return 100; } public String formatTime(long ms){ long s = ms / 1000, m = s / 60, h = m / 60, d = h / 24; s %= 60; m %= 60; h %= 24; return d > 0 ? String.format("%02d:%02d:%02d:%02d", d, h, m, s) : h > 0 ? String.format("%02d:%02d:%02d", h, m, s) : String.format("%02d:%02d", m, s); } @Override public void onPaint(Graphics2D g) { Point mP = getMouse().getPosition(); g.drawLine(mP.x - 5, mP.y + 5, mP.x + 5, mP.y - 5); g.drawLine(mP.x + 5, mP.y + 5, mP.x - 5, mP.y - 5); long runTime = System.currentTimeMillis() - startTime; cookingXP = getExperienceTracker().getGainedXPPerHour(Skill.COOKING); g.drawString("Time running: " + formatTime(runTime), 10, 290); g.drawString("Experience/h: " + String.valueOf(cookingXP), 10, 310); g.drawString("Made by GaetanoH", 10, 330); } public boolean isCooking() { boolean isCooking = false; Timer timer = new Timer(1800); while (timer.isRunning() && !isCooking) { isCooking = myPlayer().getAnimation() != -1 ? true : isCooking; } return isCooking; } public class Timer { private long period; private long start; public Timer(long period) { this.period = period; this.start = System.currentTimeMillis(); } public long getElapsed() { return System.currentTimeMillis() - this.start; } public long getRemaining() { return this.period - this.getElapsed(); } public boolean isRunning() { return this.getElapsed() <= this.period; } public void setPeriod(long period) { this.period = period; } public void reset() { this.start = System.currentTimeMillis(); } public String format(long milliSeconds) { long secs = milliSeconds / 1000L; return String.format("%02d:%02d:%02d", secs / 3600L, secs % 3600L / 60L, secs % 60L); } } } Thanks in advance! Really enjoy the community so far, really helpful! Quote Link to comment Share on other sites More sharing options...
Goaks Posted March 16, 2016 Share Posted March 16, 2016 in you're states returning null is a bad idea in my opinion. If something goes wrong and the conditions are not met returning null is not a good idea. A fail safe would be better implemented Quote Link to comment Share on other sites More sharing options...
FrostBug Posted March 16, 2016 Share Posted March 16, 2016 in you are states returning null is a bad idea in my opinion. Grammar :tears: But yas, usually you would return a default state when no other states apply (eg. WAIT or SLEEP) 1 Quote Link to comment Share on other sites More sharing options...
Goaks Posted March 16, 2016 Share Posted March 16, 2016 Grammar But yas, usually you would return a default state when no other states apply (eg. WAIT or SLEEP) 'dyslexicness' gets me every time. 1 Quote Link to comment Share on other sites More sharing options...
Vilius Posted March 16, 2016 Share Posted March 16, 2016 (edited) Instead of: isCooking = myPlayer().getAnimation() != -1 ? true : isCooking; if(emeraldBenedict != null){ if(!bank.isOpen()){ emeraldBenedict.interact("Bank"); sleep(1500); if(bank.isOpen()){ bank.depositAll(); bank.withdraw("Raw trout", 28); bank.close(); sleep(random(300,400)); } } } you could do: isCooking = myPlayer().isAnimating(); if(emeraldBenedict != null){ if(!getBank().isOpen()){ emeraldBenedict.interact("Bank"); sleep(1500); }else{ getBank().depositAll(); getBank().withdraw("Raw trout", 28); getBank().close(); sleep(random(300,400)); } } Plus you shouldn't nest your if statements so often. And don't return null in your getState(), if something goes wrong you will get npe's just add a waiting state Edited March 16, 2016 by Vilius 1 Quote Link to comment Share on other sites More sharing options...
Sumo Posted March 16, 2016 Share Posted March 16, 2016 (edited) Instead of: isCooking = myPlayer().getAnimation() != -1 ? true : isCooking; if(emeraldBenedict != null){ if(!bank.isOpen()){ emeraldBenedict.interact("Bank"); sleep(1500); if(bank.isOpen()){ bank.depositAll(); bank.withdraw("Raw trout", 28); bank.close(); sleep(random(300,400)); } } } you could do: isCooking = myPlayer().isAnimating(); if(emeraldBenedict != null){ if(!getBank().isOpen()){ emeraldBenedict.interact("Bank"); sleep(1500); }else{ getBank().depositAll(); getBank().withdraw("Raw trout", 28); getBank().close(); sleep(random(300,400)); } } Plus you shouldn't nest your if statements so often. And don't return null in your getState(), if something goes wrong you will get npe's just add a waiting state I dont know about you, but I find the isAnimating() method working very bad in situations as cooking, fletching etc. There are a few milliseconds between each animation which results isAnimating() to return false. Is there any smooth solution to this? Edited March 16, 2016 by Sumo 1 Quote Link to comment Share on other sites More sharing options...
Vilius Posted March 16, 2016 Share Posted March 16, 2016 (edited) I dont know about you, but I find the isAnimating() method working very bad in situations as cooking, fletching etc. There are a few milliseconds between each animation which results isAnimating() to return false. Is there any smooth solution to this? Its actually that that the cooking animation ends -> interp (-1) -> new cooking animation. Using a Timer allows it to skip that interp and if the idle is longer than say 1000ms then you would try to cook again/bank or what not. Edited March 16, 2016 by Vilius 1 Quote Link to comment Share on other sites More sharing options...
Explv Posted March 16, 2016 Share Posted March 16, 2016 Hello, I've been working on this for a while now and I still want to add these features. Able to cook all fishes Fish cooked TTL Add antiban But I still want some feedback, what should you guys've done better? import org.osbot.rs07.api.model.NPC; import org.osbot.rs07.api.model.RS2Object; import org.osbot.rs07.api.ui.RS2Widget; import org.osbot.rs07.api.ui.Skill; import org.osbot.rs07.script.Script; import org.osbot.rs07.script.ScriptManifest; import org.osbot.rs07.utility.ConditionalSleep; import java.awt.*; import java.text.DecimalFormat; @ScriptManifest(name = "RogueDenCooker", author = "GaetanoH", version = 1.0, info = "", logo = "") public class RogueDenCooker extends Script { private int cookingXP = 0; private long startTime; public enum State { COOKING, BANKING; } public State getState(){ if(!bank.isOpen() && inventory.contains("Raw trout")){ return State.COOKING; } if(!inventory.contains("Raw trout")){ return State.BANKING; } return null; } @Override public void onStart() { log("Thanks for using my Rogue's Den Cooker, please start with enough food in the bank"); startTime = System.currentTimeMillis(); getExperienceTracker().start(Skill.COOKING); } @Override public void onExit() { } @Override public int onLoop() throws InterruptedException { switch(getState()){ case COOKING: RS2Object fire = getObjects().closest("Fire"); RS2Widget widget = getWidgets().get(307, 2); if(!isCooking()){ if(widget == null){ getInventory().getItem("Raw trout").interact("Use"); if(fire.isVisible() && myPlayer().isVisible()){ fire.interact("Use"); new ConditionalSleep(1500){ @Override public boolean condition() throws InterruptedException { return widget != null; } }.sleep(); } } else { widget.interact("Cook All"); } } break; case BANKING: NPC emeraldBenedict = getNpcs().closest("Emerald Benedict"); if(emeraldBenedict != null){ if(!bank.isOpen()){ emeraldBenedict.interact("Bank"); sleep(1500); if(bank.isOpen()){ bank.depositAll(); bank.withdraw("Raw trout", 28); bank.close(); sleep(random(300,400)); } } } break; } return 100; } public String formatTime(long ms){ long s = ms / 1000, m = s / 60, h = m / 60, d = h / 24; s %= 60; m %= 60; h %= 24; return d > 0 ? String.format("%02d:%02d:%02d:%02d", d, h, m, s) : h > 0 ? String.format("%02d:%02d:%02d", h, m, s) : String.format("%02d:%02d", m, s); } @Override public void onPaint(Graphics2D g) { Point mP = getMouse().getPosition(); g.drawLine(mP.x - 5, mP.y + 5, mP.x + 5, mP.y - 5); g.drawLine(mP.x + 5, mP.y + 5, mP.x - 5, mP.y - 5); long runTime = System.currentTimeMillis() - startTime; cookingXP = getExperienceTracker().getGainedXPPerHour(Skill.COOKING); g.drawString("Time running: " + formatTime(runTime), 10, 290); g.drawString("Experience/h: " + String.valueOf(cookingXP), 10, 310); g.drawString("Made by GaetanoH", 10, 330); } public boolean isCooking() { boolean isCooking = false; Timer timer = new Timer(1800); while (timer.isRunning() && !isCooking) { isCooking = myPlayer().getAnimation() != -1 ? true : isCooking; } return isCooking; } public class Timer { private long period; private long start; public Timer(long period) { this.period = period; this.start = System.currentTimeMillis(); } public long getElapsed() { return System.currentTimeMillis() - this.start; } public long getRemaining() { return this.period - this.getElapsed(); } public boolean isRunning() { return this.getElapsed() <= this.period; } public void setPeriod(long period) { this.period = period; } public void reset() { this.start = System.currentTimeMillis(); } public String format(long milliSeconds) { long secs = milliSeconds / 1000L; return String.format("%02d:%02d:%02d", secs / 3600L, secs % 3600L / 60L, secs % 60L); } } } Thanks in advance! Really enjoy the community so far, really helpful! 1. You don't need to store cooking xp 2. Split your code into more methods, each method should do one thing 3. Use ConditionalSleeps Your banking should look more like: private void bank(){ if(!getBank().isOpen()) openBank(); else if(!getInventory().isEmptyExcept("Raw trout")) getBank().depositAll(); else if(!getInventory().contains("Raw trout")) getBank().withdrawAll("Raw trout"); else getBank().close(); } private void openBank(){ NPC emeraldBenedict = getNpcs().closest("Emerald Benedict"); if(emeraldBenedict != null){ emeraldBenedict.interact("Bank"); new ConditionalSleep(5000) { @Override public boolean condition() throws InterruptedException { return getBank().isOpen(); } }.sleep(); } } Your getState should only return BANKING or COOKING: private State getState(){ return !getBank().isOpen() && getInventory().contains("Raw trout") ? State.COOKING : State.BANKING; } For Widgets you should use getWidgetContainingText() wherever possible. So your cooking method should look more like: private void cook(){ RS2Widget troutWidget = getWidgets().getWidgetContainingText("Trout"); if(troutWidget != null) troutWidget.interact("Cook All"); else if(!isTroutSelected()) getInventory().getItem("Raw trout").interact("Use"); else useFire(); } private boolean isTroutSelected(){ Item selectedItem = getInventory().getSelectedItem(); return selectedItem != null && selectedItem.getName().equals("Raw trout"); } private void useFire(){ RS2Object fire = getObjects().closest("Fire"); if(fire != null){ fire.interact("Use"); new ConditionalSleep(1500){ @Override public boolean condition() throws InterruptedException { return getWidgets().getWidgetContainingText("Trout") != null; } }.sleep(); } } You can also avoid using a timer by using a ConditionalSleep. So all together your script could look more like: import org.osbot.rs07.api.model.NPC; import org.osbot.rs07.api.model.RS2Object; import org.osbot.rs07.api.ui.RS2Widget; import org.osbot.rs07.api.ui.Skill; import org.osbot.rs07.script.Script; import org.osbot.rs07.script.ScriptManifest; import org.osbot.rs07.utility.ConditionalSleep; import java.awt.*; import java.text.DecimalFormat; @ScriptManifest(name = "RogueDenCooker", author = "GaetanoH", version = 1.0, info = "", logo = "") public class RogueDenCooker extends Script { private long startTime; public enum State { COOKING, BANKING; } private State getState(){ return !getBank().isOpen() && getInventory().contains("Raw trout") ? State.COOKING : State.BANKING; } @Override public void onStart() { log("Thanks for using my Rogue's Den Cooker, please start with enough food in the bank"); startTime = System.currentTimeMillis(); getExperienceTracker().start(Skill.COOKING); } @Override public int onLoop() throws InterruptedException { switch(getState()){ case COOKING: cook(); break; case BANKING: bank(); break; } return random(200, 300); } private void cook(){ if(getTroutWidget() != null) cookAll(); else if(!isTroutSelected()) getInventory().getItem("Raw trout").interact("Use"); else useFire(); } private void cookAll(){ if(getTroutWidget().interact("Cook All")){ new ConditionalSleep(60_000){ @Override public boolean condition() throws InterruptedException { return !getInventory().contains("Raw trout"); } }.sleep(); } } private boolean isTroutSelected(){ Item selectedItem = getInventory().getSelectedItem(); return selectedItem != null && selectedItem.getName().equals("Raw trout"); } private void useFire(){ RS2Object fire = getObjects().closest("Fire"); if(fire != null){ fire.interact("Use"); new ConditionalSleep(1500){ @Override public boolean condition() throws InterruptedException { return getTroutWidget() != null; } }.sleep(); } } private RS2Widget getTroutWidget(){ getWidgets().getWidgetContainingText("Trout"); } private void bank(){ if(!getBank().isOpen()) openBank(); else if(!getInventory().isEmptyExcept("Raw trout")) getBank().depositAll(); else if(!getInventory().contains("Raw trout")) getBank().withdrawAll("Raw trout"); else getBank().close(); } private void openBank(){ NPC emeraldBenedict = getNpcs().closest("Emerald Benedict"); if(emeraldBenedict != null){ emeraldBenedict.interact("Bank"); new ConditionalSleep(5000) { @Override public boolean condition() throws InterruptedException { return getBank().isOpen(); } }.sleep(); } } @Override public void onPaint(Graphics2D g) { Point mP = getMouse().getPosition(); g.drawLine(mP.x - 5, mP.y + 5, mP.x + 5, mP.y - 5); g.drawLine(mP.x + 5, mP.y + 5, mP.x - 5, mP.y - 5); long runTime = System.currentTimeMillis() - startTime; cookingXP = getExperienceTracker().getGainedXPPerHour(Skill.COOKING); g.drawString("Time running: " + formatTime(runTime), 10, 290); g.drawString("Experience/h: " + String.valueOf(cookingXP), 10, 310); g.drawString("Made by GaetanoH", 10, 330); } private String formatTime(long ms){ long s = ms / 1000, m = s / 60, h = m / 60, d = h / 24; s %= 60; m %= 60; h %= 24; return d > 0 ? String.format("%02d:%02d:%02d:%02d", d, h, m, s) : h > 0 ? String.format("%02d:%02d:%02d", h, m, s) : String.format("%02d:%02d", m, s); } } Hope that helps 1 Quote Link to comment Share on other sites More sharing options...
GaetanoH Posted March 16, 2016 Author Share Posted March 16, 2016 Its actually that that the cooking animation ends -> interp (-1) -> new cooking animation. Using a Timer allows it to skip that interp and if the idle is longer than say 1000ms then you would try to cook again/bank or what not. Instead of: isCooking = myPlayer().getAnimation() != -1 ? true : isCooking; if(emeraldBenedict != null){ if(!bank.isOpen()){ emeraldBenedict.interact("Bank"); sleep(1500); if(bank.isOpen()){ bank.depositAll(); bank.withdraw("Raw trout", 28); bank.close(); sleep(random(300,400)); } } } you could do: isCooking = myPlayer().isAnimating(); if(emeraldBenedict != null){ if(!getBank().isOpen()){ emeraldBenedict.interact("Bank"); sleep(1500); }else{ getBank().depositAll(); getBank().withdraw("Raw trout", 28); getBank().close(); sleep(random(300,400)); } } Plus you shouldn't nest your if statements so often. And don't return null in your getState(), if something goes wrong you will get npe's just add a waiting state in you're states returning null is a bad idea in my opinion. If something goes wrong and the conditions are not met returning null is not a good idea. A fail safe would be better implemented 1. You don't need to store cooking xp 2. Split your code into more methods, each method should do one thing 3. Use ConditionalSleeps Your banking should look more like: private void bank(){ if(!getBank().isOpen()) openBank(); else if(!getInventory().isEmptyExcept("Raw trout")) getBank().depositAll(); else if(!getInventory().contains("Raw trout")) getBank().withdrawAll("Raw trout"); else getBank().close(); } private void openBank(){ NPC emeraldBenedict = getNpcs().closest("Emerald Benedict"); if(emeraldBenedict != null){ emeraldBenedict.interact("Bank"); new ConditionalSleep(5000) { @Override public boolean condition() throws InterruptedException { return getBank().isOpen(); } }.sleep(); } } Your getState should only return BANKING or COOKING: private State getState(){ return !getBank().isOpen() && getInventory().contains("Raw trout") ? State.COOKING : State.BANKING; } For Widgets you should use getWidgetContainingText() wherever possible. So your cooking method should look more like: private void cook(){ RS2Widget troutWidget = getWidgets().getWidgetContainingText("Trout"); if(troutWidget != null) troutWidget.interact("Cook All"); else if(!isTroutSelected()) getInventory().getItem("Raw trout").interact("Use"); else useFire(); } private boolean isTroutSelected(){ Item selectedItem = getInventory().getSelectedItem(); return selectedItem != null && selectedItem.getName().equals("Raw trout"); } private void useFire(){ RS2Object fire = getObjects().closest("Fire"); if(fire != null){ fire.interact("Use"); new ConditionalSleep(1500){ @Override public boolean condition() throws InterruptedException { return getWidgets().getWidgetContainingText("Trout") != null; } }.sleep(); } } You can also avoid using a timer by using a ConditionalSleep. So all together your script could look more like: import org.osbot.rs07.api.model.NPC; import org.osbot.rs07.api.model.RS2Object; import org.osbot.rs07.api.ui.RS2Widget; import org.osbot.rs07.api.ui.Skill; import org.osbot.rs07.script.Script; import org.osbot.rs07.script.ScriptManifest; import org.osbot.rs07.utility.ConditionalSleep; import java.awt.*; import java.text.DecimalFormat; @ScriptManifest(name = "RogueDenCooker", author = "GaetanoH", version = 1.0, info = "", logo = "") public class RogueDenCooker extends Script { private long startTime; public enum State { COOKING, BANKING; } private State getState(){ return !getBank().isOpen() && getInventory().contains("Raw trout") ? State.COOKING : State.BANKING; } @Override public void onStart() { log("Thanks for using my Rogue's Den Cooker, please start with enough food in the bank"); startTime = System.currentTimeMillis(); getExperienceTracker().start(Skill.COOKING); } @Override public int onLoop() throws InterruptedException { switch(getState()){ case COOKING: cook(); break; case BANKING: bank(); break; } return random(200, 300); } private void cook(){ if(getTroutWidget() != null) cookAll(); else if(!isTroutSelected()) getInventory().getItem("Raw trout").interact("Use"); else useFire(); } private void cookAll(){ if(getTroutWidget().interact("Cook All")){ new ConditionalSleep(60_000){ @Override public boolean condition() throws InterruptedException { return !getInventory().contains("Raw trout"); } }.sleep(); } } private boolean isTroutSelected(){ Item selectedItem = getInventory().getSelectedItem(); return selectedItem != null && selectedItem.getName().equals("Raw trout"); } private void useFire(){ RS2Object fire = getObjects().closest("Fire"); if(fire != null){ fire.interact("Use"); new ConditionalSleep(1500){ @Override public boolean condition() throws InterruptedException { return getTroutWidget() != null; } }.sleep(); } } private RS2Widget getTroutWidget(){ getWidgets().getWidgetContainingText("Trout"); } private void bank(){ if(!getBank().isOpen()) openBank(); else if(!getInventory().isEmptyExcept("Raw trout")) getBank().depositAll(); else if(!getInventory().contains("Raw trout")) getBank().withdrawAll("Raw trout"); else getBank().close(); } private void openBank(){ NPC emeraldBenedict = getNpcs().closest("Emerald Benedict"); if(emeraldBenedict != null){ emeraldBenedict.interact("Bank"); new ConditionalSleep(5000) { @Override public boolean condition() throws InterruptedException { return getBank().isOpen(); } }.sleep(); } } @Override public void onPaint(Graphics2D g) { Point mP = getMouse().getPosition(); g.drawLine(mP.x - 5, mP.y + 5, mP.x + 5, mP.y - 5); g.drawLine(mP.x + 5, mP.y + 5, mP.x - 5, mP.y - 5); long runTime = System.currentTimeMillis() - startTime; cookingXP = getExperienceTracker().getGainedXPPerHour(Skill.COOKING); g.drawString("Time running: " + formatTime(runTime), 10, 290); g.drawString("Experience/h: " + String.valueOf(cookingXP), 10, 310); g.drawString("Made by GaetanoH", 10, 330); } private String formatTime(long ms){ long s = ms / 1000, m = s / 60, h = m / 60, d = h / 24; s %= 60; m %= 60; h %= 24; return d > 0 ? String.format("%02d:%02d:%02d:%02d", d, h, m, s) : h > 0 ? String.format("%02d:%02d:%02d", h, m, s) : String.format("%02d:%02d", m, s); } } Hope that helps Holy shit, thanks for the feedback tomorrow I'll get in to it! Thank you very much! Quote Link to comment Share on other sites More sharing options...