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

Fix #7062: Remove ship max order distance. #7279

Merged
merged 3 commits into from Mar 31, 2019
Merged

Conversation

PeterN
Copy link
Member

@PeterN PeterN commented Feb 25, 2019

It is skipped when NPF is in use.
It is trivial to work around by adding and removing dummy orders.
It is mostly alleviated by the ship path cache in YAPF.

It is skipped when NPF is in use.
It is trivial to work around by adding and removing dummy orders.
It is mostly alleviated by the ship path cache in YAPF.
@PeterN
Copy link
Member Author

PeterN commented Feb 25, 2019

#7272 and #7245 would be useful for this.

@PeterN PeterN marked this pull request as ready for review March 3, 2019 13:58
@PeterN
Copy link
Member Author

PeterN commented Mar 3, 2019

Ideally buoys should still be used but with this change there's no reason, from a player POV, to bother. So I'm not sure.

@nielsmh nielsmh added this to the 1.10.0 milestone Mar 3, 2019
@nielsmh
Copy link
Contributor

nielsmh commented Mar 3, 2019

Putting this on 1.10 since it pretty much requires #7245 to be sensible. I believe one of the major reasons for the max order distance is the original pathfinder.

@PeterN
Copy link
Member Author

PeterN commented Mar 7, 2019

Perhaps I should split this into two PRs, one to revert the earlier change, and then the second to remove the distance checks?

Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

It's all the same set of changes, I don't see any need to split it any further than per-commit

@michicc
Copy link
Member

michicc commented Mar 24, 2019

#7272 would still make sense, but @PeterN had some outstanding comments on that.

@PeterN
Copy link
Member Author

PeterN commented Mar 24, 2019

I was thinking the first commit may be a backport candidate.

@PeterN PeterN merged commit f0336f1 into OpenTTD:master Mar 31, 2019
@PeterN PeterN deleted the fix-7062 branch March 31, 2019 16:39
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.

None yet

4 participants