Jump to content
  • Create New...

[Complete] SW1 Application


Recommended Posts

  • Administrators

So some feedback:

  1. All your logic is in one loop() method, I'd suggest breaking it out into separate tasks or branches/leaves.
  2. Don't use global variables as much, for example you have a stall variable that's written to in different methods. This makes it hard to keep track of what it is and if you were to introduce any concurrency could lead to issues. Instead use more local variables and pass them between functions, your function call chains shouldn't have any side effects.
  3. Make use of enums, you currently have multiple different variables which are used in conjunction with each other depending on the mode. Try grouping this together inside a Mode enum.
Link to post
Share on other sites

  • Dan changed the title to [On Hold] SW1 Application
  • Moderators

I don't have access to a lot of your extended methods etc so some of this is going to be trying to assume the desired effect and implementation.

PickpocketOrWait.java - Your waitUntil method, I'm assuming it's args are, boolean, looping time in ms, looping tries before exiting. If so, I don't see why you aren't just using the Condition.wait() from the api. However I also remember you saying something in my dm's about passing a timeout feature or something like that. Whatever the case, passing 10,000 and 15,000 seems a little extreme however, I can't really comment any more than that as I don't have access to the method however I would argue it's an unnecessary custom method to have with the existing Condition.wait().

Keeping with the same task, I don't quite follow the logic used. It validates if your thieving has dropped less than 5, I might be out of the loop but as far as I know thieving and failing doesn't drop your thieving level does it? Either way, IF it does activate, the first thing you check is if your hitpoints are <2, if so just idle. That seems rather dangerous logic to lock up the script for an extended period of time at 1 hp without any features to eat or to run to a safe spot included. Just because it makes no sense to do this in full 3rd age and a tbow, it doesn't mean someone won't and then come complaining to you that you lost them their whole bank because something happened and the script locked them out for 15 seconds.

I'm not particularly a fan of task validations which are standard set to true. Your StealFromStall validate is always true, no validation in there at all which then forces your execute to run and subsequently unnecessarily wait.

I personally don't like home brewed API's, over the years they break and don't get updated. "I don't want to publicly post everything I've implemented."  If there's something that the API is missing so badly, as part of the script writer team, you should aim to improve the API and add to it for the better of the group rather than working solo. This is just personal preference but that doesn't take away from your clear capabilities.

Overall it's a very well put together script. Overcomplicated extended api methods for example:

//feature is already in api

However I'm assuming the version I was sent was already amended from Consts feedback as it's been split into a nice neat structure with references to enums etc.

The commentary and suggestions provided are just that, food for thought but not required to proceed with the application as I'm happy you've shown enough understanding and beyond to be approved, promoted and accepted by all 😄

@const_ If you can do the honours.

Link to post
Share on other sites

  • Dan changed the title to [Complete] SW1 Application