Pathing improvements for walking bots #1381

MikeKGilmore · 2019-03-26T14:29:22Z

Primary changes:
Pather.moveTo() changed to Pather.pathTo() to account for behavior changes. PathTo() should be used when traveling long distances or when it's acceptable to open chests/shrines along the way.
Pather.maneuverTo() added to behave more closely to prior Pather.moveTo(). maneuverTo should be used if the bot should simply move from point A to B without opening chests/shrines (but clear monsters along the way if clearPath is set). Small movements should be performed with maneuverTo()
Added Chest object in Misc.js, improved chest opening behavior to prevent thrashing

noah- · 2019-03-26T17:09:01Z

can you submit a minimal version of this change that does not rename existing functions or move the code from one file to another so I can look at actual changes made

noah- · 2019-04-14T05:12:32Z

this needs to be rebased, it includes other commits

MikeKGilmore · 2019-04-14T05:31:38Z

good grief sorry my master is a mess. I'll fix it

MikeKGilmore · 2019-04-14T06:16:02Z

there - fixed it.

noah-

Overall, I don't think this is a robust design, see my comments from discord:

4:35 PM] noah: I think the design is redundant, take this for example: function(x, y, retry, clearPath, pop, maneuvering)
[4:35 PM] noah: you have 3 args, clearPath, pop, and manuvering - these are all actually "node actions"
[4:35 PM] noah: i would change it to something like
[4:36 PM] noah: x, y, retry, action = false   --- default argument is false, otherwise you pass an array of actions you want to do
[4:36 PM] noah: 'false' in this case would be same as maneuvering = true
[4:37 PM] noah: actions may include [clear, pop, openchest, ... ] etc
[4:38 PM] noah: this also eliminates maneuverTo
[4:44 PM] noah: also not seeing the reasoning behind this: 
Chest.scan(5);
Chest.openChests();
 why separate?
[5:49 PM] Chojun: because I've found that sometimes the two need to be done separately.  And sometimes if a bot is maneuvering it may not want to open chests but simply take note of new ones that are discovered and open them later, there was a lot of considerations
[5:50 PM] Chojun: I actually wanted to change the arguments for moveTo but decided against it because I didn't want to make any breaking changes in case people are operating out of forks
[5:51 PM] Chojun: I wanted to start with an incremental change that modified the pathing behavior and then maybe down the road make changes on how scripts consume it
[5:52 PM] Chojun: ultimately that led me to just change the name to pathTo and add an argument (maneuvering) that just turns off node actions except for attacking
[5:53 PM] Chojun: the name change forces consumers to acknowledge that there is a behavior change and all they have to do is change the name.  But this was just my train of thought on this
[5:55 PM] Chojun: because when pathing there are basically 2 goals - 1) move from point A to point B, and 2) move to a particular position to accomplish a task (like getting into position)
[5:56 PM] Chojun: you don't want your bot to run off and open a chest if its movement goal is to position itself on an arc so it can cast a spell at a monster, for example
[7:10 PM] noah: I don't see any instances where you are using them separately
[7:13 PM] noah: if it's to add APIs that can be used otherwise, that is fine, I would say rename to Attack.getChests(5) - which does not save to a global variables, and then you can have Attack.openChests() that allows you to send in an array of chests otherwise it will find them (ie the current implementation)
[7:13 PM] noah: you can do something like var blah = Attack.getChests(5);  // do other stuff in scripts like kill some mob  Attack.openChests(blah);
[7:36 PM] Chojun: they're used separately in 2 places from what I recall, once in Andariel script and the other time in the chest node action in pather.js
[7:38 PM] Chojun: I removed Attack.openChests() because I wanted separation of concerns.  Pather opening chests and Attack opening chests was causing conflicts with pathing and was contributing to the problems that I was observing
[7:39 PM] Chojun: so I limited chest opening to pathing although the scripts can make calls to open chests if they would like
[11:07 PM] noah: That’s prob a good idea to remove from attack code
[11:09 PM] noah: In that case move it to pather.js

d2bs/kolbot/libs/bots/AncientTunnels.js

d2bs/kolbot/libs/bots/Follower.js

d2bs/kolbot/libs/bots/Mephisto.js

…ransition helper

MikeKGilmore · 2019-04-22T03:11:40Z

Implemented all requested changes

…tes when pathing with clearpath

noah-

Seems okay for the most part
code still needs to be cleaned up
also please make comments consistent like //_ <- make sure there is a space after //
and remove unnecessary newlines

noah- · 2019-05-25T19:56:00Z

d2bs/kolbot/libs/common/Attack.js

 			return true;
 		}

 		var i, j, rval,
 			tempArray = [];

-EnchantLoop: // Skip enchanted monsters
+		EnchantLoop: // Skip enchanted monsters


these should not be indented

noah- · 2019-05-25T19:56:04Z

d2bs/kolbot/libs/common/Attack.js

@@ -1327,7 +1290,7 @@ EnchantLoop: // Skip enchanted monsters
 			}
 		}

-ImmuneLoop: // Skip immune monsters
+		ImmuneLoop: // Skip immune monsters


these should not be indented

noah- · 2019-05-25T19:56:08Z

d2bs/kolbot/libs/common/Attack.js

@@ -1342,7 +1305,7 @@ ImmuneLoop: // Skip immune monsters
 			}
 		}

-AuraLoop: // Skip monsters with auras
+		AuraLoop: // Skip monsters with auras


these should not be indented

noah- · 2019-05-25T19:56:26Z

d2bs/kolbot/libs/common/Pickit.js

@@ -254,8 +254,11 @@ MainLoop:
 			if (me.dead) {
 				return false;
 			}
+
+			//print("Picking item");


can you clean up these comments

noah- · 2019-05-25T19:56:30Z

d2bs/kolbot/libs/common/Pickit.js


 			while (!me.idle) {
+				//print("Pickit:  Waiting");


can you clean up these comments

noah- · 2019-05-25T19:59:00Z