PrismarineJS / mineflayer-statemachine

A state machine plugin for Mineflayer to aid in designing more complex behavior trees.
https://thedudefromci.github.io/mineflayer-statemachine/
MIT License
111 stars 26 forks source link

BehaviorMineBlock stops too much #88

Closed sefirosweb closed 3 years ago

sefirosweb commented 3 years ago

Java Versión Vanilla 1.6.2 Server Windows -> localhost Node: v14.15.1 Version of mineflayer "minecrafthawkeye": "^1.0.8", "mineflayer": "^2.40.1", "mineflayer-auto-eat": "^1.3.4", "mineflayer-pathfinder": "^1.3.6", "mineflayer-statemachine": "^1.5.1", "mineflayer-web-inventory": "^1.2.0", "prismarine-viewer": "^1.5.0"

Hello!

I'm trying to use the "BehaviorMineBlock" Behavior, for use is a simply in previos State i set targets.positions = (position)

I see is working but they stops after break 3-4 or 10 blocks (very random)

I think they have bug on the code or something,

I saw they breaks the block but don't "detect the change then the "isFinished" still on false for all time


// full code  https://github.com/sefirosweb/minecraftLegion/blob/master/NestedStateModules/miningFunction.js
  const moveToBlock = new BehaviorMoveTo(bot, targets)
  moveToBlock.stateName = 'Move To Block'

  const mineBlock = new BehaviorMineBlock(bot, targets)
  mineBlock.stateName = 'Mine Block'

...

    new StateTransition({
      parent: moveToBlock, // This is a "Move To Block" label
      child: mineBlock,
      name: 'Move To Block',
      shouldTransition: () => moveToBlock.distanceToTarget() < 3
    }),

    new StateTransition({
      parent: mineBlock, // This is a "Mine Block" label
      child: currentBlock,
      name: 'Go Next Block',
      shouldTransition: () => mineBlock.isFinished
    })

... // for set the position in previus state:

  calculateIsValid () {
    const block = this.getBlockType()
    const isValidBlockType = this.blockType.find(b => b === block.name)
    if (isValidBlockType === undefined) {
      this.targets.position = block.position // I detect is not air / lava / water then go to this position
      return false
    }

    return true
  }

I'm trying what see happens, this also happens for you?
sefirosweb commented 3 years ago

I checking the code and seems the origin of issue is from mineflayer not from -statemachine

Is calling to "bot.dig" function and never returns

sefirosweb commented 3 years ago

Maybe need to check if have error on diggin

image

TheDudeFromCI commented 3 years ago

It might be easier to implement the collectBlock in this instance since that plugin handles moving to blocks and mining them in a single API call.

sefirosweb commented 3 years ago

Hi think i have an fix :D

In behavior you are using "callbacks", but this function runs too many fast is executing before is finished, this breaks the code But is is changed to promises and wait until is finished works fine

I'm making some tests,

Another issue the "tool" no works fine for some blocks also i'm making a patch,

I hope soon send you a pull request

Ex:

  startDig (block) {
    const _this = this
    this.bot.dig(block)
      .then(() => {
        this.isEndFinished = true
      })
      .catch(err => {
        console.log(err)
        setTimeout(function () {
          _this.onStateEntered()
        }, 5000)
      })
  }
sefirosweb commented 3 years ago

It might be easier to implement the collectBlock in this instance since that plugin handles moving to blocks and mining them in a single API call.

Umm I prefer to have it separately because I want to check if the bot is "on" the block, if it picks up the block it can fall into the lava or into the void

sefirosweb commented 3 years ago

@TheDudeFromCI i saw in 1.6 you already changed the code to promise :D works high better

Same thing as i done when i have 1.5 =P then no need PR,

But one thing, when bot try to change weapon and servers reject the action the bot dies, image

I think can add retry waiting 1 second (or similar action)? or how you do this catch errors?


    const _this = this // I don't know how bind "this" in typescript
    const tool = this.getBestTool(block)
    if (tool != null) {
      this.bot.equip(tool, 'hand').then(() => {
        this.bot.dig(block).then(() => {
          this.isFinished = true
        }).catch(err => {
          console.log(err)
          setTimeout(function () {
            _this.onStateEntered()
          }, 1000)
        })
      }).catch(err => {
        console.log(err)
        setTimeout(function () {
          _this.onStateEntered()
        }, 1000)
      })
    } else {
      this.bot.dig(block).then(() => {
        this.isFinished = true
      }).catch(err => {
        console.log(err)
        setTimeout(function () {
          _this.onStateEntered()
        }, 1000)
      })
    }
  }
sefirosweb commented 3 years ago

Finally I found wich is the issue

Moving + Digging have a bit conflict

behaviorMoveTo.isFinished() don't wait unti the digging is completed,

If while moving detect for go to position need a dig and this starts digging, but at same time the moviment has been completed, and next behavior is digging, they start the second digging when the first one is not finished.

Now I need how i can detect if pathfinder is on "digging" @_@

TheDudeFromCI commented 3 years ago

Oh, I see what you mean. You can use bot.pathfinder.isMoving() || bot.pathfinder.isMining()

sefirosweb commented 3 years ago

Yes but the problem if you put "pathfinder.setGoal(null)" this is changed isMoving() & isMining to false instantaly, but in reality "setGoal" sends to event, and is changed in next physyctick, instead the current tick, then they still mining when i get isMining() and returns false @_@

Well.., I try to do some tests and I tell you anything =P

sefirosweb commented 3 years ago

I tried with:

     shouldTransition: () => {
        const block = bot.blockAt(targets.mineBlock)
        // console.log(bot.pathfinder.isMoving())
        if (bot.canDigBlock(block)) {
          console.clear()
          console.log('is moving', bot.pathfinder.isMoving())
          console.log('is mining', bot.pathfinder.isMining())
          console.log('is building', bot.pathfinder.isBuilding())

          if (!bot.pathfinder.isMoving() && !bot.pathfinder.isMining()) {
            return true
          }
          bot.pathfinder.setGoal(null)
        }
      }

With this code first is checking isMoving() & mining() and after send stops to event, then have 1 physyctick time for stop, This decresed a bit the issue, but randonly still having the same problem,

I think some times need more than 1 physyctickfor fullstop the pathfinder, I don't want make a "setimetout" I want to get the best perfomance possible =P

sefirosweb commented 3 years ago

Finally I got it!


let isDigging = false // Global variable
...
      shouldTransition: () => {
        const block = bot.blockAt(targets.mineBlock)
        if (bot.canDigBlock(block)) {
          if (bot.pathfinder.isMining() & isDigging === false) {
            isDigging = true

            bot.on('diggingAborted', () => {
              console.log('aborted')
              isDigging = false
            })

            bot.on('diggingCompleted', () => {
              console.log('completed')
              isDigging = false
            })
          }

          if (isDigging === false && !bot.pathfinder.isMining()) {
            bot.removeAllListeners('diggingAborted')
            bot.removeAllListeners('diggingCompleted')
            return true
          }
          bot.pathfinder.setGoal(null)
        }
      }

This is my brain now XD image

Well i explain a bit

Now when bot detect isMining, then puts a global variable to true, and start a 2 event listener this wait until pathfinder send stop to dig and whend is done the event is triggered,

In this moment the dig is really finished,

When is finished I remove the event and all os done!

I will try to do a PR into the core of pathfinder for isMining()

For a now if you wish can close the topic =)

I hope this can help to others programmers

sefirosweb commented 3 years ago

PR done to core of pathfinder! good night!

TheDudeFromCI commented 3 years ago

Very nice! Glad you got it working!

flowl commented 3 years ago

I have come across the same problem but it's not yet fully clear to me.

I've added

onStateExited() {
    this.finished = false
}

to the mineBlock behavior.

I also put

onTransition: () => {
    bot.pathfinder.setGoal(null)
}

into all Transitions that have a moving behvior as parent.

My transition condition (from mining to moving!) is:

new StateTransition({
    parent: mineBlock,
    child: getClosestCollectableEntity,
    shouldTransition: () => {
        let finished = mineBlock.isFinished()
        let mining = bot.pathfinder.isMining()
        return finished && !mining
    }
}),

Also I made sure to use proper Promise resolving with then() and removed some wrong await/async in my code.

This way, it seems to work.

sefirosweb commented 3 years ago

Hi @flowl well the resume of problem is:

mineflayer-pathfinder have a bug with the function of bot.pathfinder.isMining()

isMining is not retunring a "real" state of pathfinder, I created a pull request for fix this issue (https://github.com/PrismarineJS/mineflayer-pathfinder/pull/113)

Until is not merged and deployed to npm you need to check this manually,

How? Ok then you need a first global variable:

let isDigging = false Befoure you "stop" pathfinder you need check if is mining or not (is very important this point) isDigging = bot.pathfinder.isMining()

In case isMining is true then you need to create a event lisener for detect when it is really done

isDigging = true

bot.on('diggingAborted', () => { // This is when mining is force to abort console.log('aborted') isDigging = false })

bot.on('diggingCompleted', () => { // This if the minins has been finished correctly console.log('completed') isDigging = false })

When the event is triggered they are really stopped mining You need both listener because the mining can "resolved" with differents methods =P

For no colapse the program with alot of events listeners you need stop event listener when is done and also "is digging" passed to false, Because if you no "remove" the event listener, every time when the code is lunch generate a new "event" in memory,

if (isDigging === false && !bot.pathfinder.isMining()) { bot.removeAllListeners('diggingAborted') bot.removeAllListeners('diggingCompleted') return true }

at end of code you must start to launch "stop" pathfinder

bot.pathfinder.setGoal(null)

** Note: if in the first moment the isMining return false when until no stopped pathfinder is good, because is not mining, then don't no need anything, but you must check it first

Other alternative, you need wait until pullrequest is done =P only you need use !pathfinder.isMining()

If you can share your github code I try to help you,

Best!