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

MFHelper refactoring. #1188

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ryancrunchi
Copy link
Contributor

@ryancrunchi ryancrunchi commented Feb 5, 2019

Refactored MFHelper to avoid usePortal failures and delays for helper to join leader.
Refactored common duplicate code before and after commands.

// if, at worst case, the portal leads to wrong area, the command will not be executed and helper will go back to town at next loop.
delay(me.ping);
Pather.usePortal(null, player.name)
while (!me.area) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is not needed, usePortal already checks for area change, use the retry method as in the original version
also there is no reason not to check the area since the info is available

Copy link
Contributor Author

@ryancrunchi ryancrunchi Feb 6, 2019

Choose a reason for hiding this comment

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

usePortal creates delay if we pass in the leader area, this makes the helper wait some seconds not taking the tp, which may be too late when the leader area is refreshed in the next loop pass. This is because leader area returns the previous one, not the current one where the portal has just been made due to refresh delay. This is the main fix of this PR.

but you are right for the while check, this is already done in usePortal

Copy link
Collaborator

Choose a reason for hiding this comment

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

investigate the source of the delay in usePortal, area check should not cause any significant delay

Copy link
Contributor Author

@ryancrunchi ryancrunchi Feb 6, 2019

Choose a reason for hiding this comment

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

That is not usPortal that makes delay. Not explicitly at least.
This is player.area which returns the previous area when tp appears.
For example, leader runs pit.
Teleport to pit level 1, area = x-1.
Teleport to pit level 2, area = x.
Make tp, send command.
Helper receive command, check leader’s area. Here is the problem.
Leader’s area returns x-1, not refreshed.
usePortal(x-1, leader) will delay failing retrying to use portal because there is no portal to x-1, but x. (for loop of 10*(200+me.ping) between 2 and 4 seconds delay, not so important for area clearing, but for boss killing 4 seconds is too late)
Wait for the next helper loop pass where leader’s area now correctly returns x.

Copy link
Contributor Author

@ryancrunchi ryancrunchi Feb 6, 2019

Choose a reason for hiding this comment

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

By using usePortal(null, leader) we don’t check failable area, and works better because the command is sent just after making a tp, so the tp is obviously the right area. And we check area if (me.area == leader.area) after taking portal so leader.area has time to be refreshed, while taking tp.

print("Failed to use portal.");
// common for all commands, go back to town after executing command.
delay(1000);
if (!Pather.usePortal(null, player.name)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

add new line before this line

if (Town.goToTown(1) && Pather.usePortal(39)) {
break;
if (me.area != 39) {
Town.goToTown(1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

retain the retry structure, there is no guarantee that the cow portal will be near stash so this check doesn't make much sense

also this part of the code won't work properly because you would have taken the players tp, the area check would avoid this redundancy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I forget to move cows outside
the else if cows would be for the if (me.area === player.area)
if we failed to take leader tp (baal not done can't use player tp to cows), try to take the red portal as before

// Go to town if I'm not fighting with leader anymore.
// This may happens with kurast temples script for example, where leader changes temples without going to town.
if (player.area != me.area) {
if (!me.inTown && !Pather.usePortal(null, player.name)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

combine these if statements

}
Town.move("portalspot");
Copy link
Collaborator

Choose a reason for hiding this comment

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

add newline above this

@DarkHorseDre
Copy link

Guys I've been working through issues with mfhelper and thought it 'may' help if I share my observations - assuming anyone wants to do more work on this (,") - I realise from what I've read here that much of this has been catered for, but just in case..

I run a paladin and barb as classic hc mfhelpers with a sorc leader and, interestingly, the barb and paladin do not always mirror one another.
example: barb will return to town or not enter tp whereas paladin will. This is one per leader script and not permanent, and I have no idea why.

Many area conflicts:
The helpers enter the tp then check leader areaid. I've started logging these within the many if/else's to capture when the leader is not where expected. This happens often and the most common reasons I see are:

  1. leader returns to shop pots
  2. leader returns to heal
  3. leader has already killed the target and moved on
  4. target undetected (but is there - endugu is a good example)
  5. other reasons I have not captured yet

Mfehelper is set to use town.gototown if it cannot find leader/other issue, but this function throws an error which calls an exit. Clearly the throw new error is unhelpful for such a common error. I think the mfhelper needs to avoid exits and instead be told the reason for bot being absent and either wait or return to town (via a makeportal) and continue to loop. (I've switched all town.gototown for make/use portals to reduce the many abandoned games I have). or reevaluate the reasons for the throw new error in town.js, but I see that as out of scope here.

If the leader could send new command like "pause command" where the mfhelper would return to town, and then "unpause command" where the leader returns to tp then opens new tp and the follower jumps through and executes the last command (or incase of recent rejoin, the leader could just issue the same command but with the pause/unpause keyword).
An abandon keyword would be good for when the attack failed/mob not located/other error so the follower doesn't need to enter/can return to town cleanly.

Precast in town:
I've seen the bot try to precast in area 1 and 40 (and other towns I think). I'm not sure why but I saw a barb cast out of town then step through tp mid-cast and try to cast the last (shout) in town..

mfhlpers follow bot to acts when it switches to buy pots - I/we modified getPlayerAct to look at a range which excludes town area ids, so the helpers only change acts when the leader leaves town. this of course is not clean as they walk to wp later and can lag behind (especially act 2 to 3).
A new command would help - the leader could workout if it needs to change act for the next script then tell the followers, but not if it is changing acts for pots/gambling/other and will return to the initial act.
e.g.
if (player.area > 1 && player.area <= 39) { // changed from area > 0 && area <= 39 - avoids rogue camp return 1; }

@Kephren999
Copy link

Really want to thank Ryancrunchi and anyone else who worked on this much improved script, it runs so smoothly, bots no longer sit around waiting to enter portals.
one thing I did notice, in the script, the leader is fine for making and entering cows, but since the followers haven't quest killed baal yet, they get stuck trying to enter the leaders portal, in the original script they just used the red portal to enter with no problems.
no biggie, ill just go back and quest kill baal with each, just wanted to let you know.

awesome job guys love this script now it's blazing fast.

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.

4 participants