phoboslab / Impact

HTML5 Game Engine
MIT License
1.99k stars 204 forks source link

Just create one `Undo` object per tile edit #80

Open Joncom opened 4 years ago

Joncom commented 4 years ago

This property can be deleted:

https://github.com/phoboslab/Impact/blob/98037a5f310858b3328e4a4537295b8df4a19c66/lib/weltmeister/edit-map.js#L18

And so can these two methods:

https://github.com/phoboslab/Impact/blob/98037a5f310858b3328e4a4537295b8df4a19c66/lib/weltmeister/edit-map.js#L81-L97

And then patch weltmeister.js as follows:

diff --git a/lib/weltmeister/weltmeister.js b/lib/weltmeister/weltmeister.js
index 8c9bae9..7fd9d1c 100644
--- a/lib/weltmeister/weltmeister.js
+++ b/lib/weltmeister/weltmeister.js
@@ -717,14 +717,6 @@ wm.Weltmeister = ig.Class.extend({
                                        }
                                        else {
                                                this.undo.beginMapDraw();
-                                               this.activeLayer.beginEditing();
-                                               if( 
-                                                       this.activeLayer.linkWithCollision && 
-                                                       this.collisionLayer && 
-                                                       this.collisionLayer != this.activeLayer
-                                               ) {
-                                                       this.collisionLayer.beginEditing();
-                                               }
                                                this.setTileOnCurrentLayer();
                                        }
                                }
@@ -831,10 +823,12 @@ wm.Weltmeister = ig.Class.extend({
                                var mapy = y + by * this.activeLayer.tilesize;

                                var newTile = brushRow[bx];
-                               var oldTile = this.activeLayer.getOldTile( mapx, mapy );
-                               
-                               this.activeLayer.setTile( mapx, mapy, newTile );
-                               this.undo.pushMapDraw( this.activeLayer, mapx, mapy, oldTile, newTile );
+                               var oldTile = this.activeLayer.getTile( mapx, mapy );
+
+                               if( newTile !== oldTile ) {
+                                       this.activeLayer.setTile( mapx, mapy, newTile );
+                                       this.undo.pushMapDraw( this.activeLayer, mapx, mapy, oldTile, newTile );
+                               }

                                if( 
@@ -845,8 +839,10 @@ wm.Weltmeister = ig.Class.extend({
                                        var collisionLayerTile = newTile > 0 ? this.collisionSolid : 0;

                                        var oldCollisionTile = this.collisionLayer.getOldTile(mapx, mapy);
-                                       this.collisionLayer.setTile( mapx, mapy, collisionLayerTile );
-                                       this.undo.pushMapDraw( this.collisionLayer, mapx, mapy, oldCollisionTile, collisionLayerTile );
+                                       if( collisionLayerTile !== oldCollisionTile ) {
+                                               this.collisionLayer.setTile( mapx, mapy, collisionLayerTile );
+                                               this.undo.pushMapDraw( this.collisionLayer, mapx, mapy, oldCollisionTile, collisionLayerTile );
+                                       }
                                }
                        }
                }

There is no need to keep a reference to oldData because the old data already gets passed into the undo and redo objects. Additionally, there is no need to make 10's of undo objects per single tile edit, which currently happens. The added if( newTile !== oldTile ) check ensures we only have one undo object per edit, which is necessary before we can delete the oldData array.