Improve action UX and state storage

src/components/item.js

@@ -24,7 +24,7 @@ export class Item extends Stateful {
     this.immutable ??= state?.immutable ?? false
     this.type = state?.type ?? configuration?.type
     if (this.type === undefined) {
-      console.debug(`[Item:${this.id}]`, state)
+      console.debug(`[Item:${this.id}]`, state, configuration)
       throw new Error('Item must have type defined')
     }
 

src/components/itemFactory.js

@@ -5,7 +5,7 @@ import { Reflector } from './items/reflector'
 import { Wall } from './items/wall'
 import { Item } from './item'
 
-export function itemFactory (parent, state, configuration) {
+export function itemFactory (parent, state, index) {
   let item
 
   switch (state.type) {

src/components/items/portal.js

@@ -109,8 +109,7 @@ export class Portal extends movable(rotatable(Item)) {
     } else if (exitPortals.length === 1) {
       const exitPortal = exitPortals[0]
       console.debug(this.toString(), 'single exit portal matched:', exitPortal)
-      // Since no user choice was made, don't store this decision as a delta (move)
-      return this.#getStep(beam, nextStep, exitPortal, false)
+      return this.#getStep(beam, nextStep, exitPortal)
     } else {
       // Multiple matching destinations. User will need to pick one manually.
       console.debug(this.toString(), 'found multiple valid exit portals:', exitPortals)
@@ -123,6 +122,8 @@ export class Portal extends movable(rotatable(Item)) {
           onTap: (puzzle, tile) => {
             const exitPortal = data.exitPortals.find((portal) => portal.parent === tile)
             if (exitPortal) {
+              // Add a move, since the user made a decision
+              puzzle.addMove()
               beam.addStep(this.#getStep(beam, nextStep, exitPortal))
               puzzle.unmask()
             }
@@ -206,21 +207,21 @@ export class Portal extends movable(rotatable(Item)) {
     return exitPortals
   }
 
-  #getStep (beam, nextStep, exitPortal, keepDelta = true) {
+  #getStep (beam, nextStep, exitPortal) {
     const direction = Portal.getExitDirection(nextStep, this, exitPortal)
     return nextStep.copy({
       connected: false,
       direction,
       insertAbove: exitPortal,
       onAdd: (step) => {
         exitPortal.update(direction, step)
-        // Store this decision in beam state and generate a matching delta
-        beam.updateState((state) => ((state.steps ??= {})[step.index] = { [this.id]: exitPortal.id }), keepDelta)
+        // Store this decision in beam state
+        // TODO: store this less cryptically (e.g. use "portal")
+        beam.updateState((state) => ((state.steps ??= {})[step.index] = { [this.id]: exitPortal.id }))
       },
       onRemove: (step) => {
-        // Remove any associated beam state, but don't generate a delta.
-        // If the step is being removed, a delta for that action was most likely created elsewhere already.
-        beam.updateState((state) => { delete state.steps[step.index] }, false)
+        // Remove any associated beam state
+        beam.updateState((state) => { delete state.steps[step.index] })
         exitPortal.update(direction)
       },
       point: exitPortal.parent.center,

src/components/items/tile.js

@@ -5,7 +5,6 @@ import { emitEvent, getPointBetween } from '../util'
 import { modifierFactory } from '../modifierFactory'
 
 export class Tile extends Item {
-  icons = []
   selected = false
 
   #ui
@@ -25,13 +24,15 @@ export class Tile extends Item {
 
     // These need to be last, since they reference this
     this.items = (state.items || [])
-      .map((state) => itemFactory(this, state))
+      .map((state, index) => itemFactory(this, state, index))
       .filter((item) => item !== undefined)
 
     this.modifiers = (state.modifiers || [])
-      .map((state) => modifierFactory(this, state))
+      .map((state, index) => modifierFactory(this, state, index))
       .filter((modifier) => modifier !== undefined)
 
+    this.modifiers.forEach((modifier) => this.updateIcon(modifier))
+
     this.update()
   }
 
@@ -42,7 +43,7 @@ export class Tile extends Item {
 
   addModifier (modifier) {
     this.modifiers.push(modifier)
-    this.update()
+    this.updateIcon(modifier)
   }
 
   afterModify () {
@@ -57,7 +58,7 @@ export class Tile extends Item {
   }
 
   getState () {
-    const state = { type: this.type }
+    const state = { id: this.id, type: this.type }
 
     // Filter out beams, which are not stored in state
     const items = this.items.filter((item) => item.type !== Item.Types.beam).map((item) => item.getState())
@@ -106,7 +107,7 @@ export class Tile extends Item {
     const index = this.modifiers.indexOf(modifier)
     if (index >= 0) {
       this.modifiers.splice(index, 1)
-      this.update()
+      this.updateIcon(modifier)
     }
   }
 
@@ -122,18 +123,25 @@ export class Tile extends Item {
     return `[${this.type}:${this.coordinates.offset.toString()}]`
   }
 
-  update () {
-    super.update()
-
-    // Update tile modifier icons
-    this.icons = this.modifiers.map((modifier, index) => {
-      const position = getPointBetween(this.#ui.hexagon.segments[index].point, this.center, (length) => length / 3)
-      return modifier.getSymbol().place(position, { fillColor: modifier.immutable ? '#ccc' : '#333' })
-    })
-
-    // Everything after child 0 (hexagon) is an icon
-    this.group.removeChildren(1)
-    this.group.addChildren(this.icons)
+  updateIcon (modifier) {
+    const index = this.modifiers.indexOf(modifier)
+    if (index >= 0) {
+      const position = getPointBetween(
+        this.#ui.hexagon.segments[index].point,
+        this.center,
+        (length) => length / 3
+      )
+      const style = { fillColor: modifier.immutable ? '#ccc' : '#333' }
+      const icon = modifier.getSymbol().place(position, { style })
+      icon.data = { id: modifier.id, name: modifier.name, type: modifier.type }
+      const childIndex = this.group.children.findIndex((icon) => icon.data.id === modifier.id)
+      if (childIndex >= 0) {
+        // Update existing
+        this.group.children[childIndex].replaceWith(icon)
+      } else {
+        this.group.addChild(icon)
+      }
+    }
   }
 
   static parameters (height) {

src/components/layout.js

@@ -35,7 +35,7 @@ export class Layout extends Stateful {
     this.layers.items = new Layer()
 
     this.modifiers = (state.modifiers || [])
-      .map((state) => modifierFactory(null, state))
+      .map((state, index) => modifierFactory(null, state, index))
       .filter((modifier) => modifier !== undefined)
 
     // Find the widest row

src/components/modifier.js

@@ -8,6 +8,7 @@ import { Tile } from './items/tile'
 
 const modifiers = document.getElementById('modifiers')
 
+// Use incrementing IDs to preserve sort order
 let uniqueId = 0
 
 export class Modifier extends Stateful {
@@ -19,7 +20,6 @@ export class Modifier extends Stateful {
   configuration
   element
   disabled = false
-  id = uniqueId++
   immutable = false
   name
   parent
@@ -28,8 +28,11 @@ export class Modifier extends Stateful {
   type
 
   constructor (tile, state) {
+    // Retain ID from state if it exists, otherwise generate a new one
+    state.id ??= uniqueId++
     super(state)
 
+    this.id = state.id
     this.parent = tile
     this.type = state.type
   }
@@ -142,6 +145,9 @@ export class Modifier extends Stateful {
           break
         }
       }
+
+      // Keep the tile icon in sync
+      this.parent?.updateIcon(this)
     }
 
     this.#down = false

src/components/modifierFactory.js

@@ -6,30 +6,30 @@ import { Toggle } from './modifiers/toggle'
 import { Modifier } from './modifier'
 import { Swap } from './modifiers/swap'
 
-export function modifierFactory (tile, configuration) {
+export function modifierFactory (parent, state, index) {
   let modifier
 
-  switch (configuration.type) {
+  switch (state.type) {
     case Modifier.Types.immutable:
-      modifier = new Immutable(tile, configuration)
+      modifier = new Immutable(...arguments)
       break
     case Modifier.Types.lock:
-      modifier = new Lock(tile, configuration)
+      modifier = new Lock(...arguments)
       break
     case Modifier.Types.move:
-      modifier = new Move(tile, configuration)
+      modifier = new Move(...arguments)
       break
     case Modifier.Types.rotate:
-      modifier = new Rotate(tile, configuration)
+      modifier = new Rotate(...arguments)
       break
     case Modifier.Types.swap:
-      modifier = new Swap(tile, configuration)
+      modifier = new Swap(...arguments)
       break
     case Modifier.Types.toggle:
-      modifier = new Toggle(tile, configuration)
+      modifier = new Toggle(...arguments)
       break
     default:
-      console.error('Ignoring modifier with unknown type:', configuration.type)
+      console.error('Ignoring modifier with unknown type:', state.type)
       break
   }
 

src/components/modifiers/toggle.js

@@ -5,18 +5,19 @@ export class Toggle extends Modifier {
   on
   title = 'Toggle'
 
-  constructor (tile, { on }) {
+  constructor (tile, state) {
     super(...arguments)
 
-    this.on = on || false
+    // TODO: refactor to use 'toggled' everywhere
+    this.on = state.on || this.parent?.items.some(item => item.on || item.toggled)
     this.name = this.getName()
   }
 
   attach (tile) {
     super.attach(tile)
-    // Consider the modifier toggled if there is at least one toggled item in the tile
-    // TODO: refactor to be 'toggled' everywhere
-    this.on = this.tile?.items.some((item) => item.on || item.toggled)
+
+    // TODO: refactor to use 'toggled' everywhere
+    this.on = this.tile?.items.some(item => item.on || item.toggled)
     this.update({ name: this.getName() })
   }
 
@@ -26,7 +27,7 @@ export class Toggle extends Modifier {
 
   moveFilter (tile) {
     // Filter out tiles that contain no toggleable items
-    return super.moveFilter(tile) || !tile.items.some((item) => item.toggleable)
+    return super.moveFilter(tile) || !tile.items.some(item => item.toggleable)
   }
 
   onTap (event) {

src/components/puzzle.js

@@ -91,6 +91,10 @@ export class Puzzle {
     this.select()
   }
 
+  addMove () {
+    return this.#state.addMove()
+  }
+
   centerOnTile (offset) {
     const tile = this.layout.getTileByOffset(offset)
     paper.view.center = tile.center
@@ -228,12 +232,11 @@ export class Puzzle {
     return previouslySelectedTile
   }
 
-  updateState (keepDelta = true) {
-    this.#state.update(Object.assign(this.#state.getCurrent(), { layout: this.layout.getState() }), keepDelta)
+  updateState () {
+    this.#state.update(Object.assign(this.#state.getCurrent(), { layout: this.layout.getState() }))
     this.#updateActions()
-    if (keepDelta) {
-      emitEvent(Puzzle.Events.Updated, { state: this.#state })
-    }
+
+    emitEvent(Puzzle.Events.Updated, { state: this.#state })
   }
 
   #addLayers () {
@@ -249,7 +252,7 @@ export class Puzzle {
 
   #getModifiers (tile) {
     // Sort by ID to ensure they always appear in the same order regardless of ownership
-    return this.layout.modifiers.concat(tile?.modifiers || [])
+    return (tile?.modifiers || []).concat(this.layout.modifiers)
       .filter((modifier) => !modifier.immutable)
       .sort((a, b) => a.id - b.id)
   }
@@ -350,6 +353,7 @@ export class Puzzle {
         .forEach((other) => other.update({ disabled: true }))
     }
 
+    this.addMove()
     this.updateState()
 
     this.#beams
@@ -385,7 +389,8 @@ export class Puzzle {
     emitEvent(Puzzle.Events.Solved)
   }
 
-  #onStateUpdate () {
+  #onStateUpdate (event) {
+    console.debug('Puzzle.#onStateUpdate()', event)
     this.updateState()
   }
 
@@ -482,7 +487,7 @@ export class Puzzle {
       : undefined
 
     this.updateSelectedTile(selectedTile)
-    this.updateState(false)
+    this.updateState()
     this.update()
   }
 

src/components/solution.js

@@ -166,7 +166,7 @@ class Moves extends SolutionCondition {
 
   update (event) {
     console.debug('Moves.update', event)
-    this.#moves = event.detail.state.moves()
+    this.#moves = event.detail.state.moves().length
     this.#completed.textContent = this.#moves.toString()
   }