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

Splitting a 'to' member of destination_sign relations adds an extra 'to' member #9188

Open
fidelis-assis opened this issue Jun 28, 2022 · 5 comments · May be fixed by #10646
Open

Splitting a 'to' member of destination_sign relations adds an extra 'to' member #9188

fidelis-assis opened this issue Jun 28, 2022 · 5 comments · May be fixed by #10646

Comments

@fidelis-assis
Copy link

URL

No response

How to reproduce the issue?

Just split a way that is a 'to' member of any destination_sign relation.
destination_sign relations can have exactly one 'to' member.
Splitting a 'from' member also adds extra 'from', not sure if that is a problem too.

Screenshot(s) or anything else?

No response

Which iD Editor versions do you see the issue on?

Released version at openstreetmap.org/edit

Which browsers are you seeing this problem on?

Chrome

@1ec5
Copy link
Collaborator

1ec5 commented Jun 28, 2022

At the time of the split, did the relation already have from, via, and to members, or just a to member? It looks like the code for splitting only tidies up relations that already have the full complement of members:

iD/modules/osm/relation.js

Lines 259 to 265 in 9a26ae3

hasFromViaTo: function() {
return (
this.members.some(function(m) { return m.role === 'from'; }) &&
this.members.some(function(m) { return m.role === 'via'; }) &&
this.members.some(function(m) { return m.role === 'to'; })
);
},

iD/modules/actions/split.js

Lines 176 to 179 in c854564

if (relation.hasFromViaTo()) {
var f = relation.memberByRole('from');
var v = relation.membersByRole('via');
var t = relation.memberByRole('to');

This documentation points out that it’s valid for there to be no from member, which would be unlike any other relation type that uses from/via/to roles. I disagree with the note on that page about “via” nodes being nonsensical – they would make sense for the same reason as “via” nodes on any other kind of relation, such as restriction relations. This issue has been raised on the talk page as well.

@fidelis-assis
Copy link
Author

Yes, the problem occurs with complete relations. I've just created a complete one to show the issue, relation 14305516, and then split the to member. The attached print shows the problem.
I also disagree with that note. I think that 'intersection' nodes are important/necessary. 'via' ways are not part of the original proposal, but used "informally". By the way, it'd be nice if iD implemented some validation of these relations.

extra_to_after_splitting

@1ec5
Copy link
Collaborator

1ec5 commented Jun 29, 2022

OK, so the issue is that the role is intersection instead of via, which causes iD to not recognize the relation as being the sort of relation that has turn restriction–like roles. It’s quite unfortunate that the proposal co-opted from and to roles but with different rules for the from role and a novel via-like intersection role. We could work around that design problem by equating via and intersection in the hasFromViaTo() method above, but it would be prudent to limit this special case to destination_sign relations. I also wonder if we really should place such importance on the relation type’s original design, since the proposal was rejected.

@fidelis-assis
Copy link
Author

OK, limiting equating "intersection" and "via" to destinmation_sign relations seems fine. I also agree with not placing much importance to that proposal (rejected but widely used, ex. 31500+ in Germany, 5500+ in USA, 4700+ in France, 1400+ in UK, 850+ in Brazil). Anyway, it'd be interesting to have a minimum validation such as requiring exactly one "to" member.
I personally dislike optional "intersection" and "from" but that's the de facto usage.

@fidelis-assis
Copy link
Author

Hello, what's the status of this issue? I mean, avoid inclusion of multiples 'from' and 'to' to the destination_sign relation when a way ('from', 'to' or 'via') is splitted, so as not to break/change the relation.

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 a pull request may close this issue.

2 participants