Bobrocket Posted June 29, 2015 Share Posted June 29, 2015 Looking good for a first! A few things: You do a lot of unnecessary null checks on bestree. In the time it takes you to check if it is visible, it's not going to change to null. Also, you would run away if the mugger is existing (but not targeting you) - check if both the player and mugger are in combat You also define "tree" in a lot of local scopes - this can mean that the "tree" defined in getState() is different to the "tree" defined in onLoop() - keep variables like that outside the method scope and instead in the class scope (so every method can access it) You only check if the "tree" is not null, not the oak tree (assuming woodcutting is > 15) People would begin cutting oaks at level 16 (use >= 15 to start at 15) - if this is what you want then leave it as it is It takes around 6-7 seconds for an NPC to lose interest of you after you are out of its range. Out of the mugger's range may be inside the castle, for example. If you have any questions, feel free to message me 1 Quote Link to comment Share on other sites More sharing options...
chrismac1991 Posted June 29, 2015 Author Share Posted June 29, 2015 Looking good for a first! A few things: You do a lot of unnecessary null checks on bestree. In the time it takes you to check if it is visible, it's not going to change to null. Also, you would run away if the mugger is existing (but not targeting you) - check if both the player and mugger are in combat You also define "tree" in a lot of local scopes - this can mean that the "tree" defined in getState() is different to the "tree" defined in onLoop() - keep variables like that outside the method scope and instead in the class scope (so every method can access it) You only check if the "tree" is not null, not the oak tree (assuming woodcutting is > 15) People would begin cutting oaks at level 16 (use >= 15 to start at 15) - if this is what you want then leave it as it is It takes around 6-7 seconds for an NPC to lose interest of you after you are out of its range. Out of the mugger's range may be inside the castle, for example. If you have any questions, feel free to message me ahh very interesting! thanks so much i'll look into it right away! yeah i learned it only targets oaks at 16 i thought it was kinda of funny :P and yes, as it is the scripts runs away if the mugger exists. i was worried to try an in combat method because when my player is animating and the mugger is attacking me, the script would always ignore him and continue chopping while taking damage. im going to go through each of your points and try to learn from it and change the script accordingly, and of course if i have any troubles ill give ya a shout thanks man i appreciate it! Quote Link to comment Share on other sites More sharing options...
chrismac1991 Posted June 30, 2015 Author Share Posted June 30, 2015 (edited) Looking good for a first! A few things: You do a lot of unnecessary null checks on bestree. In the time it takes you to check if it is visible, it's not going to change to null. Also, you would run away if the mugger is existing (but not targeting you) - check if both the player and mugger are in combat You also define "tree" in a lot of local scopes - this can mean that the "tree" defined in getState() is different to the "tree" defined in onLoop() - keep variables like that outside the method scope and instead in the class scope (so every method can access it) You only check if the "tree" is not null, not the oak tree (assuming woodcutting is > 15) People would begin cutting oaks at level 16 (use >= 15 to start at 15) - if this is what you want then leave it as it is It takes around 6-7 seconds for an NPC to lose interest of you after you are out of its range. Out of the mugger's range may be inside the castle, for example. If you have any questions, feel free to message me Ok so here it is! sorry about the log messages lol it was often attemtpting to use the RUNNINGAWAY state much much more than it had to..like at all..so i added in if (!myPlayer.isMoving) to solve that. not sure if it was the best way but i think its working :P i attempted to do what you had mentioned though so let me know what you think import org.osbot.rs07.api.model.Entity; import org.osbot.rs07.api.model.NPC; import org.osbot.rs07.api.ui.Skill; import org.osbot.rs07.api.ui.Tab; import org.osbot.rs07.script.Script; import org.osbot.rs07.script.ScriptManifest; import org.osbot.rs07.utility.Area; import java.awt.*; @ScriptManifest(author = "ChrisFuckingMac", info = "Chops and Drops Trees Lumbridge", name = "Level3TreeKiller", version = 2.0, logo = "StonerNewb+FriendWhoSurelyGotReallyAnnoyedWithMe") public class FuckYouTrees extends Script { private Entity tree; private final Area CHOPPING_AREA = new Area(3160, 3225, 3200, 3243); @Override public void onStart() { log("Kill All The Trees!"); log("Fuck The World"); log("Dont Get Banned Get Bank"); } private enum State { CHOPPING_LOGS, DROPPING_LOGS, WAIT, RUNNINGAWAY }; private Entity getbesttree() { Entity regtree = objects.closest("Tree"); Entity besttree; Entity oakTree = objects.closest("Oak"); boolean oak = skills.getDynamic(Skill.WOODCUTTING) >= 15; inventory.deselectItem(); if (oak && oakTree != null){ log("are both trees there?"); besttree = oakTree; } else if (regtree != null) { log("There is no Oak, but is there a regular tree?"); besttree = regtree; log("best tree was a regular tree"); } else { besttree = null; log("apparently there is no fucking tree"); } log("best tree is chosen"); return besttree; } private State getState() { NPC npc = npcs.closest("Mugger"); if (inventory.isFull()) return State.DROPPING_LOGS; if (npc != null) return State.RUNNINGAWAY; if (tree != null) if (!myPlayer().isAnimating()) return State.CHOPPING_LOGS; if (myPlayer().isAnimating()) return State.WAIT; else return null; } @Override public int onLoop() throws InterruptedException { if (skills.getDynamic(Skill.WOODCUTTING) >= 30) { logoutTab.logOut(); } tree = getbesttree(); log("Onlopp started, and stated tree = getbestree"); switch (getState()) { case DROPPING_LOGS: log("get state chose Dropping Shit"); inventory.isFull(); tabs.open(Tab.INVENTORY); inventory.deselectItem(); inventory.dropAll("Oak logs"); inventory.dropAll("Logs"); break; case CHOPPING_LOGS: log("get state has chosen chopping logs"); if (tree != null) { if (!myPlayer().isAnimating()) { camera.toEntity(tree); if (!myPlayer().isMoving()) { tree.interact("Chop Down"); sleep(gRandom(400, 750)); } } } break; case WAIT: log("get state chose wait"); random(1000, 2500); if (myPlayer().isAnimating()) { } break; case RUNNINGAWAY: log("get state chose runaway"); NPC npc = npcs.closest("Mugger"); if (npc != null) { if (myPlayer().isUnderAttack()) if (myPlayer().isInteracting(npc)) log("am i being interacting?"); if (!myPlayer().isMoving()) log("am i already moving?");{ getLocalWalker().walk(CHOPPING_AREA, true); } } break; } return random(200, 300); } @Override public void onExit() { log("Done Already?"); } @Override // /do this soon!!! public void onPaint(Graphics2D g) { } } Edited June 30, 2015 by chrismac1991 Quote Link to comment Share on other sites More sharing options...
Bobrocket Posted June 30, 2015 Share Posted June 30, 2015 Ok so here it is! sorry about the log messages lol it was often attemtpting to use the RUNNINGAWAY state much much more than it had to..like at all..so i added in if (!myPlayer.isMoving) to solve that. not sure if it was the best way but i think its working i attempted to do what you had mentioned though so let me know what you think import org.osbot.rs07.api.model.Entity; import org.osbot.rs07.api.model.NPC; import org.osbot.rs07.api.ui.Skill; import org.osbot.rs07.api.ui.Tab; import org.osbot.rs07.script.Script; import org.osbot.rs07.script.ScriptManifest; import org.osbot.rs07.utility.Area; import java.awt.*; @ScriptManifest(author = "ChrisFuckingMac", info = "Chops and Drops Trees Lumbridge", name = "Level3TreeKiller", version = 2.0, logo = "StonerNewb+FriendWhoSurelyGotReallyAnnoyedWithMe") public class FuckYouTrees extends Script { private Entity tree; private final Area CHOPPING_AREA = new Area(3160, 3225, 3200, 3243); @Override public void onStart() { log("Kill All The Trees!"); log("Fuck The World"); log("Dont Get Banned Get Bank"); } private enum State { CHOPPING_LOGS, DROPPING_LOGS, WAIT, RUNNINGAWAY }; private Entity getbesttree() { Entity regtree = objects.closest("Tree"); Entity besttree; Entity oakTree = objects.closest("Oak"); boolean oak = skills.getDynamic(Skill.WOODCUTTING) >= 15; inventory.deselectItem(); if (oak && oakTree != null){ log("are both trees there?"); besttree = oakTree; } else if (regtree != null) { log("There is no Oak, but is there a regular tree?"); besttree = regtree; log("best tree was a regular tree"); } else { besttree = null; log("apparently there is no fucking tree"); } log("best tree is chosen"); return besttree; } private State getState() { NPC npc = npcs.closest("Mugger"); if (inventory.isFull()) return State.DROPPING_LOGS; if (npc != null) return State.RUNNINGAWAY; if (tree != null) if (!myPlayer().isAnimating()) return State.CHOPPING_LOGS; if (myPlayer().isAnimating()) return State.WAIT; else return null; } @Override public int onLoop() throws InterruptedException { if (skills.getDynamic(Skill.WOODCUTTING) >= 30) { logoutTab.logOut(); } tree = getbesttree(); log("Onlopp started, and stated tree = getbestree"); switch (getState()) { case DROPPING_LOGS: log("get state chose Dropping Shit"); inventory.isFull(); tabs.open(Tab.INVENTORY); inventory.deselectItem(); inventory.dropAll("Oak logs"); inventory.dropAll("Logs"); break; case CHOPPING_LOGS: log("get state has chosen chopping logs"); if (tree != null) { if (!myPlayer().isAnimating()) { camera.toEntity(tree); if (!myPlayer().isMoving()) { tree.interact("Chop Down"); sleep(gRandom(400, 750)); } } } break; case WAIT: log("get state chose wait"); random(1000, 2500); if (myPlayer().isAnimating()) { } break; case RUNNINGAWAY: log("get state chose runaway"); NPC npc = npcs.closest("Mugger"); if (npc != null) { if (myPlayer().isUnderAttack()) if (myPlayer().isInteracting(npc)) log("am i being interacting?"); if (!myPlayer().isMoving()) log("am i already moving?");{ getLocalWalker().walk(CHOPPING_AREA, true); } } break; } return random(200, 300); } @Override public void onExit() { log("Done Already?"); } @Override // /do this soon!!! public void onPaint(Graphics2D g) { } } Looks pretty good, but you still declare npc in a local scope. Pretty sure there is only 1 mugger so it may be more beneficial to have it in a global scope like tree. I would also recommend looking into a basic node framework: public interface Node //Can use an abstract class instead { public boolean validate(); public void run() throws InterruptedException; } //Example public class ChopTree implements Node { @Override public boolean validate() { return (tree != null); //Need to declare "tree" obv } @Override public void run() throws InterruptedException { //run chopping } } //In onStart ArrayList<Node> nodes = new ArrayList<Node>(); nodes.add(new ChopTree()); //In onLoop for (Node n : nodes) { if (n.validate()) { n.run(); //runs each node } } Quote Link to comment Share on other sites More sharing options...
chrismac1991 Posted June 30, 2015 Author Share Posted June 30, 2015 Lol damnnit yeah thats true i did do it in that case specifically for no real reason. and i think once im a little more comfortable just repeating the basics (What ive just been doing) like 100 more times ill be more comfortable to move on but when im ready nodes will be a priortiy. i just started on the paint graphics with a draynor willow chopper xD but before i move onto anything else ill let you know how this new suggestion goes! thanks again friend Quote Link to comment Share on other sites More sharing options...
itzDot Posted July 11, 2015 Share Posted July 11, 2015 just a little tip no need to make two if statements like that. if (tree != null) if (!myPlayer().isAnimating()) // how to decide which mode to be return State.CHOPPING_LOGS; instead do if (tree != null && !myPlayer().isAnimating()) return State.CHOPPING_LOGS; 1 Quote Link to comment Share on other sites More sharing options...
Bobrocket Posted July 11, 2015 Share Posted July 11, 2015 Another suggestion would be to try and split up your script in to chunks (like a Node framework), take a look at my project as an example: For example, in Webwalker: AStar.java is my pathfinder class, it will find the closest route from point A->B Bank.java is a handler for a series of BankEntities with other parameters (name, location, parent city) BankEntity.java is an interactible entity that will open a bank (just a container with a name, interaction, position and parent bank) BinaryMap.java is a class that loads the binary map, inflates it, decompresses it and loads it into memory for the pathfinder Map.java is a class that simply ties the functions of BinaryMap and TileMath together PointsOfInterest.java is a class that holds all the banks, cities etc for my pathfinder (so, for example, if I want to walk to Varrock I can type "algo.findPath(myPlayer().getPosition(), pointsOfInterest.cities('Varrock'));" - this helps a lot) TileMath.java is a little class that simply calculates the binary map position from a world position and vice versa My exchange package is going to contain an overload for the Item and ItemContainer classes to include prices via the OSBuddy GE API. My Node package contains all of my nodes, and my Pickpocketer package contains all the main code. Don't be afraid of too many files! Quote Link to comment Share on other sites More sharing options...