Jump to content

Improvements

Nym

By Nym
in Scripting Help

 Share

Recommended Posts

Nym Black Poster

Nym

Posted
Posted

Could anyone share some improvements I can make to this script?

  

public class Main extends Script {

    GUI gui = new GUI();
    private NPC monster;

    private long startTime;

    public enum State {
        splash,
        splashAlch
    }

    @Override
    public int onLoop() throws InterruptedException {

        if (Settings.started) {

            if (!Settings.monsterName.isEmpty() && Settings.monsterName != null) {
                monster = npcs.closest(Settings.monsterName);
            } else {
                log("Enter a real monster name.");
                stop();
            }
            if (!magic.canCast(Settings.spellToCast)) {
                log("Cant cast spell");
                stop();
            }

            switch (getState()) {

                case splash:

                    if (monster != null && monster.isVisible()) {
                        if (magic.castSpellOnEntity(Settings.spellToCast, monster)) {
                            new ConditionalSleep(3000) {
                                @Override
                                public boolean condition() throws InterruptedException {
                                    return myPlayer().isAnimating();
                                }
                            }.sleep();
                            new ConditionalSleep(3000) {
                                @Override
                                public boolean condition() throws InterruptedException {
                                    return !myPlayer().isAnimating();
                                }
                            }.sleep();
                        }
                    } else {
                        camera.toEntity(monster);
                    }

                    break;

                case splashAlch:

                    if (monster != null && monster.isVisible()) {
                        if (magic.castSpellOnEntity(Settings.spellToCast, monster)) {
                            sleep(random(150, 400));
                            if (magic.canCast(Spells.NormalSpells.HIGH_LEVEL_ALCHEMY)) {
                                if (magic.castSpell(Spells.NormalSpells.HIGH_LEVEL_ALCHEMY)) {
                                    new ConditionalSleep(2000) {
                                        @Override
                                        public boolean condition() throws InterruptedException {
                                            return tabs.getOpen().equals(Tab.INVENTORY);
                                        }
                                    }.sleep();
                                    sleep(random(50, 150));
                                    if (mouse.click(inventory.getMouseDestination(inventory.getSlot(Settings.alchName)), false)) {
                                        new ConditionalSleep(3000) {
                                            @Override
                                            public boolean condition() throws InterruptedException {
                                                return tabs.getOpen().equals(Tab.MAGIC);
                                            }
                                        }.sleep();
                                        sleep(random(150, 400));
                                    }
                                }
                            } else {
                                log("Can't cast High Alchemy");
                                stop();
                            }
                        }
                    }
                    break;
            }
        }
        return 120;
    }

    public State getState() {

        return ((skills.getDynamic(Skill.MAGIC) >= 55) && Settings.alch) ? State.splashAlch : State.splash;
    }


    @Override
    public void onStart() throws InterruptedException {
        log("Nimmogel's 1-99 Magic has started :D");
        gui.createGui();
        experienceTracker.start(Skill.MAGIC);

        Methods methods = new Methods(this);
        Settings.spellToCast = methods.spellToCastFinder();

        startTime = System.currentTimeMillis();
    }


    public void onMessage(Message message) throws InterruptedException {
        String text = message.getMessage().toLowerCase();

        if (text.contains("advanced a")) {
            Methods methods = new Methods(this);
            Settings.spellToCast = methods.spellToCastFinder();
        }

    }

    @Override
    public void onExit() throws InterruptedException {
        log("Bye :D");

        if (gui !=null) {
            gui.setVisible(false);
            gui.dispose();
        }
    }

    @Override
    public void onPaint(Graphics2D g) {

        long runTime = System.currentTimeMillis() - startTime;
        long tillNextLevel = experienceTracker.getTimeToLevel(Skill.MAGIC);

        long second = runTime / 1000L % 60L;
        long minute = runTime / 60000L % 60L;
        long hour = runTime / 3600000L % 24L;

        long secondX = tillNextLevel / 1000L % 60L;
        long minuteX = tillNextLevel / 60000L % 60L;
        long hourX = tillNextLevel / 3600000L % 24L;

        g.setColor(Color.WHITE);
        g.drawString("Time running: " + String.format("%d:%02d:%02d", hour, minute, second), 300, 285);
        g.drawString("EXP: " + experienceTracker.getGainedXP(Skill.MAGIC) + " (" + experienceTracker.getGainedXPPerHour(Skill.MAGIC) + "/hr)", 300, 300);
        g.drawString("Level: " + skills.getDynamic(Skill.MAGIC) + "(" + String.format("%02d:%02d:%02d", hourX, minuteX, secondX) + ")", 300, 315);
    }
}

 

Link to comment
Share on other sites
Auron Steel Poster

Auron

Posted
Posted

I can't really comment on the functionality of the script because it's hard to tell from just a brief look, if it works it works, good job pal. But from a coding point of view there can be a much better way of laying things out. In no particular order of things I picked up on:

1) All the settings checking at the beginning of the onLoop() are good, but only need to be checked once. The way you have it now, it will check these setting every loop. In the GUI, make the start button call a start method, which then checks all these things once, if everything is okay, then set Settings.started to true.

2) Conditional sleep can be separated into a method for neatness sake
 

Spoiler 

	public static void condSleep(int maximumMs, int deviationMs, BooleanSupplier condition) {
		new ConditionalSleep(maximumMs, deviationMs) {
			@Override
			public boolean condition() throws InterruptedException {
				return condition.getAsBoolean();
			}
		}.sleep();
	}

 

3) You have two states, imo it's simply pointless in creating an enum with a getState() function. And it's almost pointless having a getState function if it's a one liner. Not to mention I don't really understand your getState() function, wouldn't this always return Alching? Just do 

if(*getState code*) {
	alch();
} else {
	splash();
}



4) (java conventions) sound like a dick saying this, but for convention sake you should create getters in the Settings class rather than accessing variables directly. It's a good habit to get into.

5) That  GUI gui = new GUI(); at the top. I'm sure you can't even do that outside a method. Put it in the constructor of Main. That said, does this code compile?

Anyway, it's a great start dude, best of luck in the future.

Link to comment
Share on other sites
Guest

Guest

Posted
Posted
1 hour ago, Auron said:

I can't really comment on the functionality of the script because it's hard to tell from just a brief look, if it works it works, good job pal. But from a coding point of view there can be a much better way of laying things out. In no particular order of things I picked up on:

1) All the settings checking at the beginning of the onLoop() are good, but only need to be checked once. The way you have it now, it will check these setting every loop. In the GUI, make the start button call a start method, which then checks all these things once, if everything is okay, then set Settings.started to true.

2) Conditional sleep can be separated into a method for neatness sake
 

  Reveal hidden contents 


	public static void condSleep(int maximumMs, int deviationMs, BooleanSupplier condition) {
		new ConditionalSleep(maximumMs, deviationMs) {
			@Override
			public boolean condition() throws InterruptedException {
				return condition.getAsBoolean();
			}
		}.sleep();
	}

 

3) You have two states, imo it's simply pointless in creating an enum with a getState() function. And it's almost pointless having a getState function if it's a one liner. Not to mention I don't really understand your getState() function, wouldn't this always return Alching? Just do 


if(*getState code*) {
	alch();
} else {
	splash();
}



4) (java conventions) sound like a dick saying this, but for convention sake you should create getters in the Settings class rather than accessing variables directly. It's a good habit to get into.

5) That  GUI gui = new GUI(); at the top. I'm sure you can't even do that outside a method. Put it in the constructor of Main. That said, does this code compile?

Anyway, it's a great start dude, best of luck in the future.

Can you criticize my script after?

Link to comment
Share on other sites
Guest

Guest

Posted
Posted (edited)
1 minute ago, Auron said:

Eh why the hell not, I find a sick pleasure in doing so. Pm me

Once it's ready I will!

Edit: Will send you a different one for now... 

Edited by Noidlox
Link to comment
Share on other sites
Nym Black Poster

Nym

Posted
  • Author
Posted
7 hours ago, Auron said:

I can't really comment on the functionality of the script because it's hard to tell from just a brief look, if it works it works, good job pal. But from a coding point of view there can be a much better way of laying things out. In no particular order of things I picked up on:

1) All the settings checking at the beginning of the onLoop() are good, but only need to be checked once. The way you have it now, it will check these setting every loop. In the GUI, make the start button call a start method, which then checks all these things once, if everything is okay, then set Settings.started to true.

2) Conditional sleep can be separated into a method for neatness sake
 

  Hide contents 


	public static void condSleep(int maximumMs, int deviationMs, BooleanSupplier condition) {
		new ConditionalSleep(maximumMs, deviationMs) {
			@Override
			public boolean condition() throws InterruptedException {
				return condition.getAsBoolean();
			}
		}.sleep();
	}

 

3) You have two states, imo it's simply pointless in creating an enum with a getState() function. And it's almost pointless having a getState function if it's a one liner. Not to mention I don't really understand your getState() function, wouldn't this always return Alching? Just do 


if(*getState code*) {
	alch();
} else {
	splash();
}



4) (java conventions) sound like a dick saying this, but for convention sake you should create getters in the Settings class rather than accessing variables directly. It's a good habit to get into.

5) That  GUI gui = new GUI(); at the top. I'm sure you can't even do that outside a method. Put it in the constructor of Main. That said, does this code compile?

Anyway, it's a great start dude, best of luck in the future.

Thanks a lot, very helpful!

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

×
 Share
Go to topic listing

  • Recently Browsing   0 members

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