Jump to content
×
×
  • Create New...

[On Hold] Application for SW I


nsc
 Share

Recommended Posts

Hey everyone,

I had written an Essence Miner for my IM that I have been using for some time, and thought others could find it helpful as well as I saw some people asking for one in the script request channel on Discord.

The activity itself is quite straightforward so I tried to add some extra features in it, listed below.

  • Identifies best pickaxe to use from the available pickaxes on player's account
  • Progressive pickaxe upgrading as our Mining level increases (as long as we have better pickaxes available on the account)
  • Checks to see if we can equip pickaxes
  • Customizable stop conditions (# of Essence mined or certain Mining level reached)

 

https://github.com/nschaa/Essence-Miner/tree/main/src/main

 

At the end of the day, I think it's still a rather simple script and I'm sure there is room for improvement, so any feedback from the more experienced SW's would be much appreciated!

 

 

*Thanks to all the SW's who still left their application's code on public, especially (@ADivorcedFork & @Dogue) as they were extremely helpful.

Link to post
Share on other sites

  • Moderators

Hi mate, unless you have a different username on discord I don't think we've spoke so welcome!

It's nicely written tbh. However feedback wise; try not to overcomplicate things. An essence miner as you said yourself, is very simple. As such, doesn't really need a task priority as the t asks shouldn't overlap, i.e. when you want to mine, you shouldn't need to bank due to the validations of only mine if inventory space for example.

I'm really not a big fan of unconditional sleeping; see below.

(logic)
https://github.com/nschaa/Essence-Miner/blob/28ebbf7f2ffd58b5535d06f98abefd3cc0aabbc1/src/main/Logic/UtilityLogic.java#L59
You use conditional waits throughout your script so I can only assume it's supposed to be some sort of anti-pattern? Although in doing so, you're creating more of a pattern. For example you call the sleep before depositing every time, that in itself seems a tad sus.

Also just be careful forcing users to equip a pickaxe when they may not have the attack level.

However that's really the only feedback I had, as I said earlier, it's nicely written. Will reach out to some of the others for more feedback.

Despite it being nicely written, I'd like to see something use more of the API, something a little more complex rather than a walk, mine, walk bank script in order to move forward with the application.

Link to post
Share on other sites

  • Dan changed the title to [On Hold] Application for SW I
  • Moderators

Also, I forgot to check your main class, thanks to dogue who pointed it out;

what’s going on with your poll()? Seems like a second on start with a while loop? Poll() itself is a loop and loops constantly. Would suggest tidying this up as well.

Link to post
Share on other sites

3 hours ago, Dan said:

Hi mate, unless you have a different username on discord I don't think we've spoke so welcome!

It's nicely written tbh. However feedback wise; try not to overcomplicate things. An essence miner as you said yourself, is very simple. As such, doesn't really need a task priority as the t asks shouldn't overlap, i.e. when you want to mine, you shouldn't need to bank due to the validations of only mine if inventory space for example.

I'm really not a big fan of unconditional sleeping; see below.

(logic)
https://github.com/nschaa/Essence-Miner/blob/28ebbf7f2ffd58b5535d06f98abefd3cc0aabbc1/src/main/Logic/UtilityLogic.java#L59
You use conditional waits throughout your script so I can only assume it's supposed to be some sort of anti-pattern? Although in doing so, you're creating more of a pattern. For example you call the sleep before depositing every time, that in itself seems a tad sus.

Also just be careful forcing users to equip a pickaxe when they may not have the attack level.

However that's really the only feedback I had, as I said earlier, it's nicely written. Will reach out to some of the others for more feedback.

Despite it being nicely written, I'd like to see something use more of the API, something a little more complex rather than a walk, mine, walk bank script in order to move forward with the application.

 

Hi @Dan, thank you for the comments! I agree that having a task system is overkill for something like this, I just roughly imported over the same structure and needed logic for my previous scripts on another client. This is also the reasoning for the unconditional sleeps, as randomized sleeps were a standard part of antiban measures for any script on those clients. I'm not that familiar with the necessity of antiban measures for mobile botting however, so if they are not necessary then I am happy to remove them.

As for the equipping of pickaxes, I did take that into account and had checks to recognize if players had the necessary Attack levels to equip their pickaxes so hopefully that shouldn't cause any issues.

 

2 hours ago, Dan said:

Also, I forgot to check your main class, thanks to dogue who pointed it out;

what’s going on with your poll()? Seems like a second on start with a while loop? Poll() itself is a loop and loops constantly. Would suggest tidying this up as well.

Ah I see, I did not realize Poll() itself was a loop. I have made the necessary changes and removed the runScript() loop now. 

 

3 hours ago, Dan said:

Despite it being nicely written, I'd like to see something use more of the API, something a little more complex rather than a walk, mine, walk bank script in order to move forward with the application.

That's totally understandable. I just wanted to put up a script that utilizes some components of the API to make sure there would be no glaring errors before committing to writing more scripts. I have a coupe more I'm working on, and I'll update this application with another script then. Thanks again for the feedback! 

Link to post
Share on other sites

  • Moderators

You're welcome for the feedback 🙂 You're clearly capable but I'd just recommend getting familiar with the api we have and maybe develop something from scratch rather than just port something. Maybe that way you'll get a better feel for how things work. idk; just a suggestion.

Will leave this on hold for now; just ping when you've updated it.

Link to post
Share on other sites

26 minutes ago, Dan said:

You're welcome for the feedback 🙂 You're clearly capable but I'd just recommend getting familiar with the api we have and maybe develop something from scratch rather than just port something. Maybe that way you'll get a better feel for how things work. idk; just a suggestion.

Will leave this on hold for now; just ping when you've updated it.

Haha, fair enough. I'll look through the private script request channels on Discord and see if there's one which I can start from scratch. I think I saw someone asking about a Vyrewatch Sentinels Killer, so maybe I'll work on that instead. Will @ you here when that's complete.

  • Like 1
Link to post
Share on other sites

  • Moderators

Just some notes from the quick look at I had.

 

The following could be replaced with exp gained calculation(From starting) or using an inventory listener to see essence entering inventory

 


        if (Skill.Mining.experience() > lastXPCount) {
            Variables.get().essenceGained++;
            System.out.println("Essence gained: " + Variables.get().essenceGained);
            lastXPCount = Skill.Mining.experience();
            lastActivityTime = System.currentTimeMillis();
        }

 

Good to see this this. Some minor improvements

if ((UtilityLogic.timeFromMark(lastActivityTime)) >= 120000) {
            System.out.println("No experience gained for over 2 minutes, failsafe activated. Shutting script down.");
            onStop();
        }
  • ScriptManager.INSTANCE.stop(); should be called instead of OnStop() as the intent for that is when it is already stopping so you shouldn't need to call it again.
  • Should call "return" after this as otherwise it will execute tasks still.

 

BankLogic

  • Withdraw always returns false even if successfull?
  • bankisOpen name is desceptive, it should be openBank

Some considerations

  • Using the Paint API - https://docs.powbot.org/#/Basic_Fundamentals/PaintBuilderAPI
  • Why do tasks executing specific logic need a reference to the main for status? It's not something the task should have to be concerned about
  • Interacting with inventory items does not need to check the tab is open, the API handles that.
Link to post
Share on other sites

Hey @nsc@Ptyand @Danmade some good suggestions so far. I'll go through the code and mention some things as I see them:

Files

Constants.java

  • Make sure to use correct formatting on all final variables here. You did it for everything but colors:
    // COLORS:
    public static final int sailorBlue = Color.argb(90,0, 68, 102);
    public static final int Mint = Color.argb(255,0, 255, 162);

 

  • Your pickaxe arrays along with EquipmentLogic.java and especially Banking.java getAllUsablePickaxes() could be cleaned up a lot by making an enum class. Enumerations are super powerful and useful in Java. Here's an example:
package main.Data;

import java.util.ArrayList;
import java.util.List;

public enum Pickaxe {
    DRAGON("Dragon pickaxe", 61),
    RUNE("Rune pickaxe", 41),
    ADAMANT("Adamant pickaxe", 31),
    MITHRIL("Mithril pickaxe", 21),
    STEEL("Steel pickaxe", 11),
    IRON("Iron pickaxe", 0);

    private String itemName;
    private int requiredMiningLevel;

    Pickaxe(String itemName, int requiredMiningLevel) {

        this.itemName = itemName;
        this.requiredMiningLevel = requiredMiningLevel;
    }

    public static Pickaxe bestPickaxeFromLevel(int miningLevel) {
        for (Pickaxe pickaxe : Pickaxe.values()) {
            if (miningLevel >= pickaxe.getRequiredMiningLevel()) {
                return pickaxe;
            }
        }
        return null;
    }

    public static List<Pickaxe> allPickaxesFromLevel(int miningLevel) {
        List<Pickaxe> pickaxes = new ArrayList<>();
        for (Pickaxe pickaxe : Pickaxe.values()) {
            if (miningLevel >= pickaxe.getRequiredMiningLevel()) {
                pickaxes.add(pickaxe);
            }
        }
        return pickaxes;
    }

    public static String[] allPickaxeNamesFromLevel(int miningLevel) {
        List<String> pickaxeNames = new ArrayList<>();
        for (Pickaxe pickaxe : Pickaxe.values()) {
            if (miningLevel >= pickaxe.getRequiredMiningLevel()) {
                pickaxeNames.add(pickaxe.itemName);
            }
        }
        return pickaxeNames.toArray(new String[0]);
    }

    public String getItemName() {
        return itemName;
    }

    public int getRequiredMiningLevel() {
        return requiredMiningLevel;
    }
}

 

InventoryLogic.java

  • Following up, by using the enum above you can simplify a lot more logic. For instance, getPickaxeInInventory() can turn into:
    public static boolean containsPickaxe() {
        return Inventory.stream().name(Pickaxe.allPickaxeNamesFromLevel(Skill.Hunter.realLevel())).isNotEmpty();
    }

 

Variables.java

  • I'm just now noticing your availablePickaxes variable. It looks like you're adding a lot of logic in your project to keep this variable updated. Why not just query your inventory/equipment/bank for the available pickaxes instead of storing them in a variable? This seems like this could be prone to error as well. Say for instance, a user pauses the script and sells a pickaxe. 
  • You should be able to get rid of most of your pickaxe variables, with the exception of maybe currentPickaxe, which you can update every time you level up in mining. The rest of the variables are redundant or can be checked in real-time. 

 

Misc

  • Refer to the below application feedback for some additional suggestions. Specifically sleeps and guard clauses.  
  • Look at using PaintBuilder for paints. I believe it's preferred. 

 

Overall

I don't think it's a bad script per se, but there are several points mentioned above that need to be addressed. These issues aren't script breaking yet, but if you follow the same habits when developing larger scripts, the issues will become more and more apparent. 

It does seem you understand a lot of the basic mechanics with the API, specifically how to interact/query the game, so I'm interested to see this script after you refactor. Be sure to ping when you push those updates to github and I'll do a follow up.

 

Best of luck!

 

-Fork 

Link to post
Share on other sites

Please sign in to comment

You will be able to leave a comment after signing in



Sign In Now
 Share