Jump to content
  • Create New...

[SOURCE] Warriors Guild Tokens | All armours | Banking | Eating


Recommended Posts

  1. Throughout the whole script, you call "main.whatever" for accessing values. Don't do this! Instead pass the values you need to each class that needs it.
  2. XP has a maximum of 200 million so no need to make them longs - they fit perfectly in an int. (source)
  3. Instead of getting/storing each skill XP individually, you could use a loop to do everything. (source)
  4. In software, there is the principle of single responsibility... A class should do one thing. You should move looting into its own class, and not stuff it in your Attack class.
  5. Don't store queries like this. Query contents are persistent until you call select(). See here.
  6. Because query contents are persistent, and you queried for it in your activate(), you don't need to query for your food again here. You can just call inventory.poll();
  7. When you have a literal (no variable) in your code, it's called a "magic value." Don't use magic values. Set the tiles to variables with meaningful names, so you can easy find and update them when necessary. 
  • Like 1
Link to post
Share on other sites

your attack task changes name of main.currentTask 3 times, maybe make 3 separate classes to make it more clean and consistent with the task structure.

You reuse some code like your fail safe if what you want to interact with isn't in viewport, I'd make a util class to abstract those interactions. That way you only have to edit code once if you wish to improve some logic
Also why not check if interactive objects are in viewport first, then handle it and then interact. That way it doesn't have to run activate() and execute() all over again to actually accomplish what it wanted 

lambdas are your friend, makes your life easier.

also I still think your npc query is a mess. you do what you want, but i would really cache the npc inside a seperate task for animating the armour, would make activate() for attack much more obvious ( if npc.valid() kill that mofo)

i'm giving you a medium star to not discourage you


Link to post
Share on other sites

  • Moderators

Having written a full  Warriors Guild script previously, I know some things can be a little trickier to handle.

I see a lot of things that I did in my script here, which isn't always the best considering my warriors guild script is a bag of spaghetti. (Hence why I took it down)

I don't really have much feedback over and above what's already been posted; I just wanted to say it's nice to see someone new come in with something that's not a generic skiller.


Link to post
Share on other sites