The client is offline due to RuneScape update. You may see instance errors but this is due to the update and not actual instance errors. This should be resolved soon. Join our Discord for more information.

Script writer application


  • Script Writer


  • Script Writer

    Overall it's much better than before. Some small things I would change:

    • There's no reason to use an AtomicInteger.
    • Using an AtomicReference is again, useless.
    • public static non-final is bad, very bad mkay. Including this:
        public static final AtomicReference<EnchantItem> CURR = new AtomicReference<>(EnchantItem.RECOIL);
    
    • This null-check is useless
            Item item = Inventory.getFirst(CURR.get().getBaseId());
            if (item != null && Magic.cast(CURR.get().getSpell(),item)) {
                Log.info("Casting enchant " + nextDelay);
                Time.sleepUntil(()->EXP_TRACKER.getLastExpTimer().getElapsed().toMillis() < 500, 2000);
            }
    
    • Should get getProfit
    public int profit()
    

    Other than that the script is fine. It is pretty basic however.


  • Director

    I don't really like how your conditionals are setup, for example:

      private boolean withdrawStaff() {
            if (!playerNoStaff()) return true;
             Bank.withdraw(CURR.get().getStaffId(),1);
            return false;
        }
    
        private boolean playerNoStaff() {
            return !Equipment.contains(CURR.get().getStaffId()) && !Inventory.contains(CURR.get().getStaffId());
        }
    
    

    It's technically correct but its confusing because of how you check the inverse condition on most things.

    You should write conditionals to be the positive outcome instead of the negative, for example, it should be.

    private boolean playerHasStaff() { 
       return Equipment.contains(CURR.get().getStaffId()) || Inventory.contains(CURR.get().getStaffId());
    } 
    

    Then its much more clear too say

    if(playerHasStaff()) { return true; }
    

    https://forums.rspeer.org/topic/1274/muling-snippet is a bit too nested, lots of if / else, when guard clauses would be cleaner. https://refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html

    The snippet itself is very basic and hard to judge on. It is missing a lot of things that would be helpful for basic muling. For example, what happens if your bank has the items but your inventory doesn't?

    What happens if the mule is in a trade? Seems like it would just continuously trade it over and over. It should not do that.


    Over all, I need more code to see, there is not enough here to really judge, the scripts are too simple. I would suggest making a more complex script that would allow us to judge a bit better.

    Some things that will help us judge better

    • Grand exchange support for restocking / buying required items.
    • Actual full muling

    I cannot accept or deny you at this time, I need to see more code first.


  • Script Writer

    I agree with Mad. Also i don't really get why you would use an ID class containing so much data. It'd be better to just store the data that is needed for this practicular script it self rather than having useless id's inside it.


  • Script Writer

    @qverkk It seems a bit ridiculous for this example, but I use the same ID file in all my scripts imported from my api module, this script is already using about 40+ ids. With all my other scripts I'm probably using over 100 ids. I'd rather not have 10 different ID files or be placing random constants in my script classes.


Locked
 

28
Online

16.3k
Users

1.4k
Topics

19.1k
Posts