Jump to content
×
×
  • Create New...

[Complete] Script Writer Application


iZho
 Share

Recommended Posts

Hey guys,

I'd like to apply for Script Writer. I know the script is basic, But I'd still like to contribute where I can.

The script burns any logs in the GrandExchange, Paint is currently disabled however will be fixing that in the next few days. Might also add progressive leveling & more locations if that's something people would like to see.
https://github.com/q98/Firemaker

 

Link to post
Share on other sites

  • Moderators

Nice to see the first application with the V2 API. Especially with no docs available, hats off.

Some feedback below

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

https://github.com/q98/Firemaker/blob/25bbf680955f89e0c7d7cd33ab6d1b9ffd162c7f/Firemaking.java#L100

I hate static sleeps, sleeping for 1 second tells you nothing, there's no guarantee that that time is enough to complete the action and/or is too long. It also has a fixed time and therefore a likely obvious pattern will start to get created.

Take a look at this docs below. While they're still written in the V1 API, the conditional wait is still the same.

https://docs.powbot.org/#/Fundamentals_In_Practice/UsingWaits

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

Tidier structure.

While not everyone likes using a task structure, some like tree/branch scrips and graph scripts etc etc but slapping everything into 1 place makes it hard to maintain in the future. You spoke about adding more features such as progressive levelling etc but that's going to get more and more difficult with having all the code in basically 2 files.

Again I know this is written in V1's API but have a look at Tomas guide on the task framework. It might help you split your tasks into individual task files which will be easier to manage going forwards.

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

Features.

In terms of features this script has, is minimal, while it's a much needed script I'd quite like to see some of the following.

Varrock West bank - People like to firemake here too, a separate choice of location is always good.

Progressive levelling - Firemaking levels come in thick and fast and really, this script is going to be used 1-50 before wintertodt, if someone can just buy all the logs and press play from 1 and come back at 50 using all the right logs, that's much better than having to stop and start it to select the new logs.

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

All in all, it's a nice script, good foundation and you're definitely getting on well with the API. Feel free to @ me when you've made some changes and want me to take another look.

Link to post
Share on other sites

  • Dan changed the title to [On Hold] Script Writer Application
Posted (edited)

Below is a version 2 of this script, taking into consideration the points provided above.

This features:

All logs, if going for 99 🙂

 

Progressive leveling 1-50

 

Mutliple locations:

Draynor

Varrock West Bank

Varrock East Bank

Grand Exchange

https://github.com/q98/Firemakingv2

As you'll see I've rewritten most of the script to make it a bit cleaner, and also use the task system.

 

@Danif you end up looking through this before I add paint after I get out of work; please don't hold it against me 😛

Edited by iZho
Link to post
Share on other sites

  • Dan changed the title to [Under Review] Script Writer Application
  • Dan locked this topic
  • Moderators

So ignoring the missing paint for now 😉

In terms of feedback, such a good improvement on what was there. What you had wasn't bad, but was unmanageable all being in 2 files and some of the general practices were a bit awkward (sleep(1000) etc).

The biggest piece of feedback I can really offer now is to try not to use the same code multiple times and to generify your tasks. For example, you have a task for each area which look almost identical. What you could do instead is set all the necessary variables from the user selection and have 1 task which uses these variables. For example, my agility script, I didn't write a task for each course, I instead mapped the area of each object, the name and action in an enum and I passed those values to 1 action task. That 1 action task takes all the variables needed to complete. You could do the same here with your areas.

However, all that being said, I'd like to congratulate you on becoming part of the team.

@const_ please do what you gotta do and promote the man 😁

Link to post
Share on other sites

  • Administrators

Congratulations! I've messaged you with the details.

Link to post
Share on other sites

  • Dan changed the title to [Complete] Script Writer Application
Guest
This topic is now closed to further replies.
 Share