Jump to content

Script help needed! [What not to do & how I can do it better]


Recommended Posts

Posted
  @Override
    public int onLoop() throws InterruptedException {
        if (!inventory.contains("Lobster")&&MONSTER_AREA.contains(myPlayer())){
            if (!BANK_AREA.contains(myPlayer())){
                getLocalWalker().walkPath(PATH_TO_BANK); //walk to bank
            }else{
                RS2Object bankBooth = (RS2Object)this.objects.closest(new String[] {"Bank booth"});
                if (bankBooth != null){
                    if (bankBooth.interact("Bank"))
                    { //checks if it will return true
                        new ConditionalSleep(10000) {
                            @Override
                            public  boolean condition() throws InterruptedException {
                                return getBank().isOpen();
                            }
                        }.sleep();
                        if (getBank().isOpen()){
                            getBank().withdrawAll("Lobster");
                            sleep(random(377, 544));
                        }
                    }
                    bank.close();
                    log("STATE:. BANKED");
                }
            }
        }
        if (!MONSTER_AREA.contains(myPlayer())){
            getLocalWalker().walkPath(PATH_TO_MONSTER);
            log("STATE: WALKING BACK");
        }

This is the code I am dealing with. I want it to go from Varrock Palace to Varrock west bank and use the booth. Then head back to the guards. What am I doing wrong here and how can I improve my code?

Posted

1. The moment you leave monster area you'll stop walking.
2.  

if(bank.isOpen(){
  //do banking
}else{
  bank.open();
}

is prolly more secure

should end this 

if (!inventory.contains("Lobster")&&MONSTER_AREA.contains(myPlayer())){

with an else like 

if (!inventory.contains("Lobster")){
  //bank cheecks
  //walk to bank
  //bank ect
}else{
 //fighting checks
 //walk to fight
 //fight ect
}
  • Like 1
Posted

 

1. The moment you leave monster area you'll stop walking.

2.  

if(bank.isOpen(){
  //do banking
}else{
  bank.open();
}

is prolly more secure

should end this 

if (!inventory.contains("Lobster")&&MONSTER_AREA.contains(myPlayer())){

with an else like 

if (!inventory.contains("Lobster")){
  //bank cheecks
  //walk to bank
  //bank ect
}else{
 //fighting checks
 //walk to fight
 //fight ect
}

Thanks for the response! :D 

Posted (edited)

Thanks for the response! biggrin.png

Actually, I'd say there is nothing wrong with the banking you have. 

if (inventory.isFull() && BANK_AREA.contains(myPlayer().getPosition())) {
if (inventory.isFull()) {
return State.GO_TO_BANK;
}

This is part of my fighting script. It switches to the correct states when needed, I don't see how it can be improved. 

Edited by chad0ck
Posted

Actually, I'd say there is nothing wrong with the banking you have. 

if (inventory.isFull() && BANK_AREA.contains(myPlayer().getPosition())) {
if (inventory.isFull()) {
return State.GO_TO_BANK;
}

This is part of my fighting script. It switches to the correct states when needed, I don't see how it can be improved. 

 

I can: use nodes

public abstract class _Node
{
    protected Script script;
    public _Node(Script script)
    {
        this.script = script;
    }

    public abstract boolean canProcess();
    public abstract void process();

    public void run()
    {
        if (canProcess()) process();
    }
}

public class Bank extends _Node
{
    public Bank(Script script)
    {
        super(script);
    }

    @Override
    public boolean canProcess()
    {
        return script.getInventory().isFull();
    }

    @Override
    public void process()
    {
        //stuff
    }
}


//in onStart
List<_Node> nodes = new ArrayList<_Node>();
nodes.add(new Bank(this));

//in onLoop
nodes.forEach(node -> node.run());
  • Like 1
Posted

 

I can: use nodes

public abstract class _Node
{
    protected Script script;
    public _Node(Script script)
    {
        this.script = script;
    }

    public abstract boolean canProcess();
    public abstract void process();

    public void run()
    {
        if (canProcess()) process();
    }
}

public class Bank extends _Node
{
    public Bank(Script script)
    {
        super(script);
    }

    @Override
    public boolean canProcess()
    {
        return script.getInventory().isFull();
    }

    @Override
    public void process()
    {
        //stuff
    }
}


//in onStart
List<_Node> nodes = new ArrayList<_Node>();
nodes.add(new Bank(this));

//in onLoop
nodes.forEach(node -> node.run());

 

I figured he was still trying to learn the process, if I were him i'd stick to states, they are easy to understand and after you have them down you can learn new processes

Posted

I figured he was still trying to learn the process, if I were him i'd stick to states, they are easy to understand and after you have them down you can learn new processes

 

It's better to learn things like this early on, it's simple and it's gonna remove a lot of redundant code (1000+ lines in onLoop was mainstream anyway, right guys?)

  • Like 1

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 account

Sign in

Already have an account? Sign in here.

Sign In Now
  • Recently Browsing   0 members

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