Simple herb cleaner (my first script) - feedback appreciated!

[snerd]

Hey guys, this is my first post here. Wanted to say hi by posting a free to use script that I wrote in Scala.

Wondering if anyone else has written scripts in Scala. If you're unfamiliar with it, I definitely recommend checking it out.

 

Spoiler

import javax.swing._

import org.osbot.rs07.api.ui.Skill
import org.osbot.rs07.script.{MethodProvider, Script, ScriptManifest}

import scala.annotation.tailrec
import scala.util.Try

@ScriptManifest(
  name = "Simple herb cleaner (scala)",
  author = "snerd",
  version = 1.2,
  info = "Cleans any herb",
  logo = "")
class HerbCleaner2 extends Script {
  import HerbCleaner2._
  import MethodProvider._

  override def onStart(): Unit = {
    log("Starting snerd's herb cleaner")
    log("Version: 1.2")

    startMs = System.currentTimeMillis
    startXp = getSkills.getExperience(Skill.HERBLORE)

    herbName = {
      val s = JOptionPane.showInputDialog(s"Herb name ($herbName)").trim
      if (s.isEmpty) herbName else s.trim
    }

    Try(JOptionPane.showInputDialog(s"Max sleep duration ($maxSleep ms)").toInt).foreach { i =>
      maxSleep = i
    }
  }

  override def onExit(): Unit =
    log("Thank you for using snerd's herb cleaner!")

  override def onLoop: Int = {
    val xpGained  = getSkills.getExperience(Skill.HERBLORE) - startXp
    val elapsedMs = System.currentTimeMillis - startMs + 1
    log("xp gained: " + xpGained)
    Try(log("xp per hour: " + xpGained / (elapsedMs / MS_PER_HOUR)))
    Try(log("xp per min: " + xpGained / (elapsedMs / MS_PER_HOUR) / 60))

    stockUp()
    clean()
    random(100)
  }

  def stockUp(): Unit = {
    if (!bank.open) throw new Exception("Couldn't open bank")

    do {
      bank.depositAll
      sleep(200)
    } while (inventory.getAmount(_ => true) > 2)

    if (bank.getAmount(_.nameContains(herbName)) == 0) {
      log("No more " + herbName + ". Ending script")
      stop(false)
    }

    do sleep(150) while ({
      !bank.withdrawAll(herbName)
    })

    if (random(100) < 10) {
      log("waiting for a bit...")
      sleep(random(10000))
    }

    bank.close()

    sleep(100 + random(200))
  }

  def clean(): Unit = {
    sleep(100)
    if (inventory.isFull && inventory.isEmptyExcept(herbName))
      cleanSmart()
    else
      cleanDumb()
  }

  def cleanDumb(): Unit = {
    log("Cleaning dumb")
    val items = inventory.getItems
    items.find(_.getName == herbName).map { e =>
      e.hover()
      mouse.click(false)
    }
  }

  def cleanSmart(): Unit = {
    zigZag { cell =>
      mouse.click(inventory.getMouseDestination(cell))
      sleep(random(maxSleep))
    }
  }

  def fullInv: Boolean = inventory.isFull && inventory.isEmptyExcept(herbName)
}

object HerbCleaner2 {
  private var herbName        = "Grimy tarromin"
  private var maxSleep        = 120
  private var startMs: Double = 0
  private var startXp: Double = 0
  val MS_PER_HOUR: Double     = 3600000

  def zigZag[T](fn: Int => T) = {

    @tailrec
    def iter(maybeCell: Option[Cell]): Unit = {
      if (maybeCell.isDefined) {
        val cell = maybeCell.get
        fn(maybeCell.get.value)

        val next: Option[Cell] = cell.column match {
          case 0 | 2 => if (cell.isAtTop) cell.left else cell.up
          case 1 | 3 => if (cell.isAtBottom) cell.left else cell.down
        }

        iter(next)
      }
    }

    iter(Some(topRight))
  }

  case class Cell(value: Int) {
    require(value >= 0 && value < 28)

    def isAtTop    = value <= topRight.value
    def isAtBottom = value >= bottomLeft.value
    def column     = value % 4

    def up    = Try(Cell(value - 4)).toOption
    def down  = Try(Cell(value + 4)).toOption
    def right = Try(Cell(value + 1)).toOption
    def left  = Try(Cell(value - 1)).toOption
  }

  val topRight    = Cell(3)
  val topLeft     = Cell(0)
  val bottomRight = Cell(27)
  val bottomLeft  = Cell(24)
}

 

[Explv]

Would recommend sticking to Java, I don't think Scala scripts would be accepted to the SDN, and a lot of people here don't know Scala so it would be a *little* trickier to assist you with any issues.

I personally don't know Scala, but in general your script looks pretty good.

There are a few small issues with your script that I can see:

1. You throw an exception if you fail to open the bank, surely you want to just try again rather than kill the script completely?
2. Your random sleeps for example after depositing items, before cleaning herbs, at the end of banking are very unnecessary. They add nothing to the script but XP waste.
3. You try to close the bank at the end of the stockUp method, but you don't check if the bank was actually successfully closed. If it fails to close, your script is going to mess up. 
4. You have a lot of loops in your script, if something were to majorly fuck up it would get stuck and the user would have to force close the client.
5. I can't be bothered to read your "ZigZag" code, but i'm pretty sure whatever you're doing could be achieved in a much simpler fashion without extra classes and recursion.
6. Why are you using the mouse hover & click functions when you can just use interact() is this another form of "antiban"? Or is it to try and speed up mouse interactions?
7. XP gained, XP per hour, etc. can be all be obtained using the ExperienceTracker class, no need to do this yourself.