Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add BattleOrders tardy check and quit/continue vars #2013

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jmichelsen
Copy link

This removes the unnecessary delay when someone rejoins a game or joins
late and the time for battle orders has passed. The bot will now check
for other player locations and just continue on instead of waiting the
BattleOrder timeout. Instead of quitting if the timeout is reached,
now the character can optionally just continue on with the run. These
two changes combined make for smoother battle order control.

This removes the unnecessary delay when someone rejoins a game or joins
late and the time for battle orders has passed. The bot will now check
for other player locations and just continue on instead of waiting the
BattleOrder timeout. Instead of quitting if the timeout is reached,
now the character can optionally just continue on with the run. These
two changes combined make for smoother battle order control.
@jaenster
Copy link
Contributor

jaenster commented Dec 3, 2019

So, if the leader is the bo-er, it simply doesnt bo, and moves on the the next script.

As the leader, comes in game as first, and therefor seems to be empty

Copy link
Contributor

@jaenster jaenster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like an neat addition, specially removing the force quiting on failure. As, sometimes you specifically want this, and sometimes specifically do not want this.

Got some minor points of improvement, attach to much judgement to my review, as its a matter of interpretation / different point of view

d2bs/kolbot/libs/bots/BattleOrders.js Outdated Show resolved Hide resolved
d2bs/kolbot/libs/bots/BattleOrders.js Outdated Show resolved Hide resolved
d2bs/kolbot/libs/bots/BattleOrders.js Outdated Show resolved Hide resolved
d2bs/kolbot/libs/bots/BattleOrders.js Outdated Show resolved Hide resolved
d2bs/kolbot/libs/bots/BattleOrders.js Outdated Show resolved Hide resolved
d2bs/kolbot/libs/bots/BattleOrders.js Outdated Show resolved Hide resolved
d2bs/kolbot/libs/common/Config.js Outdated Show resolved Hide resolved
d2bs/kolbot/libs/bots/BattleOrders.js Outdated Show resolved Hide resolved
d2bs/kolbot/libs/bots/BattleOrders.js Outdated Show resolved Hide resolved
@jmichelsen
Copy link
Author

So, if the leader is the bo-er, it simply doesnt bo, and moves on the the next script.

As the leader, comes in game as first, and therefor seems to be empty

Hmm. I've found that even if the leader is the BOer, the townChores are usually long enough to prevent him from getting to the BO spot first and bailing before others arrive. There's also the "wait for others"...which, I'm not sure if that is checked before or after empty game. I'll look though. I think having a configurable wait time for empty game before bailing would solve this, instead of relying on townChores stalling the leader long enough.

I have three bots running, one is the leader, a second one is the BOer, and a third just tags along and helps with assistant scripts. The problems I've run into that caused this PR were if one of the bots was doing automule, the others would quit the game, or wait at the BO if it happened to be the BOer that was muling or FTJ. I'll take another pass at this though, to add some more smoothing for other situations that aren't as specific as mine.

Thanks for your feedback!

@jmichelsen
Copy link
Author

I've made this more robust and clean. It should be ready for final review. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants