Code review - Simple Edgeville Steel Smelting

[katastrophi 0]

Hi there,

First time having a play with the API. Let me know if you have any tips or feedback. This is made through following various bits of advice on other threads.

Some things I struggled with

- Not a huge fan of the way you end up with so many conditional statements which have side effects but that's just the nature of the API.
- I was initially using the BankLocation enum for Edgeville but it was all over the place with which booth I was ending up at that it felt like it was trying too hard not to be a bot.
- With smelting the animation starts and stops so it wasn't actually that helpful using the animation int. I'm not sure it's necessary in my code but it helped sometimes it randomly wanted to click on the furnace again whilst smelting and that protected against it a little bit extra.

 

@ScriptManifest(author = "Katastrophi", name = "Edgeville Steel Smelting", version = 1.0, description = "Simple Smelter", category = Category.SMITHING)
public class main extends AbstractScript {
    Task currentTask = Task.BANK;


    private static final int ANIMATION_SMELTING = 899;

    private static final Area bankArea = new Area(3094, 3495, 3098, 3498);
    private static final Area smeltArea = new Area(3109, 3500, 3107, 3497);

    private enum Task {
        SMELTING, BANK
    }

    @Override
    public int onLoop() {
        switch (currentTask) {
            case BANK:
                if(getBank().isOpen()){
                    if(getInventory().isFull()){
                        if(getBank().close()){
                            currentTask = Task.SMELTING;
                        }
                    }else if (getInventory().contains("Steel bar")) {
                        if (getBank().depositAllItems()) {
                            MethodProvider.sleepUntil(() -> getInventory().isEmpty(), 1500);
                        }
                    } else {
                        if (getInventory().contains("Iron ore")) {
                            if (getBank().withdrawAll("Coal")) {
                                MethodProvider.sleepUntil(() -> getInventory().contains("Coal"), 1500);
                            }
                        } else if (getBank().withdraw("Iron ore", 9)) {
                            MethodProvider.sleepUntil(() -> getInventory().contains("Iron ore"), 1500);
                        }
                    }
                }else{
                    if(getInventory().contains("Iron ore") && getInventory().contains("Coal ore")){
                        currentTask = Task.SMELTING;
                    }else{
                        if (!bankArea.contains(getLocalPlayer())) {
                            if(getWalking().walk(bankArea.getRandomTile())){
                                MethodProvider.sleepUntil(() -> getWalking().shouldWalk(), 6000);
                            }
                        }else{
                            if(getBank().open()){
                                MethodProvider.sleepUntil(() -> getBank().isOpen(), 3000);
                            }
                        }
                    }
                }
                break;
            case SMELTING:
                if(smeltArea.contains(getLocalPlayer())){
                    if (!getInventory().contains("Iron ore") ) {
                        currentTask = Task.BANK;
                    }else{
                        if (getDialogues().typeOption(4)) {
                            MethodProvider.sleepUntil(() -> !getInventory().contains("Iron ore") || getDialogues().canContinue(), 28000);
                        } else if ((getInventory().contains("Iron ore") && getLocalPlayer().getAnimation() != ANIMATION_SMELTING) || getDialogues().canContinue()) {
                            GameObject furnace = getGameObjects().closest("Furnace");
                            if (furnace != null && furnace.interact("Smelt")) {
                                MethodProvider.sleepUntil(() -> getDialogues().inDialogue(), 3000);
                            }
                        }
                    }
                }else{
                    if (getInventory().onlyContains("Steel bar", "Coal") || getInventory().isEmpty()) {
                        currentTask = Task.BANK;
                    }else{
                        if(getWalking().walk(smeltArea.getRandomTile())){
                            MethodProvider.sleepUntil(() -> getWalking().shouldWalk(), 6000);
                        }
                    }
                }
                break;
        }
        return Calculations.random(500, 600);
    }
}

 

 

[Chaos_Harmony 1]

Overall, the script looks good.

Whilst I'm certainly not a high level scripter - and indeed haven't released anything yet - if the amount of conditionals is annoying you, you could always switch to using a task based system by extending TaskScript as opposed to extending the AbstractScript. This makes the whole script that bit more modular and splits elements out into individual tasks - thus easier for testing and debugging.

[katastrophi 0]

4 hours ago, Chaos_Harmony said:

Overall, the script looks good.

Whilst I'm certainly not a high level scripter - and indeed haven't released anything yet - if the amount of conditionals is annoying you, you could always switch to using a task based system by extending TaskScript as opposed to extending the AbstractScript. This makes the whole script that bit more modular and splits elements out into individual tasks - thus easier for testing and debugging.

Thanks I will take a look at TaskScript. Looks nicer to organise a project for sure.

I don't think it will necessarily stop the side effect nature of the api though.

There was a few bugs with the code that was causing multiple clicks on the furnace and problems when leveling up. All sorted now

@ScriptManifest(author = "Katastrophi", name = "Edgeville Steel Smelting", version = 1.1, description = "Simple Smelter", category = Category.SMITHING)
public class main extends AbstractScript {
    Task currentTask = Task.BANK;


    private static final int ANIMATION_SMELTING = 899;

    private static final Area bankArea = new Area(3094, 3495, 3098, 3498);
    private static final Area smeltArea = new Area(3109, 3500, 3107, 3497);

    private enum Task {
        SMELTING, BANK
    }

    @Override
    public int onLoop() {
        switch (currentTask) {
            case BANK:
                if (getBank().isOpen()) {
                    if (getInventory().isFull()) {
                        if (getBank().close()) {
                            currentTask = Task.SMELTING;
                        }
                    } else if (getInventory().contains("Steel bar") && getBank().depositAllItems()) {
                            MethodProvider.sleepUntil(() -> getInventory().isEmpty(), 1500);
                    } else {
                        if (getInventory().contains("Iron ore") && getBank().withdrawAll("Coal")) {
                                MethodProvider.sleepUntil(() -> getInventory().contains("Coal"), 1500);
                        } else if (getBank().withdraw("Iron ore", 9)) {
                            MethodProvider.sleepUntil(() -> getInventory().contains("Iron ore"), 1500);
                        }
                    }
                } else {
                    if (getInventory().contains("Iron ore") && getInventory().contains("Coal ore")) {
                        currentTask = Task.SMELTING;
                    } else {
                        if (!bankArea.contains(getLocalPlayer()) && getWalking().walk(bankArea.getRandomTile())) {
                                MethodProvider.sleepUntil(() -> getWalking().shouldWalk(), 6000);
                        } else if (getBank().open()) {
                                MethodProvider.sleepUntil(() -> getBank().isOpen(), 3000);
                        }
                    }
                }
                break;
            case SMELTING:
                if (smeltArea.contains(getLocalPlayer())) {
                    if (!getInventory().contains("Iron ore")) {
                        currentTask = Task.BANK;
                    } else {
                        if (getLocalPlayer().getAnimation() != ANIMATION_SMELTING) {
                            if (getDialogues().inDialogue() && getDialogues().canContinue() && getDialogues().clickContinue()) {
                                    MethodProvider.sleepUntil(() -> !getDialogues().inDialogue() || getDialogues().canContinue(), 1500);
                            } else if (getDialogues().inDialogue() && getDialogues().typeOption(4)) {
                                MethodProvider.sleepUntil(() -> !getInventory().contains("Iron ore") || getDialogues().canContinue(), 28000);
                            } else if (getInventory().contains("Iron ore") || getDialogues().inDialogue()) {
                                GameObject furnace = getGameObjects().closest("Furnace");
                                if (furnace != null && furnace.interact("Smelt")) {
                                    MethodProvider.sleepUntil(() -> getDialogues().inDialogue(), 3000);
                                }
                            }
                        }
                    }
                } else {
                    if (getInventory().onlyContains("Steel bar", "Coal") || getInventory().isEmpty()) {
                        currentTask = Task.BANK;
                    } else {
                        if (getWalking().walk(smeltArea.getRandomTile())) {
                            MethodProvider.sleepUntil(() -> getWalking().shouldWalk(), 6000);
                        }
                    }
                }
                break;
        }
        return Calculations.random(400, 600);
    }
}