Code Review My GrandExchange Util classes

[yfoo]

As title states, please offer your critques on my classes related to the grand exchange. 

There are 3 source files comprising this project, First is the GrandExchangeOperations.java file. In here are the methods for buying and selling items. 

The return of the buy and sell methods are GrandExchangeOffer instances, these are a data structure used to hold information related the the offer, such as the itemID, GrandExchange box, current amount traded, etc. 

There is a heavy reliance on static widget ids down to the 3rd level, there is possibly a way to use some filtering on some criteria to find the correct widgets but I haven't figured out how to do that yet. 

GrandExchangeOperations.java

https://pastebin.com/cB384h6d

 

This the implentation of the aforementioned data structure. There is some awkward implementation of a object pool where only 8 GrandExchangeOffers are allowed to be created through a static getInstance method (as there are only 8 ge boxes). Looking back on this from a few days after I wrote it, I don't feel this is necessary and just makes things more complicated.

GrandExchangeOffer.java

https://pastebin.com/jUHEVZTe

 

Finally I needed a way to know when an offer has completed. I decided to the observor pattern to do this. This class uses a seperate thread to check all active ge offers and notifies all listening classes when a ge offer has updated. 

GrandExchangeObservor.java

https://pastebin.com/GVwcuvKs

 

The goal of this project was mainly because I wanted a listener for ge offers, I'm writing an unfinished potions script, one requirment is given a sufficient cash stack, handle buying clean herbs and selling the finished... unfinished potions. Collection of herbs from the GE needs to be weaved in to the standard cycle of...

bank and restock -> create unfinished potions -> bank and restock. 

A 1k stack of clean herbs is expensive (1k toadflax = 3m) and can all be converted in about 15mins.  I wanted something that can be left alone for 2hours without me having to put like 24m or whatever on the account. 

 

If you got this far, thanks for reading. 

[battleguard]

If you want to get away from using static ids for things like getting the box widgets you should look into using bounding box sizes, x and y values for figuring out which box is what. For the buy and sell button on the box you can use Sprite ID's. That is how it is done in the OSBOT source code.

[liverare]

Whenever I make a custom API for OSBot, I extend the API class onto that object:

Also, RSBuddy can read your Grand Exchange offers without you ever having to be present at the GE. My guess is that there are configs you can use to read your offers. It'd be more optimal to these, because you're likely to have fewer IDs and a lot less code. Furthermore, you don't have to rely on Widgets nearly as much, which is something you should definitely consider doing, because Jagex seem to be playing around with an awful lot of them as of late.

https://osbot.org/api/org/osbot/rs07/api/Configs.html

Also, the bot has config listener too: https://osbot.org/api/org/osbot/rs07/listener/ConfigListener.html

You can instantiate and implement listeners in the initializeModule() method of the API, which you then must call after you instantiate the API and exchange the bot with it. Translation:

public class CustomGrandExchangeAPI extends API implements ConfigListener {

	@Override
	public void initializeModule() {
		bot.addConfigListener(this);
	}

	public void onConfig(int id, int value) {
		/* Register changes to GE offers */
	}
  
  	/* Fun code stuff */
}

Then in your script
 

CustomGrandExchangeAPI customGEAPI;

@Override
public void onStart() {

	customGEAPI = new CustomGrandExchangeAPI();
	customGEAPI.exchangeContext(bot);
	customGEAPI.initializeModule();
}

 

I haven't read the code in great depth yet, so this is just my precursory evaluation.

Edited March 8, 2018 by liverare

[yfoo]

1 hour ago, battleguard said:

If you want to get away from using static ids for things like getting the box widgets you should look into using bounding box sizes, x and y values for figuring out which box is what. For the buy and sell button on the box you can use Sprite ID's. That is how it is done in the OSBOT source code.

Upon reading this and looking at the Widgets class, there are alot of things that I should be leveraging. Thanks. 

[battleguard]

23 minutes ago, liverare said:

Whenever I make a custom API for OSBot, I extend the API class onto that object:

Also, RSBuddy can read your Grand Exchange offers without you ever having to be present at the GE. My guess is that there are configs you can use to read your offers. It'd be more optimal to these, because you're likely to have fewer IDs and a lot less code. Furthermore, you don't have to rely on Widgets nearly as much, which is something you should definitely consider doing, because Jagex seem to be playing around with an awful lot of them as of late.

https://osbot.org/api/org/osbot/rs07/api/Configs.html

Also, the bot has config listener too: https://osbot.org/api/org/osbot/rs07/listener/ConfigListener.html

 

 

The configs do not hold the current grand exchange trade data sadly it must be in a hook that osbot has not implemented.

[yfoo]

18 minutes ago, liverare said:

 

CustomGrandExchangeAPI customGEAPI;

@Override
public void onStart() {

	customGEAPI = new CustomGrandExchangeAPI();
	customGEAPI.exchangeContext(bot);
	customGEAPI.initializeModule();
}

 

I haven't read the code in great depth yet, so this is just my precursory evaluation.

To clarify, by extending API and calling exchangeContext under onStart() I don't have to pass in the a script reference to access all the Script.getXXX() methods. 

Also do you happen to know the config id for the GE and how to parse the int returned by Configs.get(int id)?

[Lemons]

37 minutes ago, PayPalMeRSGP said:

To clarify, by extending API and calling exchangeContext under onStart() I don't have to pass in the a script reference to access all the Script.getXXX() methods. 

Also do you happen to know the config id for the GE and how to parse the int returned by Configs.get(int id)?

 

Use getGrandExchange().getStatus(Box) and the other Box related methods for getting GE data. It tells you everything but how many are left to collect to my knowledge. The only configs that are used with GE are from the buy/sell interfaces (quantity, item ID, price) iirc.

https://osbot.org/api/org/osbot/rs07/api/GrandExchange.html

Edited March 8, 2018 by Lemons

[liverare]

50 minutes ago, battleguard said:

The configs do not hold the current grand exchange trade data sadly it must be in a hook that osbot has not implemented.

Didn't know what.

36 minutes ago, PayPalMeRSGP said:

To clarify, by extending API and calling exchangeContext under onStart() I don't have to pass in the a script reference to access all the Script.getXXX() methods. 

Also do you happen to know the config id for the GE and how to parse the int returned by Configs.get(int id)?

Well it depends on how you implement stuff. Try to keep everything contained in the API class, and keep the inner-classes non-static so they have access to methods. Otherwise, you'd need a static reference still. However, I would avoid statically referencing the script object.

Also, @battleguard says there's a hook that OSBot hasn't implemented that will get you the information. You can't read GE information using settings (afaik).

Edited March 8, 2018 by liverare

[Alek]

 

4 hours ago, Lemons said:

 

Use getGrandExchange().getStatus(Box) and the other Box related methods for getting GE data. It tells you everything but how many are left to collect to my knowledge. The only configs that are used with GE are from the buy/sell interfaces (quantity, item ID, price) iirc.

https://osbot.org/api/org/osbot/rs07/api/GrandExchange.html

Exactly what Lemons said, not sure why we're spreading misinformation about our API not having this functionality and it being an RSBuddy exclusive. I'm not using configs anywhere in the GE API. Also you might want to leverage some of the existing methods, looks like you're rewriting a lot of code just so you have something that collects the items at the end. You can have them as separate methods without it looking bad by any means. Unless of course there's something about sellItem/buyItem that you absolutely hate.