Sebastian Posted September 20, 2016 Share Posted September 20, 2016 Hey everyone. I'm having a question. So i've made my own woodcutting script. When i'm starting the script everything runs fine. For example: When i'm in Camelot cutting magic logs. It works for like 3 runs and then it just stands in the bank doing nothing. Instead of giving me the case to walk back to the trees it just gives me case CHOP. When i world hop it workes fine again for like 3/4 times. It's random. Sometimes it works for 10 runs, sometimes for 5 runs. I can't think that i'm doing something wrong. Does it have to do with the OSBOT client? Some people having the same problem as i do? private State getState() { Entity treeToChop = objects.closest(tree); if (inventory.isFull() && powerChop == false) return State.BANK; if(inventory.isFull() && powerChop == true) return State.POWERCHOP; if (treeToChop != null) return State.CHOP; if(Camelot_Bank.contains(myPlayer()) || EdgeVille_Bank.contains(myPlayer())) return State.WALK; return State.WAIT; } case WALK: if(!inventory.isFull() && Camelot_Bank.contains(myPlayer())) { StatusUpdate = "Searching for " + tree + " tree"; walking.webWalk(MagicTree_Area); } else if(!inventory.isFull() && EdgeVille_Bank.contains(myPlayer())) { StatusUpdate = "Searching for " + tree + " tree"; walking.webWalk(EdgeVille_Yews); } break; Quote Link to comment Share on other sites More sharing options...
liverare Posted September 20, 2016 Share Posted September 20, 2016 . . . IF TREE EXISTS --WE CHOPPIN' . . . My guess is that you're at the bank and the tree exists. You need to better structure your logic and, if applicable, use Areas EVERYWHERE, so that: IF in area 'tree' --IF inventory is full ----IF powerchop ------DROP ----ELSE ------WALK TO BANK --ELSE IF tree exists ----CHOP ELSE IF in area 'bank' --IF inventory is full ----bank etc. 2 Quote Link to comment Share on other sites More sharing options...
Th3 Posted September 20, 2016 Share Posted September 20, 2016 (edited) here is a slightly more optimized version of how it should work. We don't need to do extra calculations when they are not needed. private State getState() { boolean fullINV = inventory.isFull(); Player me = myPlayer(); if (me.getAnimation() != -1) return State.WAIT; if (fullINV && !powerChop) return State.BANK; if(fullINV && powerChop) return State.POWERCHOP; if(!Camelot_Trees.contains(me) || !EdgeVille_Trees.contains(me)) return State.WALK; return State.CHOP; } Edited September 20, 2016 by Th3 Quote Link to comment Share on other sites More sharing options...
Token Posted September 20, 2016 Share Posted September 20, 2016 (edited) You missed the case where you are outside the bank but there are no nearby trees, using areas or any other predefined sets of data is generally not a good idea. Remove the areas in your logic and it should look like this private State getState() { Entity treeToChop = objects.closest(tree); if (inventory.isFull() && powerChop == false) return State.BANK; if(inventory.isFull() && powerChop == true) return State.POWERCHOP; if (treeToChop != null) return State.CHOP; else return State.WALK; } And a state that doesn't do anything at all is just bad practice, simply returning from the current loop iteration without performing any actions will yield the same result. PS: you also have the case where you are at the desired location to chop trees but all tree instances are depleted, but since you know the destination you can add this condition to your states Edit: You should post the case CHOP as the error is definitely there Edited September 20, 2016 by Token 1 Quote Link to comment Share on other sites More sharing options...
Sebastian Posted September 20, 2016 Author Share Posted September 20, 2016 (edited) You missed the case where you are outside the bank but there are no nearby trees, using areas or any other predefined sets of data is generally not a good idea. Remove the areas in your logic and it should look like this private State getState() { Entity treeToChop = objects.closest(tree); if (inventory.isFull() && powerChop == false) return State.BANK; if(inventory.isFull() && powerChop == true) return State.POWERCHOP; if (treeToChop != null) return State.CHOP; else return State.WALK; } And a state that doesn't do anything at all is just bad practice, simply returning from the current loop iteration without performing any actions will yield the same result. PS: you also have the case where you are at the desired location to chop trees but all tree instances are depleted, but since you know the destination you can add this condition to your states Edit: You should post the case CHOP as the error is definitely there I'm feeling so stupid. Never thought about an else statement in the getState... Testing this now. @@Token, I think it's still a problem in the CHOP Case: case CHOP: Entity treeToChop = objects.closest(tree); GroundItem birdNest = groundItems.closest("Bird nest"); if (treeToChop != null && ) { camera.toEntity(treeToChop); treeToChop.interact("Chop down"); StatusUpdate = "Chopping " + tree + " logs"; new ConditionalSleep(10000) { public boolean condition() throws InterruptedException { return treeToChop == null; } }.sleep(); } else{ sleep(random(500,700)); } if (birdNest != null && pickupBirdsNests == true) { birdNest.interact("Take"); } break; EDIT: I've removed the check @bank in the walk case. Testing it again now. It now walks to the tree's even when it's not in the bank. Edited September 20, 2016 by Sebastian Quote Link to comment Share on other sites More sharing options...
Token Posted September 20, 2016 Share Posted September 20, 2016 I'm feeling so stupid. Never thought about an else statement in the getState... Testing this now. @@Token, I think it's still a problem in the CHOP Case: case CHOP: Entity treeToChop = objects.closest(tree); GroundItem birdNest = groundItems.closest("Bird nest"); if (treeToChop != null) { camera.toEntity(treeToChop); treeToChop.interact("Chop down"); StatusUpdate = "Chopping " + tree + " logs"; new ConditionalSleep(10000) { public boolean condition() throws InterruptedException { return treeToChop == null; } }.sleep(); } else{ sleep(random(500,700)); } if (birdNest != null && pickupBirdsNests == true) { birdNest.interact("Take"); } break; Yes it is a problem in the CHOP case as I stated. Invoking interact() on an Entity will create a default InteractionEvent instance which will walk to that Entity if it's too far. InteractionEvent will create a WalkingEvent instance and set it as its child. This WalkingEvent instance will create a LocalPathFinder instance that will generate a path to the closest position from which the target of the InteractionEvent can be accessed. The problem in here is that LocalPathFinder will only generate paths to tiles that are on map, which results in this weird behavior depending on how the current region loaded. Those entities that are not on minimap cannot be walked to, but they are not null so your bot will be stuck there trying to find a path to it. case CHOP: Entity treeToChop = objects.closest(tree); GroundItem birdNest = groundItems.closest("Bird nest"); treeToChop.interact("Chop down"); StatusUpdate = "Chopping " + tree + " logs"; new ConditionalSleep(10000) { public boolean condition() throws InterruptedException { return !treeToChop.exists(); } }.sleep(); if (birdNest != null && pickupBirdsNests == true) { birdNest.interact("Take"); } break; Adjusted some code in there. You already null check the tree in your getState() so the if is redundant. There are many ways to check to check if you can walk to that specific Entity, easiest would be private State getState() { Entity treeToChop = objects.closest(tree); if (inventory.isFull() && powerChop == false) return State.BANK; if(inventory.isFull() && powerChop == true) return State.POWERCHOP; if (treeToChop != null && map.canReach(treeToChop)) return State.CHOP; else return State.WALK; } You can also go with LocalPathFinder lpf = new LocalPathFinder(bot); lpf.findPath(tree); And use lpf.foundPath() As condition to verify if you can walk to your tree. tree.getPosition().isOnMiniMap(bot) Is good but not perfect, it leaves out the 1 in a million case where trees are arranged in such way that the tree is on minimap, but there is no tile on minimap from which you can access it Quote Link to comment Share on other sites More sharing options...
Sebastian Posted September 20, 2016 Author Share Posted September 20, 2016 (edited) @@Token, Thanks! In code this has to work. I'm trying it now. @@Token, Didn't fix it.. Still returning: "Chopping magic tree logs" after 1 run.. EDIT: IT WORKED! @Token you are the man! I hate asking things but every time i ask i learn new things. Edited September 20, 2016 by Sebastian Quote Link to comment Share on other sites More sharing options...
Token Posted September 20, 2016 Share Posted September 20, 2016 @@Token, Thanks! In code this has to work. I'm trying it now. @@Token, Didn't fix it.. Still returning: "Chopping magic tree logs" after 1 run.. Then canReach may not be good enough either, just insert a max distance of 8 or so to your tree as condition 1 Quote Link to comment Share on other sites More sharing options...
Sebastian Posted September 20, 2016 Author Share Posted September 20, 2016 @@Token, It worked bruv. Thanks for all the help! Really appreciate it! Quote Link to comment Share on other sites More sharing options...
Sebastian Posted September 20, 2016 Author Share Posted September 20, 2016 @@Token, it disnt work.. after 7 runs it just gave the same case as normal.... "chopping logs" Do you still have an idea jow to fix this? Quote Link to comment Share on other sites More sharing options...
Token Posted September 20, 2016 Share Posted September 20, 2016 @@Token, it disnt work.. after 7 runs it just gave the same case as normal.... "chopping logs" Do you still have an idea jow to fix this? Just post your whole source code, I doubt there is anything wrong with the code above Quote Link to comment Share on other sites More sharing options...
venetox Posted September 21, 2016 Share Posted September 21, 2016 (edited) I think the problem is the way your handling your cases.I ran into similar problems when I first started doing state based scripts. I would say you have problems elsewhere in your code as well as here. The first thing you should do when making a script imo, is layout the logic that you want the script to follow. In this case it would be like this getAction // First check if the inventory is full IF Inventory Is Full IF PowerMine // If it is and we are supposed to powermine, drop our items return DROP ELSE // If it is and we are not supposed to powermine, go to bank and bank our items. return BANK ENDIF ENDIF // Now check if we are inside the place to cut trees IF CAMELOT_TREES contains myPlayer or EDGEVILLE_TREES contains myPlayer // If we are in one of the places we should cut trees, cut them. return CHOP_TREE else // Otherwise, walk to one of the places we should cut trees return WALK_TO_TREES You should not be checking if the tree is null in your getAction logic, and you should not be checking if you are inside a bank in your walk logic.you should be checking if the tree is null in your Chop logic, and in your walk logic should be checking if you are inside the are you want to chop trees. I just whipped this up with comments to explain how I would go about it.http://pastebin.com/tMJTCQNA Main.java http://pastebin.com/z78vRNJ0 Action.java Haven't tested it since I don't have any bot accounts that can cut yews but I don't see any reason for it to not work. Edited September 21, 2016 by venetox Quote Link to comment Share on other sites More sharing options...