Jump to content

Script review for my flax spinner

flewis

By flewis
in Scripting Help

 Share

Recommended Posts

flewis Iron Poster

flewis

Posted
Posted

Hey all,

Wrote my first fully functioning script (first Java project also) and was just wondering what I could do to improve it in terms of making the code more efficient and also better practices I could have used. I feel it looks to Pythonic to compared to other script source codes I've looked at. Python is the programming language I took in computer science so it makes sense but on a journey to learn Java I will need to drop old habits. Below is the entire code. Any feedback is greatly appreciated :)

Script features:

  • Will spin flax in Lumbridge castle
  • Banks on the top floor
  • Can be stopped and started anytime and will figure out where you left off.
  • Will stop when the user has no more flax in the bank
  • Has mechanisms to deal with lag for example if widgets don't open ETC
  • Tested on all server locations 
import org.osbot.rs07.api.map.Area;
import org.osbot.rs07.api.map.Position;
import org.osbot.rs07.api.model.Entity;
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 java.awt.*;

@ScriptManifest(name = "Flax Spinner", author = "Flewis", version = 1.0, info = "", logo = "")

public class FlaxSpinner extends Script {


    // Variable for the current stage of the bot
    private int currentAction = 0;

    // Used to check to see if a run has started
    private boolean started = false;

    // Location of the bank
    private final Area bankArea = new Area(new Position(3207, 3217, 2), new Position(3210, 3220, 2));

    // Location of the wheel
    private final Area wheelArea = new Area(new Position(3208, 3212, 1), new Position(3213, 3216, 1));


    @Override

    public void onStart() {

        //Code here will execute before the loop is started
        if (getInventory().contains("Flax")) {
            currentAction = 2;
        }

    }



    @Override

    public void onExit() {

        //Code here will execute after the script ends

    }

    @Override

    public int onLoop() throws InterruptedException {

        switch(currentAction) {
            // Walks to the bank
            case 0:
                getWalking().webWalk(bankArea);
                currentAction++;
            // Open bank, bank bowstring (if possible), take out flax, close bank
            case 1:
                getBank().open();

                if (getBank().isOpen()) {
                    getBank().depositAll();
                    Script.sleep(random(500, 1500));
                    if (getBank().getAmount("Flax") == 0) {
                        log("Stopped due to having no flax!");
                        stop();
                    }
                    getBank().withdraw("Flax", random(28,436));
                    Script.sleep(random(500, 1500));
                    getBank().close();
                    currentAction++;
                }
            // Walks to the wheel
            case 2:
                if (!getInventory().contains("Flax")) {
                    currentAction = 0;
                    return random(500, 1200);
                }

                getWalking().webWalk(wheelArea);
                currentAction++;
                started = false;
            // Spin the flax
            case 3:
                if (!widgets.isVisible(233) && started && !getInventory().contains("Flax")) {
                    return random(1000, 1500);
                }

                if (widgets.isVisible(233)) {
                    started = false;
                }

                if (!getInventory().contains("Flax") && wheelArea.contains(myPlayer())) {
                    currentAction = 0;
                } else {
                    Entity spinningWheel = getObjects().closest("Spinning wheel");
                    if (spinningWheel != null) {
                        spinningWheel.interact("Spin");
                        Script.sleep(random(500, 1500));
                        RS2Widget bs = getWidgets().get(270, 16, 38);
                        if (bs != null) {
                            getKeyboard().pressKey(' ');
                            Script.sleep(random(100, 250));
                            getKeyboard().releaseKey(' ');
                            started = true;
                        }
                    }
                }
        }

        return random(1000, 1500); //The amount of time in milliseconds before the loop starts over

    }

    @Override

    public void onPaint(Graphics2D g) {

        //This is where you will put your code for paint(s)

    }

}

Thank you :)

-Flewis

Link to comment
Share on other sites
Nbacon Black Poster

Nbacon

Posted
Posted (edited)

Hello Flewis,

The code works pat yourself on the back. Its a good start. The logic is most important part of a bot and  ever sagement is broken in to a logic spot. BUT using a switch statement like this is like using goto and we can do better.  If think like a state machine were all action is true false pair. Programing becomes fast and rapid becuase you can use "and then" and "what if" thinking. 

  • Will spin flax in Lumbridge castle. Yes
  • Banks on the top floor. Yes
  • Can be stopped and started anytime and will figure out where you left off. It does but there like goto.
  • Will stop when the user has no more flax in the bank. Yes.
  • Has mechanisms to deal with lag for example if widgets don't open ETC. Check for nulls is great becuase they will crash the client but for lag look into ConditionalSleep.
  • Tested on all server locations. No idea what this means.

Two tips limit accounts per loop and use conditionals logic not switches. (switches have a place like during quest ).

This is how I would do it . I  just a rearaging of your code to limit actions per loop and tried to explain  the thinking behind it.

step 1: break the bot in to 2 logical opposites like has bow stirngs or does not have bow strings.   (you can use this pattern for 95% of banking skills)

Quote 

   if (!getInventory().contains("Flax")){
        
   }else{
       
   }

 

step 2: Think to yourself If i dont have not flax were am i and how do i get ride of this not flax?

Quote 

if (!getInventory().contains("Flax")){
	if ( ! bankArea.contains(myPlayer())){//OP code did not have this but it is super usefull
          // I need to get to the bank bank my not flax
	}else if (getBank().open()){//open() uses opened() to check if it should go off and returns true both ways
          //Im at the bank and still have dont have flax
	}
}else {
  
}

 

step 3: fill in the code to get you to the bank, bank your items and with draw the flax. Once this is done the logic for getting flax is done. 

Quote 

if (!getInventory().contains("Flax")){
    if ( ! bankArea.contains(myPlayer())){
        getWalking().webWalk(bankArea);
    }else if (getBank().open()){
        if (!getInventory().empty()){
            getBank().depositAll();//line 0 still holds true we do not have flax.
        }else {
            if (getBank().getAmount("Flax") == 0) {
            	log("Stopped due to having no flax!");
            	stop();
            }
            getBank().withdraw("Flax", random(28,436)); //this will make line 0 fail and goto the else statment
			//also I dont think the random is needed just setup your bot when its in the bank to have withdraw all or x 
			//and put a large number in.
        }
    }
}else {

}

 

Step 4:  The else logic we have flax, we could be at the bank, or going to the sping wheel or making bowstrings.

 

Quote 

if (!getInventory().contains("Flax")){
    if ( ! bankArea.contains(myPlayer())){
        getWalking().webWalk(bankArea);
    }else if (getBank().open()){
        if (!getInventory().empty()){
            getBank().depositAll();
        }else {
            if (getBank().getAmount("Flax") == 0) {
                log("Stopped due to having no flax!");
                stop();
            }
            getBank().withdraw("Flax", random(28,436));
        }
    }
}else {//we do have flax now
    if (getBank().isOpen()){// this is were we left off on the if (no flax)
	//we have to close the bank
    }else if (!wheelArea.contains(myPlayer())){
	//we need to get to the wheel
    }else if (myPlayer().getAnimation()==-1){
 	//we need to use the wheel
    }else {
    //we need to do nothing. because flax is turning into not flax
    }

}

 

 

step 5 fill in and done.

Quote 

if (!getInventory().contains("Flax")){
    if ( ! bankArea.contains(myPlayer())){
        getWalking().webWalk(bankArea);
    }else if (getBank().open()){
        if (!getInventory().empty()){
            getBank().depositAll();
        }else {
            if (getBank().getAmount("Flax") == 0) {
                log("Stopped due to having no flax!");
                stop();
            }
            getBank().withdraw("Flax", random(28,436));
        }
    }
}else {
    if (getBank().isOpen()){
        getBank().close();
    }else if (!wheelArea.contains(myPlayer())){
        getWalking().webWalk(wheelArea);

    }else if (myPlayer().getAnimation()==-1){ //Animation -1 is standing
        Entity spinningWheel = getObjects().closest("Spinning wheel");
        if (spinningWheel != null) {
            spinningWheel.interact("Spin");
          
          
            new ConditionalSleep(500, 500) { // this is a better way to deal with lag.
                @Override
                public boolean condition() throws InterruptedException {
                    return  null!=getWidgets().get(270, 16, 38)  && getWidgets().get(270, 16, 38).isvisible() || myPlayer().getAnimation()==-1;
                }
            }.sleep();
          
            RS2Widget bs = getWidgets().get(270, 16, 38) ;

          
            if (bs != null) {
                keyboard.typeKey(' ');
                //this is only true if you 
                // never use the sping wheel for any thing else
            }
        }
    }else {
        //does nothing but could put a wait here
    }

}

 

 

 

 

 

 

 

 

Edited by Nbacon
spelling and grammer
  • Like 3
Link to comment
Share on other sites
Efpkaf Steel Poster

Efpkaf

Posted
Posted (edited)

Instead of using currentAction as integer, use enum. You could extract every step into other class which implements interface A and in enum parameter set implementation of this interface for example: 

interface Action{
	void doThing();
}

class A implements Action{
	public void doThing(){...}
}


class B implements Action{
	public void doThing(){...}
}

enum ActionType{
  BANK(new A()), FOO(new B())...
  
    @Getter
  private final Action action;
}

 

use private final static variables instad of private final 

Edited by Efpkaf
  • Like 1
Link to comment
Share on other sites
flewis Iron Poster

flewis

Posted
  • Author
Posted
9 hours ago, Nbacon said:

Hello Flewis,

The code works pat yourself on the back. Its a good start. The logic is most important part of a bot and  ever sagement is broken in to a logic spot. BUT using a switch statement like this is like using goto and we can do better.  If think like a state machine were all action is true false pair. Programing becomes fast and rapid becuase you can use "and then" and "what if" thinking. 

  • Will spin flax in Lumbridge castle. Yes
  • Banks on the top floor. Yes
  • Can be stopped and started anytime and will figure out where you left off. It does but there like goto.
  • Will stop when the user has no more flax in the bank. Yes.
  • Has mechanisms to deal with lag for example if widgets don't open ETC. Check for nulls is great becuase they will crash the client but for lag look into ConditionalSleep.
  • Tested on all server locations. No idea what this means.

Two tips limit accounts per loop and use conditionals logic not switches. (switches have a place like during quest ).

This is how I would do it . I  just a rearaging of your code to limit actions per loop and tried to explain  the thinking behind it.

step 1: break the bot in to 2 logical opposites like has bow stirngs or does not have bow strings.   (you can use this pattern for 95% of banking skills)

step 2: Think to yourself If i dont have not flax were am i and how do i get ride of this not flax?

step 3: fill in the code to get you to the bank, bank your items and with draw the flax. Once this is done the logic for getting flax is done. 

Step 4:  The else logic we have flax, we could be at the bank, or going to the sping wheel or making bowstrings.

 

 

step 5 fill in and done.

 

 

 

 

 

 

 

 

7 hours ago, Efpkaf said:

Instead of using currentAction as integer, use enum. You could extract every step into other class which implements interface A and in enum parameter set implementation of this interface for example: 


interface Action{
	void doThing();
}

class A implements Action{
	public void doThing(){...}
}


class B implements Action{
	public void doThing(){...}
}

enum ActionType{
  BANK(new A()), FOO(new B())...
  
    @Getter
  private final Action action;
}

 

use private final static variables instad of private final 

Thank you so much guys :)

Been a huge help honestly I can't express my gratitude enough!

<3

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
 Share
Go to topic listing

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...