zezima247 0 Share Posted April 13, 2020 hello everybody, im new to scripting but not new in the osrs community, im busy with a simple thieving script. first everything whas running smooth and after a couple changes the code broke for some reason and now it only pickpocks when the bag is not full but when it full it needs to bank and first it worked and after some changes it didnt work anymore. please can somebody tell me what im doing wrong. my code is maybe a little shit but im still learning and also im dutch and not english so please dont come with fancy words on me :P. Area thegunner_area = new Area( new Tile (2644,3227,0), new Tile (2641,3228,0)); Area bank_area = new Area( new Tile (2652,3283,0), new Tile (2655,3283,0)); @Override public void onStart() { log("starting"); } @Override public int onLoop(){ NPC thegunner = getNpcs().closest(3599); if (getInventory().isFull()) { if (thegunner_area.contains(getLocalPlayer())) { getWalking().walk(bank_area.getRandomTile()); sleep(1000,2000); } else getBank().open(); getBank().depositAll(6962); } if (!getInventory().isFull()) { if (bank_area.contains(getLocalPlayer())) { getWalking().walk(thegunner_area.getRandomTile()); sleep(1000,2000); } else thegunner.interactForceRight("Pickpocket"); } return 1500; } @Override public void onExit(){ log("stopppen"); } } Link to comment Share on other sites More sharing options...
NovaGTX 106 Share Posted April 13, 2020 I've posted about this type of stuff before. Here's a quote from that should help. On 12/15/2019 at 1:02 AM, NovaGTX said: I would suggest you look into proper structure of a script rather than using a bunch of nested if statements. This is a common mistake for beginners. For someone who isn't as well versed in scripting structure I would recommend you use a State structure rather than Nodes which is more complicated. Here's an excerpt i've posted before that should help you: Your overall layout could be improved by using an enum for the state you want to execute. I could get into nodes, this is probably out of scope for you currently, but I digress. Using enums for states is a less complicated and more entry-level method to sectioning your script rather than have it loop through conditionals in the onLoop each time. Declare your state: Private State state; Declare the enums: enum State { WALKING, TELEPORT } Next you need to define them using a function: private State getState() { if(!getLocalPlayer().isAnimating() && checkQP == 69){ return State.TELEPORT; } else if(!getLocalPlayer().getTile().equals(lumb)){ //You really should have a verification for teleporting, but for simplicity im omitting that return State.WALK; } else { return State.SLEEP; } } Then within your onloop you can use a Switch statement to check your state. public int onLoop() { state = getState(); //this is to avoid issues with calling the getState() function directly, can cause lag. switch(state){ case TELEPORT: //teleport here break; case WALK: //walk here break; } return 100; } Hopefully, this quick and dirty example can give you some insight that you can improve upon. fluffy 1 Link to comment Share on other sites More sharing options...
Moistie 0 Share Posted April 13, 2020 Notice how NovaGTX has structured his if/else statements and how every open curly brace has a matching closing brace, your code seems to have missing braces. Also, indenting the code 4 spaces in every section can make it more obvious where a brace should go to finish the IF Link to comment Share on other sites More sharing options...
zezima247 0 Author Share Posted April 13, 2020 26 minutes ago, NovaGTX said: I've posted about this type of stuff before. Here's a quote from that should help. thank you for the quick respons, i already seen that post and i will structure my script later on. i fixed my script a little bit now when im at the bank it banks all the stuf and walks back to pickpocket, but now when the bag is full again it wont walk back to the bank. is this still structure problem or is my code just invalid?: Area thegunner_area = new Area( new Tile (2644,3227,0), new Tile (2641,3228,0)); Area bank_area = new Area( new Tile (2652,3283,0), new Tile (2655,3283,0)); @Override public void onStart() { log("starting"); } @Override public int onLoop(){ NPC thegunner = getNpcs().closest(3599); if (getInventory().isFull() && (thegunner_area.contains(getLocalPlayer()))) { getWalking().walk(bank_area.getRandomTile()); sleep(1000,2000); } else { if (bank_area.contains(getLocalPlayer()) && (getInventory().isFull())) { getBank().open(); getBank().depositAll(6962); } } if (!getInventory().isFull()) { if (bank_area.contains(getLocalPlayer()) && !getInventory().isFull()) { getWalking().walk(thegunner_area.getRandomTile()); sleep(1000,2000); } else getWalking().walk(thegunner_area.getRandomTile()); thegunner.interactForceRight("Pickpocket"); } return 1500; } @Override public void onExit(){ log("stopppen"); } } AsBakedAsCake 1 Link to comment Share on other sites More sharing options...
Moistie 0 Share Posted April 13, 2020 You're still missing brackets so it's probably skipping code you expect it to run, or running code you expect it to skip. Create a STATE structure as per Nova's recommendation. But also your logic seems to only take 1 step towards the bank when it's full because then the next loop when it's meant to keep walking if not at the bank, since it's now no longer standing in the thegunner_area it wont fall into that statement. Try something like : if(inventory is full){ if(in bank area){ if(bank not open){ open bank } else{ do banking } } else{ walk to bank } } else { if(in gunner area){ pickpocket } else { walk to gunner } } Link to comment Share on other sites More sharing options...
zezima247 0 Author Share Posted April 13, 2020 18 minutes ago, Moistie said: You're still missing brackets so it's probably skipping code you expect it to run, or running code you expect it to skip. Create a STATE structure as per Nova's recommendation. But also your logic seems to only take 1 step towards the bank when it's full because then the next loop when it's meant to keep walking if not at the bank, since it's now no longer standing in the thegunner_area it wont fall into that statement. Try something like : if(inventory is full){ if(in bank area){ if(bank not open){ open bank } else{ do banking } } else{ walk to bank } } else { if(in gunner area){ pickpocket } else { walk to gunner } } still not working, i have this now and still after pickpocking the npc the player stops moving and dont do the banking process. but when i walk to the bank and start the script from there it banks al the stuff and walk back to the npc to pickpock and when the inventory is full again the player stops moving. so i dont get it, i dont think STATE structure will solve this problem because with simple if else statements it needs to work. or there is invalid code in my script. this is what i have now: @Override public void onStart() { log("starting"); } @Override public int onLoop(){ NPC thegunner = getNpcs().closest(3599); if (getInventory().isFull()) { if (bank_area.contains(getLocalPlayer())) { if(getBank().isOpen()) { getBank().open(); } else { getBank().depositAll(6962); } } else { getWalking().walk(bank_area.getRandomTile()); } } else { if (thegunner_area.contains(getLocalPlayer())) { thegunner.interactForceRight("Pickpocket"); } else { getWalking().walk(thegunner_area.getRandomTile()); } } return 1500; } @Override public void onExit(){ log("stopppen"); } } AsBakedAsCake 1 Link to comment Share on other sites More sharing options...
Moistie 0 Share Posted April 13, 2020 add some logging and check which if statement it's falling into Link to comment Share on other sites More sharing options...
zezima247 0 Author Share Posted April 13, 2020 5 hours ago, Moistie said: add some logging and check which if statement it's falling into i didnt change anything in the code besides some log(""); in the else statements and now everything works fine . i dont know why but hopefully you can tell me that? Link to comment Share on other sites More sharing options...
Moistie 0 Share Posted April 13, 2020 You probably didn't recompile the script properly. Link to comment Share on other sites More sharing options...
AsBakedAsCake 200 Share Posted April 13, 2020 (edited) Sorry, may be a bit brutal - but.. if(getBank().isOpen()) { getBank().open(); ^ Basically here, you're saying if the bank IS open, open bank. Why would you want to open the bank if it was already open? You're also not null checking the NPC and looks like you're missing brackets. Also why are you using interact.ForceRight? All you need is thegunner.interact("Pickpocket); - no reason to add a forced right click. With that being said, looks like you need to look into more scripting tutorials. I feel as if you could use some work on your if/else statements. Edited April 13, 2020 by AsBakedAsCake Link to comment Share on other sites More sharing options...
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now