diff --git a/data/core.yaml b/data/core.yaml index 8259e838c..d8c31a5c7 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -1211,6 +1211,10 @@ en: tip: "Crossing bridges should use different layers." bridge-bridge_connectable: tip: "Crossing bridges should be connected or use different layers." + indoor-indoor: + tip: "Crossing indoor features should use different levels." + indoor-indoor_connectable: + tip: "Crossing indoor features should be connected or use different levels." deprecated_tag: single: message: '{feature} has the outdated tag "{tag}"' @@ -1282,6 +1286,14 @@ en: upgrade_tag_combo: title: Upgrade these tags annotation: Upgraded an old tag combination. + use_bridge_or_tunnel: + title: Use a bridge or tunnel + use_different_layers: + title: Use different layers + use_different_levels: + title: Use different levels + use_tunnel: + title: Use a tunnel intro: done: done ok: OK diff --git a/dist/locales/en.json b/dist/locales/en.json index 9f953bce6..ffc435425 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1477,6 +1477,12 @@ }, "bridge-bridge_connectable": { "tip": "Crossing bridges should be connected or use different layers." + }, + "indoor-indoor": { + "tip": "Crossing indoor features should use different levels." + }, + "indoor-indoor_connectable": { + "tip": "Crossing indoor features should be connected or use different levels." } }, "deprecated_tag": { @@ -1579,6 +1585,18 @@ "upgrade_tag_combo": { "title": "Upgrade these tags", "annotation": "Upgraded an old tag combination." + }, + "use_bridge_or_tunnel": { + "title": "Use a bridge or tunnel" + }, + "use_different_layers": { + "title": "Use different layers" + }, + "use_different_levels": { + "title": "Use different levels" + }, + "use_tunnel": { + "title": "Use a tunnel" } } }, diff --git a/modules/validations/crossing_ways.js b/modules/validations/crossing_ways.js index ac669bbf9..cf58ee0d0 100644 --- a/modules/validations/crossing_ways.js +++ b/modules/validations/crossing_ways.js @@ -64,6 +64,23 @@ export function validationCrossingWays() { return tags[key] !== undefined && tags[key] !== 'no'; } + function tagsImplyIndoors(tags) { + return hasTag(tags, 'level') || tags.highway === 'corridor'; + } + + function allowsStructures(featureType) { + return allowsBridge(featureType) || allowsTunnel(featureType); + } + function allowsBridge(featureType) { + return featureType === 'highway' || featureType === 'railway'; + } + function allowsTunnel(featureType) { + return featureType === 'highway' || featureType === 'railway' || featureType === 'waterway'; + } + function canCover(featureType) { + return featureType === 'building'; + } + function getFeatureTypeForCrossingCheck(way, graph) { var tags = getFeatureWithFeatureTypeTagsForWay(way, graph).tags; @@ -106,54 +123,35 @@ export function validationCrossingWays() { tags1 = extendTagsByInferredLayer(tags1, way1); tags2 = extendTagsByInferredLayer(tags2, way2); - // For better readability, not chaining all the true conditions into one if statement. - if ((featureType1 === 'highway' && featureType2 === 'highway') || - (featureType1 === 'highway' && featureType2 === 'railway') || - (featureType1 === 'railway' && featureType2 === 'railway')) { - // Legit cases: - // (1) they're on different layers - // (2) only one of the two ways is on a bridge - // (3) only one of the two ways is in a tunnel - if (tags1.layer !== tags2.layer) return true; + if (tagsImplyIndoors(tags1) && tagsImplyIndoors(tags2) && tags1.level !== tags2.level) { + // assume features don't interact if they're indoor on different levels + return true; + } + + if (allowsBridge(featureType1) && allowsBridge(featureType2)) { if (hasTag(tags1, 'bridge') && !hasTag(tags2, 'bridge')) return true; if (!hasTag(tags1, 'bridge') && hasTag(tags2, 'bridge')) return true; + // crossing bridges must use different layers + if (hasTag(tags1, 'bridge') && hasTag(tags2, 'bridge') && tags1.layer !== tags2.layer) return true; + } else if (allowsBridge(featureType1) && hasTag(tags1, 'bridge')) return true; + else if (allowsBridge(featureType2) && hasTag(tags2, 'bridge')) return true; + + if (allowsTunnel(featureType1) && allowsTunnel(featureType2)) { if (hasTag(tags1, 'tunnel') && !hasTag(tags2, 'tunnel')) return true; if (!hasTag(tags1, 'tunnel') && hasTag(tags2, 'tunnel')) return true; - } - if ((featureType1 === 'highway' && featureType2 === 'waterway') || - (featureType1 === 'railway' && featureType2 === 'waterway')) { - // Legit cases: - // (1) highway/railway is on a bridge - // (2) only one of the two ways is in a tunnel - // (3) both are in tunnels but on different layers - if (hasTag(tags1, 'bridge')) return true; - if (hasTag(tags1, 'tunnel') && !hasTag(tags2, 'tunnel')) return true; - if (!hasTag(tags1, 'tunnel') && hasTag(tags2, 'tunnel')) return true; + // crossing tunnels must use different layers if (hasTag(tags1, 'tunnel') && hasTag(tags2, 'tunnel') && tags1.layer !== tags2.layer) return true; - } - if ((featureType1 === 'highway' && featureType2 === 'building') || - (featureType1 === 'railway' && featureType2 === 'building')) { - // Legit cases: - // (1) highway/railway has a bridge or tunnel tag - // (2) highway/railway has a covered tag - if (hasTag(tags1, 'bridge') || hasTag(tags1, 'tunnel') || hasTag(tags1, 'covered')) return true; - } - if (featureType1 === 'waterway' && featureType2 === 'waterway') { - // Legit cases: - // (1) only one of the water is in a tunnel - // (2) both are in tunnels but on differnt layers - if (hasTag(tags1, 'tunnel') && !hasTag(tags2, 'tunnel')) return true; - if (!hasTag(tags1, 'tunnel') && hasTag(tags2, 'tunnel')) return true; - if (hasTag(tags1, 'tunnel') && hasTag(tags2, 'tunnel') && tags1.layer !== tags2.layer) return true; - } - if (featureType1 === 'waterway' && featureType2 === 'building') { - // Legit cases: - // (1) water is in a tunnel - // (2) water has a covered tag - if (hasTag(tags1, 'tunnel') || hasTag(tags1, 'covered')) return true; - } - if (featureType1 === 'building' && featureType2 === 'building') { - // Legit case: they're on different layers + } else if (allowsTunnel(featureType1) && hasTag(tags1, 'tunnel')) return true; + else if (allowsTunnel(featureType2) && hasTag(tags2, 'tunnel')) return true; + + if (canCover(featureType1) && canCover(featureType2)) { + // crossing covered features that can themselves cover must use different layers + if (hasTag(tags1, 'covered') && hasTag(tags2, 'covered') && tags1.layer !== tags2.layer) return true; + } else if (canCover(featureType1) && hasTag(tags2, 'covered')) return true; + else if (canCover(featureType2) && hasTag(tags1, 'covered')) return true; + + if (!allowsStructures(featureType1) && !allowsStructures(featureType2)) { + // if no structures are applicable, the layers must be different if (tags1.layer !== tags2.layer) return true; } return false; @@ -166,7 +164,7 @@ export function validationCrossingWays() { 'primary', 'primary_link', 'secondary', 'secondary_link' ]; var pathHighways = [ - 'path', 'footway', 'cycleway', 'bridleway', 'pedestrian', 'steps' + 'path', 'footway', 'cycleway', 'bridleway', 'pedestrian', 'steps', 'corridor' ]; function tagsForConnectionNodeIfAllowed(entity1, entity2) { @@ -353,20 +351,29 @@ export function validationCrossingWays() { var connectionTags = tagsForConnectionNodeIfAllowed(entities[0], entities[1]); + var featureType1 = crossing.featureTypes[0]; + var featureType2 = crossing.featureTypes[1]; + + var isCrossingIndoors = tagsImplyIndoors(entities[0].tags) && tagsImplyIndoors(entities[1].tags); + var isCrossingTunnels = allowsTunnel(featureType1) && hasTag(entities[0].tags, 'tunnel') && + allowsTunnel(featureType2) && hasTag(entities[1].tags, 'tunnel'); + var isCrossingBridges = allowsBridge(featureType1) && hasTag(entities[0].tags, 'bridge') && + allowsBridge(featureType2) && hasTag(entities[1].tags, 'bridge'); + var crossingTypeID; - if (hasTag(entities[0].tags, 'tunnel') && hasTag(entities[1].tags, 'tunnel')) { + + if (isCrossingIndoors) { + crossingTypeID = 'indoor-indoor'; + } else if (isCrossingTunnels) { crossingTypeID = 'tunnel-tunnel'; - if (connectionTags) { - crossingTypeID += '_connectable'; - } - } else if (hasTag(entities[0].tags, 'bridge') && hasTag(entities[1].tags, 'bridge')) { + } else if (isCrossingBridges) { crossingTypeID = 'bridge-bridge'; - if (connectionTags) { - crossingTypeID += '_connectable'; - } } else { crossingTypeID = crossing.featureTypes.sort().join('-'); } + if (connectionTags && (isCrossingIndoors || isCrossingTunnels || isCrossingBridges)) { + crossingTypeID += '_connectable'; + } var messageDict = { feature: utilDisplayLabel(entities[0], context), @@ -415,6 +422,21 @@ export function validationCrossingWays() { } })); } + var useFixID; + if (isCrossingIndoors) { + useFixID = 'use_different_levels'; + } else if (isCrossingTunnels || isCrossingBridges) { + useFixID = 'use_different_layers'; + } else if (allowsBridge(featureType1) || allowsBridge(featureType2)) { + useFixID = 'use_bridge_or_tunnel'; + } else if (allowsTunnel(featureType1) || allowsTunnel(featureType2)) { + useFixID = 'use_tunnel'; + } else { + useFixID = 'use_different_layers'; + } + fixes.push(new validationIssueFix({ + title: t('issues.fix.' + useFixID + '.title') + })); fixes.push(new validationIssueFix({ title: t('issues.fix.reposition_features.title') })); diff --git a/test/spec/validations/crossing_ways.js b/test/spec/validations/crossing_ways.js index 81b0e631c..707e8e7f2 100644 --- a/test/spec/validations/crossing_ways.js +++ b/test/spec/validations/crossing_ways.js @@ -105,7 +105,13 @@ describe('iD.validations.crossing_ways', function () { }); it('legit crossing between railway and railway', function() { - createWaysWithOneCrossingPoint({ railway: 'rail', layer: '1' }, { railway: 'rail' }); + createWaysWithOneCrossingPoint({ railway: 'rail', bridge: 'yes' }, { railway: 'rail' }); + var issues = validate(); + expect(issues).to.have.lengthOf(0); + }); + + it('legit crossing between railway bridges on different layers', function() { + createWaysWithOneCrossingPoint({ railway: 'rail', bridge: 'yes', layer: '2' }, { railway: 'rail', bridge: 'yes' }); var issues = validate(); expect(issues).to.have.lengthOf(0); }); @@ -140,6 +146,12 @@ describe('iD.validations.crossing_ways', function () { expect(issues).to.have.lengthOf(0); }); + it('legit crossing between indoor corridors on different levels', function() { + createWaysWithOneCrossingPoint({ highway: 'corridor', level: '0' }, { highway: 'corridor', level: '1' }); + var issues = validate(); + expect(issues).to.have.lengthOf(0); + }); + // warning crossing cases between ways it('one cross point between highway and highway', function() { createWaysWithOneCrossingPoint({ highway: 'residential' }, { highway: 'residential' }); @@ -171,13 +183,23 @@ describe('iD.validations.crossing_ways', function () { verifySingleCrossingIssue(validate(), 'w-2'); }); + it('one cross point between railway bridge and highway bridge', function() { + createWaysWithOneCrossingPoint({ highway: 'residential', bridge: 'yes' }, { railway: 'rail', bridge: 'yes' }); + verifySingleCrossingIssue(validate(), 'w-2'); + }); + it('one cross point between railway and building', function() { createWaysWithOneCrossingPoint({ railway: 'rail' }, { building: 'yes' }); verifySingleCrossingIssue(validate(), 'w-2'); }); it('one cross point between waterway and waterway', function() { - createWaysWithOneCrossingPoint({ waterway: 'canal' }, { waterway: 'river' }); + createWaysWithOneCrossingPoint({ waterway: 'canal' }, { waterway: 'canal' }); + verifySingleCrossingIssue(validate(), 'w-2'); + }); + + it('one cross point between waterway tunnels', function() { + createWaysWithOneCrossingPoint({ waterway: 'canal', tunnel: 'yes' }, { waterway: 'canal', tunnel: 'yes' }); verifySingleCrossingIssue(validate(), 'w-2'); }); @@ -191,6 +213,11 @@ describe('iD.validations.crossing_ways', function () { verifySingleCrossingIssue(validate(), 'w-2'); }); + it('one cross point between indoor corridors on the same level', function() { + createWaysWithOneCrossingPoint({ highway: 'corridor', level: 0 }, { highway: 'corridor', level: 0 }); + verifySingleCrossingIssue(validate(), 'w-2'); + }); + it('two cross points between two highways', function() { createWaysWithTwoCrossingPoint(); var issues = validate();