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.

RedHawkLuffy

RedHawkLuffy Scripter Application

7 posts in this topic

1) Snipplets: [SOURCE] (Link to thread)

None

2) Tutorials: [sOURCE] (Link to thread)

None worth mentioning.

3) Randoms/updates submitted: [sOURCE] (Link to thread)

I remember I submitted a random to Trilez a couple years ago but it was through pm and don't have code either.

4) Scripts available to the public: [sOURCE] (Link to thread)

RedSandCrabs

RedPlanker

RedCooker

Link to source for all scripts: https://github.com/razahamza/tribot

5) Short biography / Coding Experience: [1-2 short paragraphs]

I started coding about 6 years ago off and on. I started with nexus (IMPSoft) and was a script developer there until it shut down. I moved on to powerbot and was a script writer there but went inactive and then when I came back I moved onto tribot and but quit shortly after as well because my accounts got banned on runescape but I recently came back and decided to help the community a bit. I think I have written a pretty successful sand crab script, have personally gotten over 50+ hours on it. I have been coding for 6+ years as I've stated which obviously influenced by current undergraduate degree in engineering. I am in third year in computer engineering and planning to get an internship in software development next year. I know I have a lot to improve and will continue working hard to improve myself.

6) Reasons why you feel you deserve Scripter: [1-3 short paragraphs]

I was a developer on IMPSoft and powerbot as stated and I can write scripts that work well. I may not be an expert in Java but my scripts do function, I feel, as good as some premium scripts such as my sand crab script. I have many personal scripts I wrote but I did not release them yet because I have to update them before releasing them since they were customized to me. But I plan on releasing them all to the community when I polish them up.

7) What you plan to provide the community with: [1-3 short Paragraphs]

Seems like I keep answering the question below be before I get to it..

Many of my private scripts I plan on polishing and releasing in the future as well as providing useful help as I am beginning to really understand the tribot api.

8) Do you agree to continue to not only update, but provide more free, open sourced scripts to the community? [YES/NO]

Yes

Also I already know someone will point out why I used a textfile for saving and loading profiles which I know isn't very good but it works I just didn't know how else to handle it at the time. I've just recently saw properties configuration in the java api and plan on switching to it in the next sandcrabs update.

 

--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

 

@xCode The getItem method was something I used a very long time ago when I wasn't very knowledgeable about the API, but if you look through the code you will see I never actually use the method. I do use Inventory.find(). I should have probably removed it but I seem to have forgotten about it.

You are absolutely right about the Player p, that was a mistake I forgot there was a loop and there is no need to recreate the object and I will fix that but I do hope you don't assume that was caused by my ineptitude of Java, rather it was a simple mistake that I missed when reviewing my code.

This method is for stray crabs that only occur once every say 30-40 minutes in the script. It does not need to run this method every single time you need to fight a crab as the crabs are aggressive.  I agree it looks really iffy and I'm not personally fond with this method either. I honestly did not like the way I was doing it when I was writing it but I couldn't really think of another way to do it. There is a 0.5 second delay between the crabs spawn and its interaction with a player if I don't use a timer how will I know whether it is a stray crab running around or its someone else's crab that just spawned? I don't want it to keep attacking someone else's crab, when I ran it without the timer every time it was not in combat and someone else's crab spawned it would attack it. So I just coded this as a failsafe to wait and see whether it is an attackable crab (stray) or one that's already taken. If there is a better way, which there probably is please let me know I will switch to it. I do agree a fixed timer is not ideal I should have randomized it.

@Final Caliburpoint taken to make it simple, though my method and variable names are pretty self explanatory for the most part, really shouldn't be that difficult to read the code most of the if statements result from checks (array length checks, null checks, is on screen checks, interface up checks) which are all necessary and required to be nested but I do understand it could be made cleaner

@c#2Bot

1) You're right, don't know how I missed that

2) Again I don't know how I missed that, I rushed the cooker as I only needed it for like 30 cooking levels for a quest I should have looked at it more carefully. It would be better if you looked at my sand crabs script as I've spent much more time on that

3) Oops, I just forgot to check 

4) If they have the same id's and distance it will simply return the first one in the array, not really sure how this is a problem.

5) Yea, I will work on that most of it is just failsafes but I can probably make it cleaner

6) Banking.openBank() returns true if the bank is already open. (tested it) In practice though, I see why you would think to have a failsafe first since you don't know the actual code of openBank() and certainly I would have included a check but you have to give Trilez some credit I'm sure he coded his api with checks.

7) True.. don't know how I didn't see that

8) I just prefer'd it for future changes, having an extra task doesn't necessarily reduce performance. I mean its negligible really.

9) Already stated earlier, I don't use that method in the code, it's there by mistake but never used. Though I honestly don't seem to see it as a big concern that I left it there but everyone else seems to think otherwise o-o

@Final CaliburYou're right, I fixed it let me know if this is suitable.

@Final CaliburFixed some more stuff, did not know tribot didn't return null elements in the array so I removed them and consequently the code looks much cleaner now. Hopefully I got them all. Fixed the magic numbers you mentioned, there are probably some more in my code but I will look at it after I get back and will fix it.

@Encoded I updated the code with your suggestions and I understand your point, I should not have created a Method class, I don't really know what I was thinking there I just didn't want to reuse the walk method over and over again in multiple classes but I should have named the class relating to the functions that will be in the class it such as Walking

@laniax I generate them during the walk method of my sandcrab script.

https://github.com/razahamza/tribot/blob/master/autosandcrabs/tasks/Walk.java#L44

Trilez's guide says it should be generated after clicking something which you have to wait a variable time and since this is a afk sand crab script after you click your tile you have to wait until the sandcrabs lose aggro but since you don't want to make it log out automatically, I set the waiting time to 60 seconds.

Edited by RedHawkLuffy

Share this post


Link to post
Share on other sites

No.

( scripts.autosandcrabs.Methods.java)

public static RSItem getItem(String item) {
		RSItem[] items = Inventory.getAll();
		for (RSItem i : items) {
			if (i != null) {
				RSItemDefinition definition = i.getDefinition();
				if (definition != null) {
					String name = definition.getName();
					if (name != null && name.toLowerCase().equalsIgnoreCase(item.toLowerCase())) {
						return i;
					}
				}
			}
		}
		return null;
}

I think the above method shows your knowledge of the API. It's the first class I opened. Try Inventory#find(name). Check the length and Bingo!

The next:

(scripts.autosandcrabs.tasks.Attack.java)

public static boolean isAttackRequired() {
		RSNPC[] npcs = NPCs.findNearest("Sand Crab");
		for (int i = 0; i < npcs.length; i++) {
			RSNPC npc = npcs[i];
			if (npc != null) {
				Timer combatTimer = new Timer(2000);
				RSPlayer p = Player.getRSPlayer();
				if (p == null)
					break;
				while (!p.isInCombat() && !npc.isInCombat() && npc.getInteractingCharacter() == null
						&& npc.getPosition().distanceTo(Player.getPosition()) <= 5 && combatTimer.isRunning()) {
					General.sleep(100);
				}
				if (!p.isInCombat() && !npc.isInCombat() && npc.getInteractingCharacter() == null
						&& npc.getPosition().distanceTo(Player.getPosition()) <= 5) {
					return true;
				}
			}
		}
		return false;
}

Why are you setting RSPlayer p for every loop cycle?

Why are you sleeping for 2 seconds max per Sand Crab when you ' can't fight'. Your entire structure seems very fuzzy. There are much easier ways to accomplish what you have shown here.

In my opinion you just lack experience atm. I'll try to look into more of your code later on.

Share this post


Link to post
Share on other sites

Not bad, but far too many Egyptian-like pyramids of code throughout your scripts. Here is an example.

Convoluted code not only makes your script hard to read and analyze (like we're trying to do now), but also hampers future debugging and development.

It's a no from me for now, I'd like to see you clean up your code a bit, think of simpler and cleaner ways to accomplish your tasks, and reapply in the near future.

@RedHawkLuffy

Responding to your comments, it isn't the variable / method naming that makes it hard to read, it's the method length. There are multiple ways to improve this, whether it be through abstraction, or even taking advantage of short-circuit evaluation in your if statements.

A big part of the Scripter rank, in my opinion, is driving the community forward with open source scripts. It is for this reason that clean, elegant, and readable code is the most important criteria that I think about when looking at Scripter applications. I hope this clarifies my perspective.

@RedHawkLuffy

That's a step in the right direction. Also, consider abstracting away any frequently-used conditions across all of your Scripts, so you don't have to keep nesting / rewriting them whenever you make a Timing#waitCondition call. Obviously there will be cases where there are script specific wait-conditions, but in the vast majority of cases you will find yourself re-using the same type of Condition.

Also, I've noticed that you're null checking on many occasions that you don't actually need to. Here and here for example. Once you length check the array that is returned by the API, you don't need to null check the elements. TRiBot will not return arrays with null elements.

Game#isUptext will make your life easier in this method.

Magic numbers here and here.

Also I despise your code for painting, it's brute-forced in every script and could benefit from you creating a nice framework for it.

This was analysis of just one class, but fix up these issues throughout your code and make it generally cleaner, and you'll be well on your way (in my opinion) to achieving the rank.

 

 

Edited by Final Calibur

Share this post


Link to post
Share on other sites

Hello,

i will point out a few things i have noticed in your application, however i will leave the voting to the fellow senior ranks as i am  a fresh scripter.

 

- You have cached the interface, however did not null check it before your operation here:

https://github.com/razahamza/tribot/blob/master/autocooker/tasks/Cook.java#L65

- You did not cache nor null check the gameuptext as it can return null here:

https://github.com/razahamza/tribot/blob/master/autocooker/tasks/Cook.java#L87

- Sometimes you check if clicking is successful  sometimes not as shown here:

https://github.com/razahamza/tribot/blob/master/autocooker/tasks/Cook.java#L60

- What if two objects with similar ids are in the same distance?

https://github.com/razahamza/tribot/blob/master/autocooker/tasks/Cook.java#L37

- It is very difficult to trace whats happening in this method because its huge:

https://github.com/razahamza/tribot/blob/master/autocooker/tasks/Cook.java#L36

- Why are you only opening the bank without checking if the bank screen is open?

https://github.com/razahamza/tribot/blob/master/autocooker/tasks/Bank.java#L30

- This check is what triggers this task in your validate method, why are you repeating the check here?

https://github.com/razahamza/tribot/blob/master/autocooker/tasks/Bank.java#L35

- Do you really need a task class to execute this method?

https://github.com/razahamza/tribot/blob/master/autoplanker/tasks/Walk.java#L27

- Inventory.find() method in api exists, this isnt needed:

https://github.com/razahamza/tribot/blob/master/autosandcrabs/Methods.java#L16

 

All in all, small mistakes but are repeated a lot on your scripts, i am sure you can fix them, familiarize yourself with the api, clean up your code, i dont think your application is bad, but work on those points and i wish you all the best, good luck!

Edited by c#2Bot

Share this post


Link to post
Share on other sites

I haven't read any of the replies above so sorry if I point out something that was already mentioned.

RedCooker

https://github.com/razahamza/tribot/blob/master/autocooker/AutoCooker.java#L43
- You are not creating your GUI object on the AWT event dispatching thread. 

In the future I would recommend having all those calculations in the onPaint method to be on their own thread since the paint thread refreshes at a much higher rate than is needed.

Use Game#getItemSelectionState() rather than checking uptext to see if an item is selected.
I don't know if it's mentioned anywhere on the forums, but you should use Clicking#click over other forms of clicking when applicable as it has additional antiban measures.

https://github.com/razahamza/tribot/blob/master/autocooker/tasks/Cook.java#L110-L112
- Could be simplified to return Player.getAnimation() != -1;

RedPlanker

Not a fan of Methods.java; too generic. 
Most of your execute methods are way too long.
Same GUI problem as your cooker.


I'm going to be honest, I'm not impressed based on your experience, but I do think you have enough knowledge to hold the scripter rank.

To stay consistent with the other applicants that I deem worthy enough for scripter, you will be given a short code challenge to earn my official vote.
You will hopefully be familiar with another popular variation of this challenge and will have no issue at all.

SharkBrew
Based on the user's current HP ratio:
Print "Shark" if the ratio is a multiple of 3.
Print "Brew" if the ratio is a multiple of 5.
Print "SharkBrew" if the ratio is a multiple of both 3 and 5.
Otherwise print the ratio.

 

Since I can't edit my posts on scripter apps, I'll have to double post.

My vote is yes.

Share this post


Link to post
Share on other sites

Where do you generate your ABC2 trackers? i cannot find them anywhere.

I would've liked to see some more age on your scripts, most of them aren't even a month old. I'd would've also liked to see some more advanced scripts, you won't impress me with a planker or cooker.

I'll await your response before voting.

1 person likes this

Share this post


Link to post
Share on other sites

Who were you at Nexus? I don't remember you


autocooker is more of a hello world, there really isn't enough substance to evaluate here other than your understanding of how to create a script.


autoplanker demonstrates something more of the base level in terms of length of what we would be looking for

			int bankMode = vars.getBankMode();
			if (bankMode == 2) {

An enum of type BankMode would be more readable

String law = "Law rune";

Constants belong in the class declaration as a static field

private final static String LAW_RUNE = "Law rune"


AutoSandCrabs looks like a useful addition to the site, thank you.

You do not however implement ABC correctly as mentioned above, the structure should follow 

  1. Sleep reaction time
  2. Set start time
  3. Perform action, sleep until complete or breakout
  4. Set end time
  5. Generate trackers for (end time - start time)

You are using the reaction generated from the action you previously executed to create data for the ABC class to return a time you should sleep before the next action, this applies to any action which would cause your player to idle for a variable amount of time (attacking crabs, cooking food, etc.). They should be implemented in all of those scenarios to include any others where your player may move onto something else, (i.e. new resource, walking to a new location, etc.)

		RSNPC[] npcs = NPCs.findNearest("Sand Crab");
		for (RSNPC npc : npcs) {
			if (!p.isInCombat() && npc.getInteractingCharacter() == null
					&& npc.getPosition().distanceTo(Player.getPosition()) <= 5) {
				if (DynamicClicking.clickRSNPC(npc, "Attack Sand Crab")) {
					Timing.waitCondition(new Condition() {
						@Override
						public boolean active() {
							RSPlayer p = Player.getRSPlayer();
							if (p == null)
								return false;
							return p.isInCombat();
						}
					}, General.random(4000, 6000));
					break;
				}
			}
		}

I would move away from an implementation like this and instead call a method on the instance of that npc to perform the action, you will see why this is important when you use ABC to select the next target based on the array of targets you send to the method ABC#selectNextTarget()


Overall a decent showing, I would encourage you to re-apply with Scripts that have a little more substance/complexity where we can see your skill level. In addition, you should apply with a script that implements ABC correctly as stated in the guidelines.

Good luck

Share this post


Link to post
Share on other sites
Guest
This topic is now closed to further replies.

  • Recently Browsing   0 members

    No registered users viewing this page.