Jump to content
Search In
  • More options...
Find results that contain...
Find results in...
Buy OSRS Gold

Sell OSRS Gold
im4ever12c

My Script (Works Perfectly, but need feedback)

Recommended Posts

Just trying to get some feedback on ways I could have made my code any different/condenced/maybe even better? I'm new to scripting, and new to java in general, so please give me any feedback, or things I just flat out do wrong?

 

This script essentially removes wine from a banker, mixes it, and re-deposits it while making sure you are not AFK to long and also makes sure that you always have the supplies to continue.

Spoiler

package scripts;

import java.awt.Color;
import java.awt.Font;
import java.awt.Graphics;

import org.tribot.script.interfaces.Painting;
import org.tribot.api2007.Banking;
import org.tribot.api2007.Game;
import org.tribot.api2007.Interfaces;
import org.tribot.api2007.Inventory;
import org.tribot.api2007.Login;
import org.tribot.api2007.Player;
import org.tribot.api2007.Skills.SKILLS;
import org.tribot.api2007.types.RSInterfaceChild;
import org.tribot.api2007.types.RSItem;
import org.tribot.script.Script;
import org.tribot.script.ScriptManifest;

@ScriptManifest(authors = { "IM4EVER12C" }, category = "Cooking", name = "Whiner")
public class WineMaker extends Script implements Painting {

    private boolean hasClickedGrapes = false;
    private boolean hasClickedJugOfWater = false;
    private int winesMade;
    private int actualGrapeAmount;
    RSInterfaceChild inventoryTab = Interfaces.get(548, 53);
    private int AFKtick = 0;
    private int cookingLevelBefore;
    private int maxAFKTicks = 3;
    private int currentAFKTicks = 0;
    private boolean noGrapes = false;
    private boolean noJugs = false;

    RSItem[] Grapes;
    RSItem[] JugOfWater;

    @Override
    public void run() {
        if (cookingLevelBefore < 1) {
            cookingLevelBefore = SKILLS.COOKING.getCurrentLevel();
        }
        while ((currentAFKTicks < maxAFKTicks) && (noJugs == false) && (noGrapes == false)) {
            sleep(loop());
        }
    }

    @Override
    public void onPaint(Graphics g) {
        int alpha = 127;
        int alpha2 = 186;
        Color myColour = new Color(255, 25, 25, alpha);
        Color myColour2 = new Color(1, 1, 125, alpha2);
        Color myColour3 = new Color(255, 255, 255, alpha2);
        g.setColor(myColour);
        g.fillRect(5, 244, 510, 90);
        g.setColor(myColour2);
        g.setFont(new Font("Calibri", Font.BOLD, 35));
        g.drawString("Whiner V1.0", 190, 268);
        g.drawString("___________", 190, 268);
        g.setFont(new Font("Calibri", Font.BOLD, 19));
        // g.drawString("[*]hasClickedGrapes=" + hasClickedGrapes, 10, 330);
        // g.drawString("[*]hasClickedJugOfWater=" + hasClickedJugOfWater, 10, 315);
        g.drawString("[*]Wines Made: " + winesMade, 10, 300);
        g.drawString("[*]Experience Gained: ", 270, 330);
        g.drawString("[*]Cooking Level: " + SKILLS.COOKING.getCurrentLevel() + " ("
                + ((SKILLS.COOKING.getCurrentLevel() - cookingLevelBefore) + ")"), 270, 315);
        // g.drawString("[*]AFK Ticks: " + AFKtick + " / 1000", 270, 300);
        g.setFont(new Font("Calibri", Font.BOLD, 14));
        g.setColor(myColour3);
        g.drawString("" + (winesMade * 200), 446, 329);
    }

    private void bankProcess() {
        if (Banking.isBankLoaded() == false) {
            Banking.openBankBanker();
        } else {
            Banking.depositAllExcept(1937, 1987);
            if (Inventory.getCount(1987) < 14) {
                if (hasClickedGrapes == false) {
                    Banking.withdraw(14, "Grapes");
                    hasClickedGrapes = true;
                }
            }
        }
        if (Inventory.getCount(1937) < 14) {
            if (hasClickedJugOfWater == false) {
                Banking.withdraw(14, "Jug of water");
                hasClickedJugOfWater = true;
            }
        }
    }

    private int loop() {
        if (currentAFKTicks < 3) {
            if (hasClickedGrapes == true) {
                if ((Inventory.getCount("Grapes") < 1) && (Banking.isBankScreenOpen() == false)) {
                    hasClickedGrapes = false;
                    hasClickedJugOfWater = false;
                }
                if ((Inventory.getCount("Jug of water") < 1) && (Banking.isBankScreenOpen() == false)) {
                    hasClickedGrapes = false;
                    hasClickedJugOfWater = false;
                }
                if (Inventory.getCount("Unfermented wine") == 14) {
                    hasClickedGrapes = false;
                    hasClickedJugOfWater = false;
                }
            }
            if (AFKtick == 1000) {
                hasClickedGrapes = false;
                hasClickedJugOfWater = false;
                inventoryTab.click("Inventory");
                AFKtick = 0;
                currentAFKTicks += 1;
            }
            if (((Game.getUptext().contains("Grapes ->")) == true)
                    && ((Inventory.getCount("Grapes") < 1) || (Inventory.getCount("Jug of water") < 1))) {
                inventoryTab.click("Inventory");
            }
            if (((Game.getUptext().contains("Unfermented wine ->")) == true)) {
                inventoryTab.click("Inventory");
            }
            if ((Banking.isBankLoaded() == true) && (Banking.isBankScreenOpen() == true)) {
                RSItem[] nullTest = Banking.find(0);
                RSItem[] myGrapes = Banking.find(1987);
                RSItem[] myJugs = Banking.find(1937);
                if ((myJugs.length == nullTest.length) && (Inventory.getCount(1937) < 1)) {
                    noJugs = true;
                    println("No jugs of water found in bank. Stopping script.");
                } else if ((myGrapes.length == nullTest.length) && (Inventory.getCount(1987) < 1)) {
                    noGrapes = true;
                    println("No grapes found in bank. Stopping script.");
                }
            }
            if (Inventory.getCount("Grapes") < actualGrapeAmount) {
                int x = Math.abs((Inventory.getCount("Grapes") - actualGrapeAmount));
                winesMade += x;
                actualGrapeAmount = Inventory.getCount("Grapes");
            }
            if ((Inventory.getCount(1987) < 1) || (Inventory.getCount(1937) < 1)) {
                bankProcess();
            } else if (Banking.isBankLoaded() == true) {
                Banking.close();
                hasClickedGrapes = false;
                hasClickedJugOfWater = false;
                actualGrapeAmount = Inventory.getCount("Grapes");
            } else {
                if (Player.getAnimation() == -1) {
                    Grapes = Inventory.find("Grapes");
                    JugOfWater = Inventory.find("Jug of water");
                    if (hasClickedGrapes == false) {
                        if ((Game.getUptext().contains("Grapes ->")) == false) {
                            Grapes[0].click("Use");
                            hasClickedGrapes = true;
                        }
                    }
                    if (hasClickedJugOfWater == false) {
                        if (Game.getUptext().contains("Grapes ->")) {
                            JugOfWater[0].click();
                            hasClickedJugOfWater = true;
                        }
                    }
                }
            }
            if ((Interfaces.isInterfaceValid(270) == true) && Interfaces.isInterfaceSubstantiated(270, 13)) {
                RSInterfaceChild rsInterfaceChild = Interfaces.get(270, 13);
                rsInterfaceChild.click();
                hasClickedGrapes = false;
                hasClickedJugOfWater = false;
                AFKtick = 0;
                currentAFKTicks = 0;
                sleep(1000);
            }
            AFKtick += 1;
        } else {
            return 525;
        }
        return 25;
    }
}

 

Share this post


Link to post
Share on other sites
2 hours ago, testing1 said:

avoid writing a script in one class 

Why? He's making a Wine Maker. It fits in one class fine. The real issue is the use of booleans, the lack of API usage and general code cleanliness.

This really could be simplified. I've written a quick example with some pseudo code. You can obviously adjust this to your liking.

   @Override
    public void run() {
        while (true) {
            if (!canMakeWines()) {
                getRequiredItems();
            } else {
                makeWines();
            }
        }
    }

    private boolean canMakeWines() {
        return inventoryContainsAll("Jug of water", "Grapes");
    }

    private boolean inventoryContainsAll(String... items) {
        return Arrays.stream(items).noneMatch(i -> Inventory.getCount(i) == 0);
    }

    private void getRequiredItems() {
        // open bank and withdraw items
    }

    private void makeWines() {
        if (!Player.isAnimating()) {
            if (!Inventory.isUsing("Jug of water")) {
                // select item
            }

            if (Inventory.isUsing("Jug of water")) {
                // if we click grape
                idleWhileMaking();
            }
        }
    }

    private void idleWhileMaking() {
        long lastAnimated = System.currentTimeMillis();
        long idleBreak = General.randomSD(1000, 5000, 2500, 1500);

        while (System.currentTimeMillis() < (lastAnimated + idleBreak)) {
            ABCOld.handleIdleActions();

            if (Player.getAnimation() > -1) {
                lastAnimated = System.currentTimeMillis();
            }
        }
    }

 

  • Like 1

Share this post


Link to post
Share on other sites

Suggestions:

I would avoid writing all of this in one script as mentioned above. Also when checking booleans, you can just do this instead.

if (hasClickedJugOfWater == false) => if (!hasClickingJugOfWater) // false
if (hasClickedJugOfWater == true) => if (hasClickedJugOfWater) // true

 Also, when you are accessing an index in array, make sure you check that it's not out of bounds. Also avoid capitalizing names unless they are classes.

grapes = Inventory.find("Grapes");
if (grapes.length > 0) {
  grapes[0].click();
}

Make sure when you are accessing values you check they are not null first (interfaces)

Keep doing

Your naming conventions are very detailed and make the code easier to read. Great job!

You are pulling out logic into functions that do specific tasks. You can improve this by making functions do only one specific thing!

Share this post


Link to post
Share on other sites
20 hours ago, rs06botHein said:

Its a good start. You could get more structurised by implementing a framework (see link below) and it always helped me to look at some sources of other scripters. GL!

 

Thank you for this, I have never heard of implementing framework to a script. I will definitely look more into this.

@HeyImJamie

Thanks for this as well, i didn't know you could do half of this, I should probably look more into the API documentation before writing and larger scripts, Seeing your example, I could have definitely used a lot of this to condense 90% of what I coded. This will definitely simplify and save me a lot of time in the future!

@boe123

I appreciate the constructive response. It's nice to see what I was doing good vs what I could have done better.  I think all of these responses definitely will help me in the future the more I script. I have messed around with the OutOfBounds check, I made it a little more complicated on myself though in this script comparing the lengths with a random indexed item. This alone will save me a lot of time, Thank you so much for this info.

@testing1

Do you have an example of how you would break this into separate classes, or do you mean more if I ever get into larger constructed scripts?

Share this post


Link to post
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.


  • Our picks

    • Hi everyone,

      I'd like to thank everyone for their patience in this transition period. Since last week, we've worked out the remaining bugs with this integration.

      Some users have still been having issues with connecting their forums account to their Auth0 account. To resolve this, we've imported all forums accounts into Auth0.

      Unfortunately, the accounts which were imported today were using an unsupported password hashing algorithm. Hence, random passwords were set during the import.

      What does this mean for me?

      If you've previously linked your forums account to your Auth0 account, you don't have to do anything. Nothing changes for you.


      If you haven't logged in via our new login yet,

      Try logging in with your forums email address and the last password you used


      If you are unable to login, please use the "Forgot password" tool on the login page:
      Follow the instructions to reset your password
       
        • Like
      • 2 replies
    • Hello everyone,

      Last week we tried to roll out Auth0 Login, but we lost that battle. Now it's time to win the war!

      Important changes

      When logging into the client, you'll now have to enter your Auth0 account credentials instead of your forums credentials

      Note: 2FA is still handled through your forums account (for the time being)



      Changes for existing users

      You'll have to link your Auth0 account to your forums account here: https://tribot.org/forums/settings/login/?service=11


      Auth0 accounts have been created for most existing users. Please use your forums email address and password to login.



      Important notes

      Make sure to verify your email address upon creating a new Auth0 account


      When we mention your Auth0 account, we mean your account used for auth.tribot.org as displayed below
      • 71 replies
    • To better support the upcoming changes (TRiBot X, new repository), we're switching our login handler to Auth0. Instead of logging in with the standard form, you'll now be required to login through our Auth0 application.

      All existing accounts which have been used within approximately the past year have been imported into Auth0 using the same email and password combination which has been stored on the forums.

      What does this mean for users?

      Your account credentials are now even more securely stored


      You'll be able to login via Facebook, Google, and others in the future


      Is there anything users have to do differently now?

      Existing users: You'll have to login with the standard login, open your Account Settings, then link your Auth0 account


      New users: You'll be redirected to our Auth0 app (auth.tribot.org) where you'll be able to create an account


      Why was this change made?

      The new apps we are creating (such as the new repository) aren't able to use the forums to handle user logins


      To centralize all user accounts in one area


      To ensure that the client login doesn't go down when the forums are having problems


      To speed up our development


      Other considerations

      There's no documentation or official support for using Invision Community combined with Auth0, so there are still a few kinks we're working out


      We're in the works of creating an account management panel specifically for Auth0 accounts (ETA August)


      It's not possible to change email addresses for the time being (this will be resolved this August)


      Changing passwords is a weird process for the time being. To change your password, you'll have to use the "Don't remember your password" tool on the Auth0 login page
        • Like
      • 11 replies
    • Over the past month, we've been working hard on TRiBot's new repository - a much needed update. This change has been deemed necessary for TRiBot X, and will allow us to really speed up development of all aspects of TRiBot.

      Today we are going to share what we've been working on!


      Now you must be wondering what kind of features the new repository will have.... well, you'll have to be patient for a little while longer. We're still figuring out various technical aspects so we can't provide answers to all possible questions. We're also focusing on development rather than writing about it so that everyone can get access to our latest developments at lightning speed. I will however answer a few users' questions.

      We're planning on a release of this early to mid August, giving users some goodies before TRiBot X's release.

      Thank you all for being patient. I hope everyone is excited as much as I am!
        • Like
      • 17 replies
    • Over the past few months, I’ve been working diligently on a new project - TRiBot X. Everything has been written from the ground up, with all of the best practices of software engineering. Every aspect of TRiBot has been re-imagined to support three main goals: flexibility, useability, and reliability.
        • Like
      • 50 replies
  • Recently Browsing   0 members

    No registered users viewing this page.

×
×
  • Create New...