I'm trying to implement undo and redo in a pygame application using lambdas, but something to do with references or else my understanding of the implementation of list.remove()
is causing my program to crash. The code that creates undoable actions is the following
elif MOUSEBUTTONUP == event.type:
x, y = pygame.mouse.get_pos()
if leftClick:
if len( objects ) > 0 and objects[ -1 ].is_open():
actions.do( \
[ lambda: objects[ -1 ].add( Point( x, y, 0 ) ) ], \
[ lambda: objects[ -1 ].remove( Point( x, y, 0 ) ) ] \
)
else:
actions.do( \
[ lambda: objects.append( Polygon( ( 255, 255, 255 ) ).add( Point( x, y, 0 ) ) ) ],
[ lambda: objects.pop() ] \
)
where objects is a list of Polygon
s, which are defined as
class Polygon:
def __init__( self, colour, width = 0 ):
self._points = []
self._colour = colour
self._isopen = True
self._width = width
def get_colour( self ):
return self._colour
def add( self, point ):
self._points.append( point )
return self
def remove( self, point ):
print "List contains " + str( self._points )
print "Trying to remove " + str( point )
self._points.remove( point )
return self
def num_points( self ):
return len( self._points )
def get_points( self ):
""" Returns a copy of the points in this vector as a list. """
return self._points[:]
def open( self ):
self._isopen = True
return self
def close( self ):
self._isopen = False
return self
def is_open( self ):
return self._isopen
def set_width( self, width ):
self._width = width
return self
def get_width( self ):
return self._width
def is_filled( self ):
return self._filled
and the points that are added are defined as
class Point:
def __init__( self, x, y, z ):
self.x = x
self.y = y
self.z = z
def rel_to( self, point ):
x = self.move( point.z, point.x, self.z, self.x )
y = self.move( point.z, point.y, self.z, self.y )
return ( x, y )
def move( self, viewer_d, displacement, object_d, object_h ):
over = object_h * viewer_d + object_d * displacement
under = object_d + viewer_d + 1
return over / under
def __str__( self ):
return "(%d, %d, %d)" % ( self.x, self.y, self.z )
def __eq__( self, other ):
return self.x == other.x and self.y == other.y and self.z == other.z
def __ne__( self, other ):
return not ( self == other )
actions
, in the first snippet, is an instance of Action
, the definition of which follows
class Actions:
#TODO implement immutability where possible
def __init__( self ):
self.undos = []
self.redos = []
def do( self, do_steps, undo_steps ):
for do_step in do_steps:
do_step()
self.undos.append( ( do_steps, undo_steps ) )
self.redos = []
def can_undo( self ):
return len( self.undos ) > 0
def undo( self ):
if self.can_undo():
action = self.undos.pop()
_, undo_steps = action
for undo_step in undo_steps:
undo_step()
self.redos.append( action )
def can_redo( self ):
return len( self.redos ) > 0
def redo( self ):
if self.can_redo():
action = self.redos.pop()
redo_steps, _ = action
for redo_step in redo_steps:
redo_step()
self.undos.append( action )
So the problem is that when I click more than twice and try to call actions.undo()
I get an exception with list.remove(x)
, it says x
isn't present in the list, I think because it tries to remove the same point twice. The reason this doesn't happen before the first two clicks is that the first undo tries to remove the most recent point, whereas the second undo simply pops the polygon off the objects stack. My question is why the Point
that actions.undo()
tries to remove the same point twice, even though the first point should have been popped off the self.undos
stack and pushed onto the self.redos
stack? Thanks very much for any feedback.
I think I figured it out, it works as expected anyway. The theory I have deduced is that the lambda actions were creating closures over
x
,y
andz
of thePoint()
parameter, so instead of copying the values of these variables when the anonymous function was created it instead maintained a pointer to their values. So I could delete the most recent point, asx
,y
andz
still referred to an existing point, but failed on the second undo as the point no longer existed. I used a leaf from stackp's undo/redo tutorial at http://stackp.online.fr/?cat=8, and passed the parameter to the anonymous functions as a list toaction.do()
, which meant that the parameters were evaluated when thedo()
function executed, and so the proper arguments were sent todo()
. Sorry for the long question and answer.