fixed issue Merging two ways corrupts step_count #10492 #10633
Conversation
|
to anyone who's reading this , if i made any mistakes in raising this pr feel free to criticize it's my first time raising a pr in other repo besides mine 🙃 |
Darshit42
changed the title
modules/osm/entity.js
Outdated
| utilArrayUnion(t1.split(/;\s*/), t2.split(/;\s*/)).join(';'), | ||
| 255 // avoid exceeding character limit; see also context.maxCharsForTagValue() | ||
| ); | ||
| merged[k2] = `${t1};${t2}`; |
There was a problem hiding this comment.
The previous code intentionally eliminated duplicates from the list. Merging two features with identical values for a given key shouldn’t result in the value being repeated twice. Tests are failing because this code unconditionally concatenates the two values.
There was a problem hiding this comment.
ill change it and fix the repeated value error
modules/osm/entity.js
Outdated
| merged[k2] = t2; | ||
| } else if (!isNaN(t1) && !isNaN(t2)) { | ||
| changed = true; | ||
| merged[k2] = String(Number(t1) + Number(t2)); |
There was a problem hiding this comment.
While it may be appropriate to treat step_count=* as a summable number, this is not necessarily the case with every tag value that consists of digits. For example, if I merge two ways with maxspeed=30 and maxspeed=35, iD should reject the merge because of conflicting tags rather than resulting in maxspeed=65 (a speed limit of 65 kilometers per hour).
There was a problem hiding this comment.
ill look into these tags and make a list to exclude them from being adding up , this might fix the issue, if there's anything im missing please let me know @1ec5
There was a problem hiding this comment.
step_count=* is probably an outlier as a summable key. Some other keys like hoops=* (for the number of hoops on a basketball court) and capacity=* might be summable in some cases but not others, so I don’t think it would be safe to assume we can sum them.
this pr fixes #10492 ,changes are such that it adds up the values of tags
before fixing the issue tags such as step_count , parking:left, etc values were not adding up when lines were merged, this creates confusion to user on how merging works
merging two ways resulted in 3+4 => Error (values are not same) or 2+2 => 2 (values are not summed)
after this , when lines are merged it adds up the values of tags successfully
merging two ways resulted in 3+4 => 7
Screen-Recording.mp4