Jump to content
Search In
  • More options...
Find results that contain...
Find results in...
Buy OSRS Gold

Sell OSRS Gold
Dentist

Testing1 2nd Application

Recommended Posts

So i am reapplying ^_^ 

1) Snippets: [Source] (Link to thread)
none.

 

2) Tutorials: [Source] (Link to thread)
Scripting guide to help to start scripting for tribot

Complete Guide For Breaking System

Complete Guide For Fixing LG Problems

3) Randoms/updates submitted: [Source] (Link to thread)
none.

 

4) Scripts available to the public: [Source] (Link to thread)
This is my old Github repo if any scripter wan know the changes https://github.com/GeniuSKilleR95/Application

Source Code          GeniuS FarminG

Source Code          GeniuS Feather Buyer

Source Code          GeniuS Mixer 

GeniuS P2P Quester   this is my biggest script it is still Beta so still no source code for it yet 

 

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

As i said in my old application i learned alot from the last time i applied it been more than a month i kept digging into java more and more and more still there is tons i dont know yet and willing to keep digging into it 

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

i have been almost the most active person on tribot for like 2 months helping people  out on discord or on forum i think i deserve it as i love helping people and willing to keep helping and providing helpful updates and scripts for them ^_^ 

 

7) What you plan to provide the community with: [1-3 short Paragraphs]
For tribot i am willing on adding largest script in rune scape botting  community which will include almost all rs quests that i think will bring more ppl to tribot 

for the ppl tribot have now i am willing to provide them will all help i can give as i have tons of free time and i will ofc provide them will good quality free scripts as well 

 

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

 as i said before in the 1st apply ofc i am going to keep providing more open sources and free scripts 

 

 

 

@FALSkills  I followed the standard java naming conventions from the link u provided in the 1st application 

I made an full general  api folder that is used for almost all my scripts as u suggested 

Removed all the magic numbers in all of my scripts as u suggested 

About the spellcheck  i did my best but i think it isnt  100% learning second lang is hard but isnt impossible and i think i am doing my best learning more 

 

@Netami  I think my time frame is good now ^_^ !? 

java naming conventions i followed them and is following them  in all next scripts i made 

Null checking i noted it fixed  and i did used Laniax's  but ended up not using it but i do a null check or use isInterfaceSubstantiated which auto check null thanks for recommendation 

about the OOP knowledge i am still learning about it it helped alot to be honest 

 

@wastedbro

Added a simple GUI for the GeniusMixer Script 

Followed the Naming conventions 

that ctrl+alt+L  fucking helped me alot thanks for sharing the info 

I did improved my English a bit and still improving it ^_^ 

 

That been said waiting for the comments ^_^ 

Share this post


Link to post
Share on other sites
Posted (edited)

Here are some things that you might want to improve:

  • some lines are too long and there are simple ways to avoid writing them
  • there are too many unnecessary blank lines that make the code hard to read
  • the usage of static variables is discouraged
  • some "validate" methods include too many checks, many of which are probably redundant; while this doesn't take a significant toll on efficiency, it does make the code harder to understand. It should be very clear when tasks should execute
  • too much code stuffed in the "execute" method; it's hard to follow and it's unclear what it's supposed to do. Instead of using random blank lines to space out the code, split the whole cluster into small, concise methods that have a very clear purpose and are named appropriately

 

Overall the code is of poor quality. Instead of releasing more and more scripts, I would suggest focusing on their quality. The code needs to be clean because otherwise you won't be able to create and maintain large scale, complex scripts.

Get more practice; refactor your code.

 

Edited by Einstein

Share this post


Link to post
Share on other sites

So I'm looking at your farming script.

Why is your node class an abstract class and not an interface? do you understand the difference?

I think you abuse the Node framework a bit, break down your tasks a bit smaller. Why do you have so many options/methods packed into one execute()?

I'm not positive, but it seems like you don't have much logic for when things fail. Generally, you want to give tribot room to fail & check to see if it failed WITHOUT looping through all your checks again. This can be a tedious process but can really sharpen the humanness of your scripts.

Use a setter method here instead of setting the variable.

image.png.06f2b77bd3bed28e74160eb82382a956.png

Other things just seem like poor logic choices. A whole node to failing ring of dueling? I'm not sure what to think about this. I'm all about doing what you need to do to make a script work, but it seems like in this stage of the script it could simply be planned better.


For a farming script I would consider implementing a dynamic node system. (A node system where you can add and remove nodes)

 

Share this post


Link to post
Share on other sites

You're improving, definitely. Very glad to see it. But as Einstein said, the quality is a bit lacking. I would really prefer that you make one really good script instead of 15 mediocre ones.

 

1. As Todd said, be sure to include more fail-handling:

For example, your scripts will sleep for no reason if something fails:

Keyboard.typeString("1");
BooleanSupplier supplier = () -> plantSpace.length > 1 || Player.getAnimation() == -1;
GeniuSWait.waitTill(supplier);
General.sleep(General.randomSD(600, 1800, 900, 65)); // If the player doesn't do the animation, this sleep still happens. Bad.

 

You need to instead utilize return values (note: you can delete my comments in this code, of course):

Keyboard.typeString("1");
// "supplier" is not a good variable name. Let's make it more descriptive
BooleanSupplier isPlayerAnimatingFunc = () -> Player.getAnimation() == -1; // plantSpace will never change, by the way, so checking it's length is pointless
if (Timing.waitCondition(isPlayerAnimatingFunc)) {
    General.sleep(General.randomSD(600, 1800, 900, 65)); // Only sleeps if the player ACTUALLY does something
}

 

You basically make this same mistake in all of your scripts, everywhere. 

 

2. Need to clean up some sloppiness:

This makes me feel like you're a bit too new to programming in general:

public static boolean hasGold() {
    if (Inventory.getCount(995) >= 5000) {
        return true;
    }
    return false;
}

You can reduce this to: public static boolean hasGold() { return Inventory.getCount(995) >= 5000; }

 

And this....

RSObject[] door = Objects.find(15, "Door hotspot");
GeniuSWait.waitTill((() -> door.length > 0));

That array will never change, so waiting on it to have a length > 0 is very pointless

 

Also, ensure that you're always trying to find better ways to structure your code than "Utility.java" or "Vars.java". Your scripts are very small right now, but if you ever make a large one, keep in mind that these classes will start to get bloated and confusing to follow.

 

3. Need ABC2 Implementation

Sorry, I think I may have missed this the first time. But, frankly, I don't see any ABC in this code. In some of your code, I see that you call on your library for ABC stuff, but you never included the source code to that. I understand that a good number of your scripts don't have features that need to use a lot of ABC-type implementation, but that just means you need to make a script that does.

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

 

PS: You should really include your whole source code. I can't see what you've done with much of the code, because it's all tucked away in "GeniusBanking" or "GeniusWait" or something.

 

Verdict: No

But, if you show your ABC2 implementation, and it includes everything and is done correctly, I'll change my vote to a "Maybe".

 

Share this post


Link to post
Share on other sites

Todd asked me to do a review as well, so here are my thoughts:

I read through most of the code on your github, and overall I think it's good! I do have a bunch of small comments that I'll add at the bottom, but really most of them aren't big enough things to really worry me. There are a few bigger things I'd like to address though:

  1. Your full source isn't on your Github, which to me is a problem. Not only does it provide code I can't read (which _could_ be doing something wrong) but it means nobody can download that script and run it. Scripter Applications are commonly recommended as a place to go for source code to read and learn from. Without providing your full source it takes away from the opportunity for someone to learn from your code. Your API especially might have some excellent utilities that new scripters could use, but without the source they can't.
  2. I believe Todd spoke to this, but I'll add it on as well. The code in your nodes is at times unorganized. One example in particular (in your Mixing script I believe, it's in the below notes) is that you deposit items in the bank in two nodes, under different conditions. Just make a node for depositing, that way you can easily organize when the deposits should and shouldn't happen. Having (in this case) the deposit code in different places will most likely produce major headache if you're trying to debug a rogue deposit.
  3. A lot of nested if statements, and not many else statements. Something like this:
    if (Game.getItemSelectionState() == 0) {
        if (secondMaterial[0].click("Use")) {
            General.sleep(500, 900);
        }
    }

    Can be shortened into:

    if (Game.getItemSelectionState() == 0 && secondMaterial[0].click("Use")) {
        General.sleep(500, 900);
    }

    This doesn't change performance, but it does reduce a code pyramid of if statements. On the flip side something like this is problematic (it's also in the notes):

    if (Banking.isBankScreenOpen()) {
        if (Banking.close()) {
            GeniuSWait.waitTill((() -> !Banking.isBankScreenOpen()));
            General.sleep(General.randomSD(500, 1000, 800, 65));
        }
    
        GeniuSTabing.checkTab(GameTab.TABS.INVENTORY);
    
        RSItem[] houseTeleport = Inventory.find(Constants.TELEPORT_TO_HOUSE);
        RSObject[] portal = Objects.find(10, Filters.Objects.nameEquals("Portal").and(Filters.Objects.actionsContains("Lock")));
    
    
        if (houseTeleport.length > 0 && portal.length == 0) {
            if (houseTeleport[0].click("Break")) {
                GeniuSWait.waitTill((() -> !Constants.CASTLE_WAR.contains(Player.getRSPlayer())));
            }

    So, here's the logic of this code:

      - If the bank is open, continue
          - If the bank is closed, then sleep
          - If we have house teleports and we're not in the house
              - If we successfully click the house teleport
              - ...
    The problem here is that what if the bank doesn't close? Even if the close fails, we still go all the way to the click code. Using an else statement could reduce this by jumping out of the code if it fails. While I don't like this nested structure at all, even this would help (alternatively put all the code below the else inside the Banking.close() if):

    if (Banking.isBankScreenOpen()) {
        if (Banking.close()) {
            GeniuSWait.waitTill((() -> !Banking.isBankScreenOpen()));
            General.sleep(General.randomSD(500, 1000, 800, 65));
        } else {
            return;
        }
    
        GeniuSTabing.checkTab(GameTab.TABS.INVENTORY);
    
        RSItem[] houseTeleport = Inventory.find(Constants.TELEPORT_TO_HOUSE);
        RSObject[] portal = Objects.find(10, Filters.Objects.nameEquals("Portal").and(Filters.Objects.actionsContains("Lock")));
    
    
        if (houseTeleport.length > 0 && portal.length == 0) {
            if (houseTeleport[0].click("Break")) {
                GeniuSWait.waitTill((() -> !Constants.CASTLE_WAR.contains(Player.getRSPlayer())));
            }

    Boom, now if the close fails we exit the method. Which is great, because we assumed it was closed everywhere else anyways. To be honest, not writing these nested ifs is something that comes with practice and experience (at least it did for me). Even though it might be best practice to not write them, as long as you use else conditions appropriately and nest properly it's okay.

  4. Some of your validate conditions are too heavy (albeit most are genuinely fine). Validate conditions are going to get ran (due to the structure of the node framework) a fair bit. At worst we could call one around every 20 - 40 milliseconds. This means the validate conditions need to be short, quick evaluation methods to ensure you don't slow down your script and use a bunch of CPU on a function that's getting ran a ton. As an example (from TeleportingToHouse.java):

    @Override
    public boolean validate() {
    
        RSObject[] door = Objects.find(15, "Door hotspot");
    
        return Utility.plantCount() > 0 && Utility.teleportToHouseCount() > 0 && Utility.wateringCanCount() > 0
                    && door.length == 0;
    }

    In this validate method you check your inventory three times and checking all objects in a 15 tile radius to see if they have a name equal to "Door hotspot". Sometimes these are definitely the best way don't get me wrong, but whenever possible use quicker methods to try and reduce CPU usage. A quick way to make this a bit better is:

    @Override
    public boolean validate() {
        if (Utility.plantCount() == 0 || Utility.teleportToHouseCount() == 0 || Utility.wateringCanCount()  == 0) {
            return false;
        } else {
            RSObject[] door = Objects.find(15, "Door hotspot");
            return door.length == 0;
        }
    }

    Now we only run the object check if the inventory checks pass. Not a huge difference, but we're not searching for objects every time.

Overall, I'm happy with it! I'd vote yes now, but you don't have a script with ABC2 which is a requirement. Once you have a script with ABC2 I'll be happy to vote yes. There are things I want you to work on (and things you will need to fix if you ever wanted to go for Premium) but for Scripter none of them are a deal breaker for me.

As a note if there's anything here you want help with, or you don't understand feel free to let me know! If I'm available I'll happily help!

Detailed notes compiled from reading all code on your github.

Spoiler

Farmer

- Farming.java
  - run function declared inside the passArguments function, I'm not sure if that'll compile.
- BankingItems.java
  - Line 60: You're checking if your not in the bank and if the bank is not open
  - Line 63: Checking if the bank is open
  - Line 96: Else, open the bank
  - You can cut the if the bank is not open check from line 60, as you don't use it. Switch that if statement to just checking if you're in the bank and if you're not in the bank walk there (and if you're not in the bank I'm fairly certain the bank can't also be open). If you're in the bank, then we care if the bank is open.
  - You don't have the source up for the walkAndOpenBank method but if I had to guess I'd say it's doing some very similar checks, which (albeit extremely minor) is performance being wasted.

  - Line 68: `Inventory.getAll().length == 0 || Inventory.getAll().length == 1` can be replaced with Inventory.getAll().length <= 0 or Inventory.getAll() < 1 or just Inventory.getAll().length == 0 as that call cannot return an array of length -1.

  - Line 85: Unless you're updating a constant (at which point it isn't constant :sweat_smile:) that call is false throughout the entire script or true throughout the entire script.

  - Line 96: Conditional sleeps are designed exactly for this :slight_smile: (and static sleeps are generally not recommended)

- BuildingPlant.java
  - Line 41 and 57: There's no else clauses here. Which isn't inherently bad, but shouldn't we do something if we have no plants and/or no plant spaces.

- FailSafeNoRingOfD.java
  - Line 43: Hmmm... I think we're missing a condition here, cause that's going to be false all the time. If you just want to sleep (but we shouldn't, as we have conditional sleeps) use a static sleep at least. That way you're not burning CPU cycles to check if an infinitely false condition is true.

- TeleportingToHouse.java
  - Line 47: What if the bank didn't close? We can't click teleport with the bank still open. Perhaps we move this outside the bankScreenOpen if, as we never want to try and teleport if the bank screen is open.
  - Line 65: We should null check that interface
  - Line 67: The length is never going to change 😕
  - Line 68: Ah, but we just slept in the conditional

FeatherBuyer

- FeatherBuyer.java
  - Line 62: Makes zero noticable difference, but an else if here would be better than two ifs. Mainly because you don't want to do a second evaluation on the if statement when you know both conditions cannot be true (or at the very least shouldn't be).
    - That's just best practice though, there's no noticable performance difference.

- Buying.java
  - Line 18: You need to cache Interfaces.get(300, 16, 9). Otherwise (technically) it can null out between those checks.
  - Line 31: Sleeping after a conditional sleep 😕
  - Line 34: So if the close fails, the only way to press close again would be to buy another 50 feathers? That's the big concern with nesting if's, if something fails inside, you'll need to pass all the prior checks in order to get back to your failure.

- OpeningFeathers.java
  - Line 32: At this point, we know we have Feather packs in our inventory and we want to open them. Therefore, regardless of whether or not the packs length is greater than 0 (albeit it should be given the validate()) we want to close the shop if possible. This if statement should be outside of the other if statement. That way we close the shop regardless.
   - Line 41: Sleeping after a conditional sleep

- OpeningFeathersAll.java
  - Line 41: I'd add in some kind of sleep here. ABC2 doesn't really work due to it not being set up for fixed waits, perhaps a conditional or at least a randomSD sleep.

- Trading.java
  - Line 15: I don't think this condition's right. The logic reads (to me): we want to trade Gerrant if we have no feather packs and the shop is open?
  - Fun fact: PathFinding.canReach is a relatively CPU expensive call. So you want to avoid it (essentially) as much as possible. In this situation what I would do (and this is not the only solution) is have an RSArea for the shop where you know Gerrant is reachable. If you're not in it, walk there. Otherwise we know he's reachable. That way you're able to check but in a quicker and lighter fashion :slight_smile:

- WalkToShop.java
  - Line 16: Ah, you should use Player.getPosition instead of getRSPlayer, lighter calls and getRSPlayer can return null

GeniusMixer

- GeniuSMixer.java
  - Line 46: You should dispose of the gui object with gui.dispose() as it helps clear up some resources on the system as the JFrame gets destroyed and garbage collected.

- Combine.java
  - Line 54: This is cached but not used. If you wanted to make sure the interface was up you should be checking that the cached interface is substantiated

- OpenBank.java
  - I'm not a big fan of this node. I don't like the if condition as it checks the same condition twice but in different ways. I'd move the depositing into a different node, as it doesn't really fit here.

- Withdraw.java
  - Line 23: Not that what you did is bad, but you can do != instead of !(==)
  - Line 49: Why do we have another deposit? Now we have deposits here and in OpenBank.java. If we move the deposits into their own node we can avoid these issues.

Vars

- Vars.java
  As a rule static vars are bad, statics should only be used for constants and methods that do not modify anything. However, in this context it's not going to hurt anything.

 

  • Thanks 1

Share this post


Link to post
Share on other sites

I'm approving this application. I think you have a ton to work on, but you seem to show signs of improvement. However, I think you're very far off from a premium script. Also I talked to wasted & fluffee and we all agree ABC2 is not required to get a scripter rank. However you should learn it.

here are a few recommendations:

  • Keep all your code open source, and don't work on anything for awhile that's closed source. You keep making quest scripts that literally don't work for anybody but yourself. This is a huge problem. Make sure to test scripts on multiple accounts before you release them
  • Plan your scripts more. Don't code as you play.

Don't fall back into your old habits.

  • Like 2

Share this post


Link to post
Share on other sites

  • Our picks

    • Hi everyone,

      I'd like to thank everyone for their patience in this transition period. Since last week, we've worked out the remaining bugs with this integration.

      Some users have still been having issues with connecting their forums account to their Auth0 account. To resolve this, we've imported all forums accounts into Auth0.

      Unfortunately, the accounts which were imported today were using an unsupported password hashing algorithm. Hence, random passwords were set during the import.

      What does this mean for me?

      If you've previously linked your forums account to your Auth0 account, you don't have to do anything. Nothing changes for you.


      If you haven't logged in via our new login yet,

      Try logging in with your forums email address and the last password you used


      If you are unable to login, please use the "Forgot password" tool on the login page:
      Follow the instructions to reset your password
       
      • 1 reply
    • Hello everyone,

      Last week we tried to roll out Auth0 Login, but we lost that battle. Now it's time to win the war!

      Important changes

      When logging into the client, you'll now have to enter your Auth0 account credentials instead of your forums credentials

      Note: 2FA is still handled through your forums account (for the time being)



      Changes for existing users

      You'll have to link your Auth0 account to your forums account here: https://tribot.org/forums/settings/login/?service=11


      Auth0 accounts have been created for most existing users. Please use your forums email address and password to login.



      Important notes

      Make sure to verify your email address upon creating a new Auth0 account


      When we mention your Auth0 account, we mean your account used for auth.tribot.org as displayed below
      • 71 replies
    • To better support the upcoming changes (TRiBot X, new repository), we're switching our login handler to Auth0. Instead of logging in with the standard form, you'll now be required to login through our Auth0 application.

      All existing accounts which have been used within approximately the past year have been imported into Auth0 using the same email and password combination which has been stored on the forums.

      What does this mean for users?

      Your account credentials are now even more securely stored


      You'll be able to login via Facebook, Google, and others in the future


      Is there anything users have to do differently now?

      Existing users: You'll have to login with the standard login, open your Account Settings, then link your Auth0 account


      New users: You'll be redirected to our Auth0 app (auth.tribot.org) where you'll be able to create an account


      Why was this change made?

      The new apps we are creating (such as the new repository) aren't able to use the forums to handle user logins


      To centralize all user accounts in one area


      To ensure that the client login doesn't go down when the forums are having problems


      To speed up our development


      Other considerations

      There's no documentation or official support for using Invision Community combined with Auth0, so there are still a few kinks we're working out


      We're in the works of creating an account management panel specifically for Auth0 accounts (ETA August)


      It's not possible to change email addresses for the time being (this will be resolved this August)


      Changing passwords is a weird process for the time being. To change your password, you'll have to use the "Don't remember your password" tool on the Auth0 login page
      • 11 replies
    • Over the past month, we've been working hard on TRiBot's new repository - a much needed update. This change has been deemed necessary for TRiBot X, and will allow us to really speed up development of all aspects of TRiBot.

      Today we are going to share what we've been working on!


      Now you must be wondering what kind of features the new repository will have.... well, you'll have to be patient for a little while longer. We're still figuring out various technical aspects so we can't provide answers to all possible questions. We're also focusing on development rather than writing about it so that everyone can get access to our latest developments at lightning speed. I will however answer a few users' questions.

      We're planning on a release of this early to mid August, giving users some goodies before TRiBot X's release.

      Thank you all for being patient. I hope everyone is excited as much as I am!
      • 17 replies
    • Over the past few months, I’ve been working diligently on a new project - TRiBot X. Everything has been written from the ground up, with all of the best practices of software engineering. Every aspect of TRiBot has been re-imagined to support three main goals: flexibility, useability, and reliability.
      • 50 replies
  • Recently Browsing   0 members

    No registered users viewing this page.

×
×
  • Create New...