Question

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 Polygons, 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.

Was it helpful?

Solution

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 and z of the Point() 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, as x, y and z 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 to action.do(), which meant that the parameters were evaluated when the do() function executed, and so the proper arguments were sent to do(). Sorry for the long question and answer.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top