Skip to content

Fix incorrect calculation in get distance for diagonals #78

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.

Sign up for GitHub

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

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

Fix incorrect calculation in get distance for diagonals #78

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4767e3a
Fix incorrect calculation in get distance for diagonals
mikeparker Jan 7, 2020
dacff88
Fix typo in easystar diagonal func
mikeparker Jan 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/easystar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,9 +533,9 @@ EasyStar.js = function() {
var dx = Math.abs(x1 - x2);
var dy = Math.abs(y1 - y2);
if (dx < dy) {
return DIAGONAL_COST * dx + dy;
return DIAGONAL_COST * dx + (dy-dx);
} else {
return DIAGONAL_COST * dy + dx;
return DIAGONAL_COST * dy + (dx=dy);
Copy link
Copy Markdown

@aJamDonut aJamDonut Jan 23, 2024

Choose a reason for hiding this comment

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

@mikeparker sorry i know its old, i implemented this change myself also but wanted to ask... why was it dx=dy

Copy link
Copy Markdown
Author

@mikeparker mikeparker Jan 23, 2024

Choose a reason for hiding this comment

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

Yeah it's pretty old, that looks like a typo to me, it's probably supposed to be (dx-dy)

If I was better with javascript this is exactly the sort of thing a unit test should check for.

Copy link
Copy Markdown
Author

@mikeparker mikeparker Jan 23, 2024

Choose a reason for hiding this comment

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

Yes that makes sense thinking about it. If dx < dy then you travel all the X distance via diagonals then the remainer dy via vertical moves.

If its the other way around then you travel all the Y distance via diagonals then the remainder dx via horizontal moves.

Copy link
Copy Markdown
Author

@mikeparker mikeparker Jan 23, 2024

Choose a reason for hiding this comment

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

I've fixed the typo.

}
} else {
// Manhattan distance
Expand Down