Welcome to TRiBot Forums

Register now to gain access to all of our features. Once registered and logged in, you will be able to contribute to this site by submitting your own content or replying to existing content. You'll be able to customize your profile, receive reputation points as a reward for submitting content, while also communicating with other members via your own private inbox, plus much more! This message will be removed once you have signed in.

Sign in to follow this  
Followers 0
Tsuyoshi

(Constructive) Critisicm please :)

9 posts in this topic

I've recently started to learn Java, and decided to make my own script. Most of the code is from Dibe's tutorial, and from some others which I can not remember, and I just adapted it to fit my needs. Any advice or criticism (about the script!) is gladly appreciated :P

http://pastebin.com/mxPeLin7

Share this post


Link to post
Share on other sites
private boolean loop() {
   
            if (Inventory.isFull() && shouldBank) {
                bank(START_TILE);
            } else if (Inventory.isFull() && !shouldBank) {
                dropOre();
            } else {
                mineOre();
            }
            return true;
        }

can be simiplifed to

 

private boolean loop() {
   
            if (Inventory.isFull()) {
				if(shouldBank))
                bank(START_TILE);
				else 
				dropOre();
				} else {
                mineOre();
            }
            return true;
        }

 

and don't use dibes tutorial. Use trilez scripting tutorial

don't have the time to go through everything, but there are definitely a lot of improvements you can do. And please format your code (indentations)

You did the right choice asking for criticism, though

1 person likes this

Share this post


Link to post
Share on other sites

Null checks, homie. Within your mine function, do something like this:

private void mineOre() {
	ores = Objects.findNearest(20, ORE_ID);
	if (ores.length > 0 && ores != null) {
		if (Player.getAnimation() != MINING_ANIM && !Player.isMoving()) {
			if (ores[0].isOnScreen()) {
				ores[0].click(); // add some click offsets here [e.g click("Mine", new Point(-2, -2), new Point(2, 2)) ]
				sleep(1800, 2900);
			} else {
				// good idea, except usually the ore is in the view, and this will cause hella camera shifts.
				Camera.turnToTile(ores[0].getPosition());
				Walking.walkTo(ores[0].getPosition());
			}
		}
	} else {
		// no ores found
	}
}

Good luck, and happy coding!

PS: Always experiment. Continue learning from sources & criticism. Code (especially yours) will never be "perfect". Constantly try and improve it.

Edited by Hersche
Goddamn code format

Share this post


Link to post
Share on other sites

Why does everyone tend to do this

bank(START_TILE);

and this

START_TILE = Player.getPosition();

This heavily limits the functionality of your script. This always require you to start at the mining location i suppose.

Share this post


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

Null checks, homie. Within your mine function, do something like this:


private void mineOre() {
	ores = Objects.findNearest(20, ORE_ID);
	if (ores.length > 0 && ores != null) {
		if (Player.getAnimation() != MINING_ANIM && !Player.isMoving()) {
			if (ores[0].isOnScreen()) {
				ores[0].click(); // add some click offsets here [e.g click("Mine", new Point(-2, -2), new Point(2, 2)) ]
				sleep(1800, 2900);
			} else {
				// good idea, except usually the ore is in the view, and this will cause hella camera shifts.
				Camera.turnToTile(ores[0].getPosition());
				Walking.walkTo(ores[0].getPosition());
			}
		}
	} else {
		// no ores found
	}
}

Good luck, and happy coding!

PS: Always experiment. Continue learning from sources & criticism. Code (especially yours) will never be "perfect". Constantly try and improve it.

You don't need to nullcheck finding methods (object.find, npc.find, etc) because the array will be empty if it can't find anything. (Read the docs).

Even if it did return null, you're doing it wrong. There is no point to null check it AFTER you length check it because that could result in a NPE.

if(x != null && x.length > 0)

 

4 people like this

Share this post


Link to post
Share on other sites
8 hours ago, erickho123 said:

You don't need to nullcheck finding methods (object.find, npc.find, etc) because the array will be empty if it can't find anything. (Read the docs).

Even if it did return null, you're doing it wrong. There is no point to null check it AFTER you length check it because that could result in a NPE.

if(x != null && x.length > 0)

 

Oo, I'm aware of the ordering - I was quite hasty whipping that up.

 

Also, the way I referenced the object is why I have the null check. If I attempt to use ores[0] while ores is null, that's an NPE (unless I'm completely stupid).

I reference them by index because sometimes I do not always use the first / closest found; but that's my style. 

 

Regardless, thanks for the clarification.

Share this post


Link to post
Share on other sites
16 minutes ago, Hersche said:

Oo, I'm aware of the ordering - I was quite hasty whipping that up.

 

Also, the way I referenced the object is why I have the null check. If I attempt to use ores[0] while ores is null, that's an NPE (unless I'm completely stupid).

I reference them by index because sometimes I do not always use the first / closest found; but that's my style. 

 

Regardless, thanks for the clarification.

If you try to access a null object, you'll get a NullPointerException, that's true.

But the API specifically returns an empty array, not null, when you call Objects.find (and the same thing happens with Npcs.find and probably a few other places), so there's no reason to check if the array is null.

1 person likes this

Share this post


Link to post
Share on other sites
2 hours ago, deus-x said:

If you try to access a null object, you'll get a NullPointerException, that's true.

But the API specifically returns an empty array, not null, when you call Objects.find (and the same thing happens with Npcs.find and probably a few other places), so there's no reason to check if the array is null.

I see! Thanks for the clarification. Old habits, I suppose. ;>

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now
Sign in to follow this  
Followers 0

  • Recently Browsing   0 members

    No registered users viewing this page.