Open source F2P quester

[flexike]

4 hours ago, thatguycalledrob said:

Just picking this up again now I am moved into my new place for the year.

Classic Java noob mistake, I forgot to break; my case in my switch statement. Of all the things to go wrong!

I submitted a fix just now, and am hoping to add the next quest tonight or tomorrow!

I'm gonna test it for you, and post the results:) Gl for your next quests

 

Also you can check if the quest is completed with: "quests.isComplete(SHEEP_HERDER)" you don't have to do a new method for it

 

Quote

private int WhackAQuest() {
        if (!quests.isComplete(SHEEP_HERDER)){
            return 1;
        }
        if (!quests.isComplete(ROMEO_JULIET)){
            return 2;
        }
        return 0;
    }

Edited October 9, 2017 by flexike
+code

[thatguycalledrob]

3 hours ago, flexike said:

I'm gonna test it for you, and post the results:) Gl for your next quests

 

Also you can check if the quest is completed with: "quests.isComplete(SHEEP_HERDER)" you don't have to do a new method for it

 

I found "quests.isComplete()" to be buggy, it uses the colour of the quest in the quest menu, meaning that it would open the quest tab and run a (relatively) complex function call to iterate through all quests in the menu checking if the colour was red, yellow or green.

Using config ID's means that you can:

a) check without opening the quest menu
b) process faster, as it just tests a memory value

 

Thanks for testing though!

Edited October 9, 2017 by thatguycalledrob

[Antonio Kala]

2 hours ago, thatguycalledrob said:

I found "quests.isComplete()" to be buggy, it uses the colour of the quest in the quest menu, meaning that it would open the quest tab and run a (relatively) complex function call to iterate through all quests in the menu checking if the colour was red, yellow or green.

Using config ID's means that you can:

a) check without opening the quest menu
b) process faster, as it just tests a memory value

 

Thanks for testing though!

How did you know exactly how quest.isComplete worked and that it checked the quests by color? Just curious.

[ProjectPact]

Love the effort and end goal! Here is my two cents... before continuing development, maybe think about looking into tasks. The reason I say this is because you will be able to reuse code very efficiently. This not only saves time, but cleanliness as well. Not to mention finding bugs is very easy since tasks are only found in specific classes. It is not a big deal to keep it state. I had my Jug of Wine maker as a state script for 3 years without having to edit it once. However I switched to a task based system because of how much easier it is to manage and makes the entire code look clean. Don't be afraid of tasks, they aren't as scary as some people make them seem. Either way, you're on the right track. I was not able to look at the code in detail, but if you have questions you can always ask! Just some other advice, make sure you don't use static code and unneeded checks. For one this will help insure your code does not break upon updates, and two it will help with CPU problems that larger scripts tend to get.

[thatguycalledrob]

On 10/9/2017 at 9:14 PM, Antonio Kala said:

How did you know exactly how quest.isComplete worked and that it checked the quests by color? Just curious.

In intelliJ press mouse 3 or "f4" when clicked onto a function call - this will decompile the function.

Unfortunately, the decompiler isn't perfect, so you get odd variable names. However, it is still readable!

This is the isComplete function call:

Spoiler

public boolean isComplete(Quests.Quest quest) {
    return this.iIIIiiiIIIii() && this.iiiiIiiIIIiI.contains(quest);
}

 

this.iiiiIiiIIIiI.contains(quest); is checking that the quest is in the array list holding all of the quests - so will always be true for valid input.

return this.iIIIiiiIIIii() is what we are really interested in:

Spoiler

private boolean iIIIiiiIIIii() {
    if (this.getQuestPoints() == this.iIiIiiiIIiiI) {
        return true;
    } else {
        Tab iiiiIiiIIIiI = this.getTabs().getOpen();
        if (!this.IIiIIiiIIIiI()) {
            return false;
        } else {
            Collection iiiiIiiIIIiI = new ArrayList();
            RS2Widget[] var3;
            int var4 = (var3 = this.getWidgets().getWidgets(399)).length;

            int var5;
            int var10000;
            for(var10000 = var5 = 0; var10000 < var4; var10000 = var5) {
                RS2Widget iiiiIiiIIIiI;
                if ((iiiiIiiIIIiI = var3[var5]).getChildWidgets() != null && iiiiIiiIIIiI.getChildWidgets().length > 1) {
                    iiiiIiiIIIiI.addAll(Arrays.asList(iiiiIiiIIIiI.getChildWidgets()));
                }

                ++var5;
            }

            if (iiiiIiiIIIiI.isEmpty()) {
                return false;
            } else {
                this.iiiiIiiIIIiI.clear();
                this.iIIIiiiiiIii.clear();
                Iterator var10 = iiiiIiiIIIiI.iterator();

                while(true) {
                    while(true) {
                        while(true) {
                            label53:
                            while(true) {
                                for(Iterator var14 = var10; var14.hasNext(); var14 = var10) {
                                    RS2Widget iiiiIiiIIIiI;
                                    if (!(iiiiIiiIIIiI = (RS2Widget)var10.next()).isThirdLevel()) {
                                        continue label53;
                                    }

                                    if (iiiiIiiIIIiI.getMessage() != null) {
                                        String iiiiIiiIIIiI = iiiiIiiIIIiI.getMessage().toUpperCase().replaceAll(BFSWalkableAreaFinder.IIiIIiiIIIiI("?&9&2"), NullGraphics.IIiIIiiIIIiI("#")).replaceAll(BFSWalkableAreaFinder.IIiIIiiIIIiI(">&Ctc}c\u0005@"), "");
                                        Quests.Quest[] var13;
                                        int var7 = (var13 = Quests.Quest.values()).length;

                                        int var8;
                                        for(var10000 = var8 = 0; var10000 < var7; var10000 = var8) {
                                            Quests.Quest iiiiIiiIIIiI;
                                            if ((iiiiIiiIIIiI = var13[var8]).name().equals(iiiiIiiIIIiI)) {
                                                switch(iiiiIiiIIIiI.getTextColor()) {
                                                case 65280:
                                                    while(false) {
                                                        ;
                                                    }

                                                    this.iiiiIiiIIIiI.add(iiiiIiiIIIiI);
                                                    continue label53;
                                                case 16776960:
                                                    this.iIIIiiiiiIii.add(iiiiIiiIIIiI);
                                                default:
                                                    continue label53;
                                                }
                                            }

                                            ++var8;
                                        }
                                        continue label53;
                                    }
                                }

                                this.iIiIiiiIIiiI = this.getQuestPoints();
                                this.getTabs().open(iiiiIiiIIIiI);
                                return true;
                            }
                        }
                    }
                }
            }
        }
    }
}

 

 

So it's a little cumbersome to read, and I didn't make it so this may not be exactly how it works, but in order:
 

>Gathers all widgets into an array list:

Spoiler

Collection iiiiIiiIIIiI = new ArrayList();
RS2Widget[] var3;
int var4 = (var3 = this.getWidgets().getWidgets(399)).length;

int var5;
int var10000;
for(var10000 = var5 = 0; var10000 < var4; var10000 = var5) {
    RS2Widget iiiiIiiIIIiI;
    if ((iiiiIiiIIIiI = var3[var5]).getChildWidgets() != null && iiiiIiiIIIiI.getChildWidgets().length > 1) {
        iiiiIiiIIIiI.addAll(Arrays.asList(iiiiIiiIIIiI.getChildWidgets()));
    }

    ++var5;
}

 

 

> finds the widgets which relate to the quest you're looking for. I'm guessing the nested "while(true) {" loops are due to the decompiler - not how the actual source code is

Spoiler

if (iiiiIiiIIIiI.isEmpty()) {
    return false;
} else {
    this.iiiiIiiIIIiI.clear();
    this.iIIIiiiiiIii.clear();
    Iterator var10 = iiiiIiiIIIiI.iterator();

    while(true) {
        while(true) {
            while(true) {
                label53:
                while(true) {
                    for(Iterator var14 = var10; var14.hasNext(); var14 = var10) {
                        RS2Widget iiiiIiiIIIiI;
                        if (!(iiiiIiiIIIiI = (RS2Widget)var10.next()).isThirdLevel()) {
                            continue label53;
                        }

                        if (iiiiIiiIIIiI.getMessage() != null) {
                            String iiiiIiiIIIiI = iiiiIiiIIIiI.getMessage().toUpperCase().replaceAll(BFSWalkableAreaFinder.IIiIIiiIIIiI("?&9&2"), NullGraphics.IIiIIiiIIIiI("#")).replaceAll(BFSWalkableAreaFinder.IIiIIiiIIIiI(">&Ctc}c\u0005@"), "");
                            Quests.Quest[] var13;
                            int var7 = (var13 = Quests.Quest.values()).length;

                            int var8;

 

 

> Checks the text color of the label

Spoiler

if ((iiiiIiiIIIiI = var13[var8]).name().equals(iiiiIiiIIIiI)) {
    switch(iiiiIiiIIIiI.getTextColor()) {
    case 65280:
        while(false) {
            ;
        }

        this.iiiiIiiIIIiI.add(iiiiIiiIIIiI);
        continue label53;
    case 16776960:
        this.iIIIiiiiiIii.add(iiiiIiiIIIiI);
    default:
        continue label53;
    }
}

 

I hope this helps?

Mods pls don't ban me for revealing secret information or something lol

Edited October 14, 2017 by thatguycalledrob
submitted too early

[thatguycalledrob]

On 10/9/2017 at 9:40 PM, Project said:

Love the effort and end goal! Here is my two cents... before continuing development, maybe think about looking into tasks. The reason I say this is because you will be able to reuse code very efficiently. This not only saves time, but cleanliness as well. Not to mention finding bugs is very easy since tasks are only found in specific classes. It is not a big deal to keep it state. I had my Jug of Wine maker as a state script for 3 years without having to edit it once. However I switched to a task based system because of how much easier it is to manage and makes the entire code look clean. Don't be afraid of tasks, they aren't as scary as some people make them seem. Either way, you're on the right track. I was not able to look at the code in detail, but if you have questions you can always ask! Just some other advice, make sure you don't use static code and unneeded checks. For one this will help insure your code does not break upon updates, and two it will help with CPU problems that larger scripts tend to get.

This is the sort of feedback I need!

I have had a look at task-based coding, namely:

However, it seems to me that the only method that I can move to a task-based system is the talk to character "ConverseWithCharacter" method, which has clear "Can process" conditions and run conditions. I was actually wanting to move it to my Abstract Quest class for just that reason.

What else in the script would you automate tasks?

[thatguycalledrob]

UPDATE:
Now does 7QP - working on more though.

[thatguycalledrob]

UPDATE:
added rune mysteries

[aftabdear]

Don't use randomPosition() already does that when walking to an area?

[thatguycalledrob]

UPDATE:

Now completes The Restless Ghost 

[thatguycalledrob]

7 hours ago, aftabdear said:

Don't use randomPosition() already does that when walking to an area?

I know that now! I wish someone had told me earlier

I thought I was being all anti-pattern, but I ended up getting this weird bug where I pass WebWalk a position, and it dosn't walk to it. Actually, it webwalks just outside and then stops. I then get a conflict: Webwalk thinks its there, but its not.

This is why there is a LOCATION and LOCATION_SUPER in the questlocation enum. I'm going to have to do a walking investigation & rewrite for this script at some point soon...

Edited October 17, 2017 by thatguycalledrob

[TheWind]

On 10/14/2017 at 4:35 PM, thatguycalledrob said:

However, it seems to me that the only method that I can move to a task-based system is the talk to character "ConverseWithCharacter" method, which has clear "Can process" conditions and run conditions. I was actually wanting to move it to my Abstract Quest class for just that reason.

What else in the script would you automate tasks?

You should move any generic methods you would add to all your Quest classes. You seem to have an instance of EastyEntManipulator in every single quest class. You could just make an instance in your abstract class and have a method to retrieve it.

[Alek]

On 10/9/2017 at 1:57 PM, thatguycalledrob said:

I found "quests.isComplete()" to be buggy, it uses the colour of the quest in the quest menu, meaning that it would open the quest tab and run a (relatively) complex function call to iterate through all quests in the menu checking if the colour was red, yellow or green.

Using config ID's means that you can:

a) check without opening the quest menu
b) process faster, as it just tests a memory value

 

Thanks for testing though!

It sounds like you're implying that using text color means that it's buggy. Also part a and b are not completely valid arguments, the Quest API caches one time for initial values - then it's stored in memory (and called just as fast thereafter). The benefit to the way I set it up is that you don't need the configs and their values for hundred(s?) of quests, which would actually make the API a lot more prone to breaking than the color check. 

Just some things I noticed:

 

Quote

 

 public boolean isNpcValid(NPC npc) {
        DebugLog("Testing if an NPC is valid");
        if (npc != null && map.canReach(npc) && map.isWithinRange(npc, 9) && !combat.isFighting()) {
            int id = npc.getId();
            if (id != -1) {
                for (NPC i : getNpcs().get(npc.getX(), npc.getY())) {
                    if (i.getId() == id)
                        DebugLog("NPC is valid");
                        return true;
                }
            }
        }
        DebugLog("NPC is NOT valid");
        if (npc == null) {DebugLog("NPC is null");}
        if (!map.canReach(npc)) {DebugLog("NPC is unreachable");}
        if (combat.isFighting()) {DebugLog("you are in combat");}
        if (!map.isWithinRange(npc, 15)) {DebugLog("NPC is >15 away");}

        return false;
    }

 

 

The code in red is duplicate code, some of which is pretty expensive.

Also:

 

Quote

 

NPC Ghost = getNpcs().getAll().stream()
                .filter(n -> n.getId() == 922)
                .findFirst().orElse(null);

 

RS2Object Coffin = getObjects().getAll().stream()
                    .filter(n -> n.getName().equals("Coffin") && (n.getId() == 15061 || n.getId() == 2145))

 

I'm very certain this script will break in 2 days because you're using a ton of static ids. Also your streams should really be optionals, using the orElse(null) kind of defeats the purpose. 

It's not bad but with some minor changes you can make a really big improvement.

[thatguycalledrob]

4 hours ago, TheWind said:

You should move any generic methods you would add to all your Quest classes. You seem to have an instance of EastyEntManipulator in every single quest class. You could just make an instance in your abstract class and have a method to retrieve it.

Will implement this, thanks! 
I started coding ad-hoc at the start, and I have a lot of cleaning up to do!

[thatguycalledrob]

2 hours ago, Alek said:

It sounds like you're implying that using text color means that it's buggy. Also part a and b are not completely valid arguments, the Quest API caches one time for initial values - then it's stored in memory (and called just as fast thereafter). The benefit to the way I set it up is that you don't need the configs and their values for hundred(s?) of quests, which would actually make the API a lot more prone to breaking than the color check. 

 

Ah the man himself! Okay okay, I admit that the real reason that I didn't use it was because it didn't work as expected the first time (wouldn't return true when a quest was completed). However, since this whole script is config-based it did make some logical sense to use a different implementation! I guess the API focuses on robustness over most else.

Thanks for looking through the code though, it means a lot to have one of the main devs check it!

 

3 hours ago, Alek said:

The code in red is duplicate code, some of which is pretty expensive.

 

I still want to log which one is causing an error when I have Verbose mode, but you're right in that each if statement calls the function, even if VERBOSE = False.
I think I will wrap the whole thing in an if(Verbose){} statement, as to only call them the second time if I am actually wanting an output. Good catch on this, I wouldn't have seen this if I had been looking!

 

3 hours ago, Alek said:

I'm very certain this script will break in 2 days because you're using a ton of static ids. Also your streams should really be optionals, using the orElse(null) kind of defeats the purpose. 

 

Valid, I was worried about this yesterday.
I'm fairly new to OSbot & java coding here, so could you, or someone else please help me with a best practice example?
I am unsure how often ID's change, and the best implementation for options.

are we talking about:
https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html ?
 

if so - would you replace my stream:
RS2Object Coffin = getObjects().getAll().stream().filter(n -> n.getName().equals("Coffin") && (n.getId() == 15061 || n.getId() == 2145))
With something like:
RS2Object Coffin = Optional.ofNullable(getObjects().getAll()).filter(n -> n.getName().equals("Coffin")).orElse("Some default")?

Examples would be appreciated!
Looks like tomorrows session is just going to be a lot of code cleaning