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 #8750 - Follow operation should prefer the focused way #8978

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

Pho3b
Copy link

@Pho3b Pho3b commented Feb 10, 2022

After studying the ID software architecture for a bit i came up with this solution for #8750.

I performed various test on my local machine and it seems to cover the cases described in the issue.
Unit tests were started, and no errors were found.

Hope it can be helpful, i will be available for any further updates.

@Pho3b Pho3b marked this pull request as ready for review February 10, 2022 22:54
@Pho3b Pho3b changed the title FIX: issue-8750 fix #8705 - Follow operation should prefer the focused way Feb 10, 2022
@tordans
Copy link
Collaborator

tordans commented Feb 11, 2022

Ping #8750

@tyrasd tyrasd added the enhancement An enhancement to make an existing feature even better label Feb 15, 2022
@tyrasd tyrasd changed the title fix #8705 - Follow operation should prefer the focused way fix #8750 - Follow operation should prefer the focused way Feb 15, 2022
Copy link
Member

@tyrasd tyrasd left a comment

Choose a reason for hiding this comment

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

Hey. Thanks for the pull request.

While testing it, I noticed some strange behavior: when following another way which ends in a T-junction, the appended way jumps back to the first contact with the followed way and repeats in a circle infinitely. I think this is a bug, isn't it?

Can you please explain in more detail how you think the follow operation should work in situations like this (and in general). Thanks!

@Pho3b
Copy link
Author

Pho3b commented Feb 17, 2022

hey @tyrasd thank you very much for the code review.

I executed some test and, correct me if i am wrong, it seems to me that i could reproduce the behavior you reported even without my code changes.
What i noticed is that now, not stopping when a node that has multiple parent is encountered, lets that behavior to show up more often.

That said, i just pushed a new commit that ends the following action if a node that has already been followed is encountered again, this should avoid the user to end up following infinite circles.

I hope i correctly answered your question, waiting for your feedback.
Thanks!

@Pho3b
Copy link
Author

Pho3b commented May 15, 2022

Hey guys, any news on this ?
If there's there something i can do to simplify or speed up the review process just let me know

Thanks

@tyrasd
Copy link
Member

tyrasd commented May 20, 2022

Sorry, I haven't had time to give it a deeper look yet. I'll try to come around it next week. :)

@Pho3b
Copy link
Author

Pho3b commented Oct 22, 2022

Hi, any updates regarding this PR ?

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement to make an existing feature even better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants