-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Let harvesters only search for refineries when needing to unload. #18586
Conversation
if (!searchFromLoc.HasValue) | ||
{ | ||
searchFromLoc = self.Location; | ||
searchRadius = harvInfo.SearchFromHarvesterRadius; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this mean we use SearchFromProcRadius
in case we use self.Location
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is the idea. Without this, the range for the initial search is too short, causing harvesters in the shell maps to stall.
Apart from the initial search, Self.Location
is only really used for the center of the search as a last resort.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sigh, guess I need to finally revisit #18198 to get rid of this mess. (Or at least reduce it.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can take that over if you like. After all, I did the the big harvester refactor last time and some issues where deliberately left hanging then. I would be happy to finish what I started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to go for it. I'm just no longer sure if it should search from the refinery first. Probably it still should, but return to the refinery to unload first before doing more scanning in case no ore is in the refinery radius.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already have some ideas on how to deal with this. I will probably split things in multiple PRs and take on some related dangling issues with harvester behaviour while I'm at it, so that we can close this topic once and for all. I would like to get this PR merged first though before I go further, because it is all closely related and it makes more sense to handle this in a linear fashion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want the has dependencies labels on this PR then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works as promised and the code changes look convincing.
Fixes #18563
Queueing a callfunc on creation was unneeded, since
DeliverResources
will take care of linking with a new refinery when needed anyway.