Continuing core refactor, particularly for EditSystem

(followup from ca9be60, also see #961)

Continuing to remove editing and localization operations from Context.
The idea here is to remove "magic" from the code.
These functions were just redirects from the Context to the EditSystem,
so using them obscured what was really going on.  It was easy to get
confused about "which graph" the code was really using.

- Removed `context.graph`, `.entity`, `.hasEntity`
  Code must go through the EditSystem to do these things now.

- Convert several EditSystem accessor methods to ES6 getters
  - `.graph()`  - removed
  - `.base`     - now a property
  - `.current`  - added
  - `.current` and `.base` now return the `Edit`, which has `.graph` as a property.
    This makes it easier to compare the current and base graph, and just more consistent flow.

- Also WIP on replacing `context.t` and `.tHtml`
  Code to do these things should go through the LocalizationSystem
  There is quite a lot of this, and it will take a while.

modules/Context.js

@@ -125,10 +125,6 @@ export class Context extends EventEmitter {
       };
     };
 
-    this.graph = editor.graph;
-    this.hasEntity = (id) => editor.graph().hasEntity(id);
-    this.entity = (id) => editor.graph().entity(id);
-
     // LocalizationSystem
     const l10n = this.systems.l10n;
     this.t = l10n.t;
@@ -141,7 +137,7 @@ export class Context extends EventEmitter {
     // FilterSystem
     const filters = this.systems.filters;
     this.hasHiddenConnections = (entityID) => {
-      const graph = editor.graph();
+      const graph = editor.current.graph;
       const entity = graph.entity(entityID);
       return filters.hasHiddenConnections(entity, graph);
     };
@@ -347,7 +343,8 @@ export class Context extends EventEmitter {
 
     } else {
       canSave = this.selectedIDs().every(id => {
-        const entity = this.hasEntity(id);
+        const graph = editor.current.graph;
+        const entity = graph.hasEntity(id);
         return entity && !entity.isDegenerate();
       });
     }
@@ -476,7 +473,7 @@ export class Context extends EventEmitter {
   get copyIDs() { return this._copyIDs; }
   set copyIDs(val) {
     this._copyIDs = val;
-    this._copyGraph = this.systems.editor.graph();
+    this._copyGraph = this.systems.editor.current.graph;
   }
 
   get copyLoc()     { return this._copyLoc; }

modules/behaviors/DrawBehavior.js

@@ -281,7 +281,7 @@ export class DrawBehavior extends AbstractBehavior {
 //        const activeIndex = target.nodes.indexOf(activeID);
 //        if (activeIndex !== -1) {
 //          isActiveTarget = true;
-//          const graph = context.graph();
+//          const graph = context.systems.editor.current.graph;
 //          const projection = context.projection;
 //          const choice = geoChooseEdge(graph.childNodes(target), eventData.coord, projection, activeID);
 //
@@ -341,7 +341,7 @@ export class DrawBehavior extends AbstractBehavior {
 //        const activeIndex = target.nodes.indexOf(activeID);
 //        if (activeIndex !== -1) {
 //          isActiveTarget = true;
-//          const graph = context.graph();
+//          const graph = context.systems.editor.current.graph;
 //          const projection = context.projection;
 //          const choice = geoChooseEdge(graph.childNodes(target), eventData.coord, projection, activeID);
 //

modules/behaviors/HoverBehavior.js

@@ -124,7 +124,7 @@ export class HoverBehavior extends AbstractBehavior {
 //        const activeIndex = target.nodes.indexOf(activeID);
 //        if (activeIndex !== -1) {
 //          isActiveTarget = true;
-//          const graph = context.graph();
+//          const graph = context.systems.editor.current.graph;
 //          const projection = context.projection;
 //          const choice = geoChooseEdge(graph.childNodes(target), eventData.coord, projection, activeID);
 //

modules/behaviors/LassoBehavior.js

@@ -42,7 +42,8 @@ export class LassoBehavior extends AbstractBehavior {
     this._lassoing = false;
     this._extent = null;
 
-    const eventManager = this.context.systems.map.renderer.events;
+    const map = this.context.systems.map;
+    const eventManager = map.renderer.events;
     eventManager.on('pointerdown', this._pointerdown);
     eventManager.on('pointermove', this._pointermove);
     eventManager.on('pointerup', this._pointerup);
@@ -60,7 +61,8 @@ export class LassoBehavior extends AbstractBehavior {
     this._lassoing = false;
     this._extent = null;
 
-    const eventManager = this.context.systems.map.renderer.events;
+    const map = this.context.systems.map;
+    const eventManager = map.renderer.events;
     eventManager.off('pointerdown', this._pointerdown);
     eventManager.off('pointermove', this._pointermove);
     eventManager.off('pointerup', this._pointerup);
@@ -72,21 +74,22 @@ export class LassoBehavior extends AbstractBehavior {
    * Handler for pointerdown events.
    * @param  `e`  A Pixi FederatedPointerEvent
    */
-   _pointerdown() {
+  _pointerdown() {
     // Ignore it if we are not over the canvas
     // (e.g. sidebar, out of browser window, over a button, toolbar, modal)
-     const eventManager = this.context.systems.map.renderer.events;
-     if (!eventManager.pointerOverRenderer) return;
-
-     const modifiers = eventManager.modifierKeys;
-     const drawLasso = modifiers.has('Shift');
-
-     if (drawLasso) {
-       this._lassoing = true;
-       const coord = this.context.systems.map.mouseLoc();
-       this._extent = new Extent(coord);
-       this._coords.push(coord);
-     }
+    const map = this.context.systems.map;
+    const eventManager = map.renderer.events;
+    if (!eventManager.pointerOverRenderer) return;
+
+    const modifiers = eventManager.modifierKeys;
+    const drawLasso = modifiers.has('Shift');
+
+    if (drawLasso) {
+      this._lassoing = true;
+      const coord = map.mouseLoc();
+      this._extent = new Extent(coord);
+      this._coords.push(coord);
+    }
   }
 
 
@@ -98,19 +101,20 @@ export class LassoBehavior extends AbstractBehavior {
   _pointermove() {
     if (!this._lassoing) return;
 
-    const eventManager = this.context.systems.map.renderer.events;
+    const map = this.context.systems.map;
+    const eventManager = map.renderer.events;
     if (!eventManager.pointerOverRenderer) return;
 
-    const coord = this.context.systems.map.mouseLoc();
+    const coord = map.mouseLoc();
 
     // Update geometry and extent
     this._extent = this._extent.extend(new Extent(coord));
     this._coords.push(coord);
 
     // Push the polygon data to the map UI for rendering.
-    const mapUILayer = this.context.scene().layers.get('map-ui');
+    const mapUILayer = map.scene.layers.get('map-ui');
     mapUILayer.lassoPolygonData = this._coords;
-    this.context.systems.map.immediateRedraw();
+    map.immediateRedraw();
   }
 
 
@@ -123,7 +127,8 @@ export class LassoBehavior extends AbstractBehavior {
     if (!this._lassoing) return;
 
     this._lassoing = false;
-    const mapUILayer = this.context.scene().layers.get('map-ui');
+    const map = this.context.systems.map;
+    const mapUILayer = map.scene.layers.get('map-ui');
 
     const ids = this._lassoed();
     this._coords = [];
@@ -148,21 +153,23 @@ export class LassoBehavior extends AbstractBehavior {
 
 
   _lassoed() {
-    const graph = this.context.graph();
     const context = this.context;
-    const polygonLocs = this._coords;
-    const locationSystem = context.systems.locations;
+    const editor = context.systems.editor;
+    const locations = context.systems.locations;
+    const filters = context.systems.filters;
+    const graph = editor.current.graph;
 
     if (!this.context.editable()) return [];
 
-    let intersects = this.context.systems.editor
+    const polygonLocs = this._coords;
+    let intersects = editor
       .intersects(this._extent)
       .filter(entity => {
         return (
           entity.type === 'node' &&
           geomPointInPolygon(entity.loc, polygonLocs) &&
-          !context.systems.filters.isHidden(entity, graph, entity.geometry(graph)) &&
-          !locationSystem.blocksAt(entity.loc).length
+          !filters.isHidden(entity, graph, entity.geometry(graph)) &&
+          !locations.blocksAt(entity.loc).length
         );
       });
 

modules/behaviors/PasteBehavior.js

@@ -73,14 +73,16 @@ export class PasteBehavior extends AbstractBehavior {
   _doPaste(e) {
     const context = this.context;
     const editor = context.systems.editor;
+    const l10n = context.systems.l10n;
+    const map = context.systems.map;
 
     // Nothing to copy..
     const copyIDs = context.copyIDs;
     if (!copyIDs.length) return;
 
     // Ignore it if we are not over the canvas
     // (e.g. sidebar, out of browser window, over a button, toolbar, modal)
-    const eventManager = context.systems.map.renderer.events;
+    const eventManager = map.renderer.events;
     if (!eventManager.pointerOverRenderer) return;
 
     e.preventDefault();
@@ -96,6 +98,7 @@ export class PasteBehavior extends AbstractBehavior {
     let extent = new Extent();
     let pasteIDs = new Set();
     let originalIDs = new Set();
+    let currGraph = editor.current.graph;
     Object.values(copies).forEach(entity => originalIDs.add(entity.id));
 
     for (const id in copies) {
@@ -105,7 +108,7 @@ export class PasteBehavior extends AbstractBehavior {
       extent = extent.extend(oldEntity.extent(copyGraph));
 
       // Exclude child nodes from pasteIDs if their parent way was also copied.
-      const parents = context.graph().parentWays(newEntity);
+      const parents = currGraph.parentWays(newEntity);
       const parentCopied = parents.some(parent => originalIDs.has(parent.id));
       if (!parentCopied) {
         pasteIDs.add(newEntity.id);
@@ -116,17 +119,18 @@ export class PasteBehavior extends AbstractBehavior {
     // Default to map center if we can't determine the mouse pointer
     const copyLoc = context.copyLoc;
     const copyPoint = (copyLoc && projection.project(copyLoc)) || projection.project(extent.center());
-    const mousePoint = eventManager.coord || context.systems.map.centerPoint();
+    const mousePoint = eventManager.coord || map.centerPoint();
     const delta = vecSubtract(mousePoint, copyPoint);
 
-    const annotation = context.t('operations.paste.annotation', { n: pasteIDs.size });
+    const annotation = l10n.t('operations.paste.annotation', { n: pasteIDs.size });
     editor.perform(actionMove(Array.from(pasteIDs), delta, projection), annotation);
 
     // Put the user in move mode so they can place the pasted features
     // Grab the current versions from the graph (because they were just moved).
     const pasted = new Map();    // Map (entityID -> Entity)
+    currGraph = editor.current.graph;
     for (const pasteID of pasteIDs) {
-      pasted.set(pasteID, context.entity(pasteID));
+      pasted.set(pasteID, currGraph.entity(pasteID));
     }
     context.enter('move', { selection: pasted });
   }

modules/behaviors/SelectBehavior.js

@@ -444,31 +444,34 @@ export class SelectBehavior extends AbstractBehavior {
 
     const context = this.context;
     const editor = context.systems.editor;
+    const l10n = context.systems.l10n;
+    const validator = context.systems.validator;
+
     const coord = this.lastUp.coord;
     const data = this.lastUp.target?.data;
 
     const isOSMWay = data instanceof osmWay && !data.__fbid__;
     const isMidpoint = data.type === 'midpoint';
 
     if (isOSMWay) {
-      const graph = editor.graph();
+      const graph = editor.current.graph;
       const projection = context.projection;
       const loc = projection.invert(coord);
       const choice = geoChooseEdge(graph.childNodes(data), coord, projection);
       const edge = [data.nodes[choice.index - 1], data.nodes[choice.index]];
       editor.perform(
         actionAddMidpoint({ loc: loc, edge: edge }, osmNode()),
-        context.t('operations.add.annotation.vertex')
+        l10n.t('operations.add.annotation.vertex')
       );
-      context.systems.validator.validate();
+      validator.validate();
 
     } else if (isMidpoint) {
       const edge = [data.a.id, data.b.id];
       editor.perform(
         actionAddMidpoint({ loc: data.loc, edge: edge }, osmNode()),
-        context.t('operations.add.annotation.vertex')
+        l10n.t('operations.add.annotation.vertex')
       );
-      context.systems.validator.validate();
+      validator.validate();
     }
   }
 

modules/core/EditSystem.js

@@ -43,15 +43,14 @@ export class EditSystem extends AbstractSystem {
     this._mutex = utilSessionMutex('lock');
     this._hasRestorableChanges = false;
 
+    this._stack = [];          // stack of edits
+    this._index = 0;           // index of the current edit
     this._checkpoints = {};
     this._pausedGraph = false;
-    this._stack = [];
     this._tree = null;
-    this._index = 0;
     this._initPromise = null;
 
     // Make sure the event handlers have `this` bound correctly
-    this.graph = this.graph.bind(this);
     this.perform = this.perform.bind(this);
     this.replace = this.replace.bind(this);
     this.overwrite = this.overwrite.bind(this);
@@ -122,18 +121,28 @@ export class EditSystem extends AbstractSystem {
   }
 
 
-  graph() {
-    return this._stack[this._index].graph;
+  /**
+   * base
+   * @return {Edit} The base Edit in the stack
+   */
+  get base() {
+    return this._stack[0];
   }
 
-
-  tree() {
-    return this._tree;
+  /**
+   * current
+   * @return {Edit} The current Edit in the stack
+   */
+  get current() {
+    return this._stack[this._index];
   }
 
-
-  base() {
-    return this._stack[0].graph;
+  /**
+   * tree
+   * @return {Tree} The Tree (spatial index)
+   */
+  get tree() {
+    return this._tree;
   }
 
 
@@ -165,8 +174,8 @@ export class EditSystem extends AbstractSystem {
    *  @param  seenIDs?   Optional - All entity IDs on the tile (including previously seen ones)
    */
   merge(entities, seenIDs) {
-    const baseGraph = this.base();
-    const headGraph = this.graph();
+    const baseGraph = this.base.graph;
+    const headGraph = this.current.graph;
 
     if (!(seenIDs instanceof Set)) {
       seenIDs = new Set(entities.map(entity => entity.id));
@@ -435,7 +444,7 @@ export class EditSystem extends AbstractSystem {
   toIntroGraph() {
     let nextID = { n: 0, r: 0, w: 0 };
     let permIDs = {};
-    let graph = this.graph();
+    let graph = this.current.graph;
     let result = new Map();   // Map(entityID -> Entity)
 
     // Copy base entities..
@@ -510,7 +519,7 @@ export class EditSystem extends AbstractSystem {
   toJSON() {
     if (!this.hasChanges()) return;
 
-    const baseGraph = this.base();       // The initial unedited graph
+    const baseGraph = this.base.graph;   // The initial unedited graph
     const modifiedEntities = new Map();  // Map(Entity.key -> Entity)
     const baseEntities = new Map();      // Map(entityID -> Entity)
     const stackData = [];
@@ -593,7 +602,7 @@ export class EditSystem extends AbstractSystem {
     const context = this.context;
     const map = context.systems.map;
 
-    const baseGraph = this.base();   // The initial unedited graph
+    const baseGraph = this.base.graph;   // The initial unedited graph
     const hist = JSON.parse(json);
     let loadComplete = true;
 
@@ -872,7 +881,7 @@ export class EditSystem extends AbstractSystem {
 
   // determine difference and dispatch a change event
   _change(previous) {
-    const difference = new Difference(previous, this.graph());
+    const difference = new Difference(previous, this.current.graph);
     if (!this._pausedGraph) {
       this.emit('change', difference);
     }