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

Astar overrides cost of tiles next to the starting point #371

Closed
eeetem opened this issue Dec 6, 2022 · 4 comments
Closed

Astar overrides cost of tiles next to the starting point #371

eeetem opened this issue Dec 6, 2022 · 4 comments

Comments

@eeetem
Copy link

eeetem commented Dec 6, 2022

Describe the bug
When setting the initial values of tiles next to the starting point they're not assigned an open state which often leads to them being overriden other nearby tiles.

This is the code that initiates the costs at the starting point

// Add connecting nodes if traversable
                if (node.Traversable)
                {
                    // Calculate the Costs
                    node.CurrentCost = from.CurrentCost + from.DistanceTo(node) * node.TraversalCostMultiplier;
                    node.EstimatedCost = from.CurrentCost + node.DistanceTo(to);
                    // Enqueue
                    open.Enqueue(node);
                }

And this is the code that iteratively sets the costs

  // Adds a previously not "seen" node into the Queue
                if (connected.State == NodeState.Unconsidered)
                {
                    connected.Parent = current;
                    connected.CurrentCost =
                        current.CurrentCost + current.DistanceTo(connected) * connected.TraversalCostMultiplier;
                    connected.EstimatedCost = connected.CurrentCost + connected.DistanceTo(to);
                    connected.State = NodeState.Open;
                    queue.Enqueue(connected);
                }
                else if (current != connected)
                {
                    // Updating the cost of the node if the current way is cheaper than the previous
                    var newCCost = current.CurrentCost + current.DistanceTo(connected);
                    var newTCost = newCCost + current.EstimatedCost;
                    if (newTCost < connected.TotalCost)
                    {
                        connected.Parent = current;
                        connected.CurrentCost = newCCost;
                    }
                }

The solution is a one line fix, altough it also might make more sense to move both of those code snipets into a sningle function

node.State = NodeState.Open;
i was considering doing a pull request however i do not have the time to write tests for this to fit in wth the guidelnes(since astar does not have any to begin with)

To Reproduce
There's no tests so it's rather difficult as i've only discovered it myself when using the code as part of a bigger project

Expected behavior
The iterative code that sets the costs does NOT override costs of tiles if the cost is higher - which it does as seen in the snipped above UNLESS the tile is unconsidered at which point it assumes the cost is 0 and overrides the cost without checking. The issue here is that all initial tiles start out as unconsidered rather than as open.

Actual behavior
In scenarios where one of the corner nodes is evaluated first
image

@eeetem eeetem added the bug label Dec 6, 2022
@siriak
Copy link
Member

siriak commented Dec 26, 2022

Thanks for reporting this, could you open a PR with tests to highlight the issue and a fix? Thanks

@eeetem
Copy link
Author

eeetem commented Dec 27, 2022

aye that's what i wanted to do originally but currently writting tests for this is a bit out of the scope for my schedule, so hopefully someone else will take over on the issue

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jan 29, 2023
@github-actions
Copy link

github-actions bot commented Feb 5, 2023

This issue was closed because it has been stalled for 7 days with no activity.

@github-actions github-actions bot closed this as completed Feb 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants