Fix split way action, add decent tests for them (fix #171), enable split road on cyclestreets theme

Logic/FeatureSource/TiledFeatureSource/DynamicGeoJsonTileSource.ts

@@ -48,6 +48,7 @@ export default class DynamicGeoJsonTileSource extends DynamicTileSource {
                 if(whitelist !== undefined){
                     const isWhiteListed = whitelist.get(zxy[1])?.has(zxy[2])
                     if(!isWhiteListed){
+                        console.log("Not downloading tile", ...zxy, "as it is not on the whitelist")
                         return undefined;
                     }
                 }

Logic/FeatureSource/TiledFeatureSource/OsmFeatureSource.ts

@@ -109,6 +109,8 @@ export default class OsmFeatureSource {
 
                 geojson.features = geojson.features.filter(feature => this.allowedTags.matchesProperties(feature.properties))
 
+                geojson.features.forEach(f => f.properties["_backend"] = this._backend)
+                
                 console.log("Tile geojson:", z, x, y, "is", geojson)
                 const index = Tiles.tile_index(z, x, y);
                 new PerLayerFeatureSourceSplitter(this.filteredLayers,

Logic/GeoOperations.ts

@@ -230,7 +230,7 @@ export class GeoOperations {
      *  The properties object will contain three values:
      // - `index`: closest point was found on nth line part,
      // - `dist`: distance between pt and the closest point (in kilometer),
-     // `location`: distance along the line between start and the closest point.
+     // `location`: distance along the line between start (of the line) and the closest point.
      * @param way The road on which you want to find a point
      * @param point Point defined as [lon, lat]
      */

Logic/Osm/Actions/SplitAction.ts

@@ -15,17 +15,21 @@ export default class SplitAction extends OsmChangeAction {
     private readonly wayId: string;
     private readonly _splitPointsCoordinates: [number, number] []// lon, lat
     private _meta: { theme: string, changeType: "split" };
+    private _toleranceInMeters: number;
 
     /**
-     * 
+     * Create a changedescription for splitting a point.
+     * Will attempt to reuse existing points
      * @param wayId
      * @param splitPointCoordinates: lon, lat
      * @param meta
+     * @param toleranceInMeters: if a splitpoint closer then this amount of meters to an existing point, the existing point will be used to split the line instead of a new point
      */
-    constructor(wayId: string, splitPointCoordinates: [number, number][], meta: {theme: string}) {
+    constructor(wayId: string, splitPointCoordinates: [number, number][], meta: {theme: string},  toleranceInMeters = 5) {
         super()
         this.wayId = wayId;
         this._splitPointsCoordinates = splitPointCoordinates
+        this._toleranceInMeters = toleranceInMeters;
         this._meta = {...meta, changeType: "split"};
     }
 
@@ -51,7 +55,7 @@ export default class SplitAction extends OsmChangeAction {
         const originalNodes = originalElement.nodes;
 
         // First, calculate splitpoints and remove points close to one another
-        const splitInfo = this.CalculateSplitCoordinates(originalElement)
+        const splitInfo = this.CalculateSplitCoordinates(originalElement, this._toleranceInMeters)
         // Now we have a list with e.g. 
         // [ { originalIndex: 0}, {originalIndex: 1, doSplit: true}, {originalIndex: 2}, {originalIndex: undefined, doSplit: true}, {originalIndex: 3}]
 
@@ -230,17 +234,19 @@ export default class SplitAction extends OsmChangeAction {
                 // We keep the original points
                 continue
             }
-            if (point.dist * 1000 >= toleranceInM) {
+         
-                // No need to remove this one
-                continue
-            }
-
             // At this point, 'dist' told us the point is pretty close to an already existing point.
             // Lets see which (already existing) point is closer and mark it as splitpoint
             const nextPoint = allPoints[i + 1]
             const prevPoint = allPoints[i - 1]
             const distToNext = nextPoint.location - point.location
-            const distToPrev = prevPoint.location - point.location
+            const distToPrev = point.location - prevPoint.location
+            
+            if(distToNext * 1000 > toleranceInM && distToPrev * 1000 > toleranceInM){
+                // Both are too far away to mark them as the split point
+                continue;
+            }
+            
             let closest = nextPoint
             if (distToNext > distToPrev) {
                 closest = prevPoint

Logic/SimpleMetaTagger.ts

@@ -25,7 +25,8 @@ export default class SimpleMetaTagger {
                 "_last_edit:contributor:uid",
                 "_last_edit:changeset",
                 "_last_edit:timestamp",
-                "_version_number"],
+                "_version_number",
+            "_backend"],
             doc: "Information about the last edit of this object."
         },
         (feature) => {/*Note: also called by 'UpdateTagsFromOsmAPI'*/

Models/ThemeConfig/LayoutConfig.ts

@@ -90,14 +90,8 @@ export default class LayoutConfig {
         this.startZoom = json.startZoom;
         this.startLat = json.startLat;
         this.startLon = json.startLon;
-        if(json.widenFactor < 0.02){
+        if(json.widenFactor <= 0){
-            if(official){
+                throw "Widenfactor too small, shoud be > 0"
-                throw "Widenfactor too small"
-            }else{
-                // Unofficial themes get away with this
-                console.warn("Detected a very small widenfactor for theme ", this.id ,", bumping this above 1.")
-                json.widenFactor = json.widenFactor + 1
-            }
         }
         if(json.widenFactor > 20){
             throw "Widenfactor is very big, use a value between 1 and 5 (current value is "+json.widenFactor+") at "+context

UI/BigComponents/MoreScreen.ts

@@ -52,23 +52,19 @@ export default class MoreScreen extends Combine {
 
     private static createUnofficialThemeList(buttonClass: string, state: UserRelatedState, themeListClasses): BaseUIElement {
         return new VariableUiElement(state.installedThemes.map(customThemes => {
-            const els: BaseUIElement[] = []
+            if (customThemes.length <= 0) {
-            if (customThemes.length > 0) {
+                return undefined;
-                const customThemesElement = new Combine(
-                    customThemes.map(theme => MoreScreen.createLinkButton(state, theme.layout, theme.definition)?.SetClass(buttonClass))
-                )
-                els.push(customThemesElement)
             }
+            const customThemeButtons = customThemes.map(theme => MoreScreen.createLinkButton(state, theme.layout, theme.definition)?.SetClass(buttonClass))
             return new Combine([
                 Translations.t.general.customThemeIntro.Clone(),
-                new Combine(els).SetClass(themeListClasses)
+                new Combine(customThemeButtons).SetClass(themeListClasses)
             ]);
         }));
     }
 
     private static createPreviouslyVistedHiddenList(state: UserRelatedState, buttonClass: string, themeListStyle: string) {
         const t = Translations.t.general.morescreen
-        console.log("Hidden themes init...")
         const prefix = "mapcomplete-hidden-theme-"
         const hiddenTotal = AllKnownLayouts.layoutsList.filter(layout => layout.hideFromOverview).length
         return new Toggle(

UI/Popup/SplitRoadWizard.ts

@@ -58,10 +58,10 @@ export default class SplitRoadWizard extends Toggle {
         miniMap.SetStyle("width: 100%; height: 24rem")
             .SetClass("rounded-xl overflow-hidden");
 
-        miniMap.installBounds(BBox.get(roadElement))
+        miniMap.installBounds(BBox.get(roadElement).pad(0.25), false)
 
         // Define how a cut is displayed on the map
-
+        
         // Datalayer displaying the road and the cut points (if any)
         new ShowDataLayer({
             features: new StaticFeatureSource(splitPoints, true),

assets/themes/cyclestreets/cyclestreets.json

@@ -284,6 +284,6 @@
     }
   ],
   "overrideAll": {
-    "allowSplit": false
+    "allowSplit": true
   }
 }

assets/themes/speelplekken/speelplekken.json

@@ -256,7 +256,7 @@
     }
   ],
   "clustering": {
-    "maxZoom": 16,
+    "maxZoom": 6,
     "minNeededElements": 100
   },
   "overrideAll": {

test/TestHelper.ts

@@ -13,19 +13,19 @@ export default class T {
      * Returns an empty list if successful
      * @constructor
      */
-    public Run() : ({testsuite: string, name: string, msg: string} []) {
+    public Run(): ({ testsuite: string, name: string, msg: string } []) {
-        const failures: {testsuite: string, name: string, msg: string} []  = []
+        const failures: { testsuite: string, name: string, msg: string } [] = []
         for (const [name, test] of this._tests) {
             try {
                 test();
             } catch (e) {
-                failures.push({testsuite: this.name, name: name, msg: ""+e});
+                failures.push({testsuite: this.name, name: name, msg: "" + e});
             }
         }
         if (failures.length == 0) {
             return undefined
         } else {
-           return failures
+            return failures
         }
     }
 
@@ -40,12 +40,12 @@ export default class T {
             throw "Expected true, but got false: " + msg
         }
     }
-    
+
-    static equals(a, b, msg?){
+    static equals(a, b, msg?) {
-        if(a !== b){
+        if (a !== b) {
-            throw "Not the same: "+(msg??"")+"\n" +
+            throw "Not the same: " + (msg ?? "") + "\n" +
-            "Expcected: "+a+"\n" +
+            "Expcected: " + a + "\n" +
-            "Got      : "+b
+            "Got      : " + b
         }
     }
 
@@ -54,4 +54,21 @@ export default class T {
             throw "Expected false, but got true: " + msg
         }
     }
+
+    static listIdentical<T>(expected: T[], actual: T[]): void {
+        if(expected === undefined){
+            throw "ListIdentical failed: expected list is undefined"
+        }
+        if(actual === undefined){
+            throw "ListIdentical failed: actual list is undefined"
+        }
+        if (expected.length !== actual.length) {
+            throw `ListIdentical failed: expected a list of length ${expected.length} but got a list of length ${actual.length}`
+        }
+        for (let i = 0; i < expected.length; i++) {
+            if (expected[i] !== actual[i]) {
+            throw `ListIdentical failed at index ${i}: expected ${expected[i]} but got ${actual[i]}`
+            }
+        }
+    }
 }