Starting code review

3 files changed, 83 insertions, 63 deletions

diff --git a/activity.py b/activity.py
index 8d2dc2a..1295f2d 100644
--- a/activity.py
+++ b/activity.py
@@ -63,10 +63,10 @@ class AnimateActivity(activity.Activity):
         separator = gtk.SeparatorToolItem()
         toolbarbox.toolbar.insert(separator, -1)
 
-        add_image = ToolButton('list-add')
-        add_image.set_tooltip(_('Add a frame'))
-        add_image.connect('clicked', self._add_frame)
-        toolbarbox.toolbar.insert(add_image, -1)
+        add_frame = ToolButton('list-add')
+        add_frame.set_tooltip(_('Add a frame'))
+        add_frame.connect('clicked', self._add_frame)
+        toolbarbox.toolbar.insert(add_frame, -1)
 
         remove_frame = ToolButton('list-remove')
         remove_frame.set_tooltip(_('Remove the selected frame'))
@@ -116,7 +116,7 @@ class AnimateActivity(activity.Activity):
         options_toolbar.insert(pingpong, -1)
 
         self.modes_buttons = [normalmode, returnbutton, pingpong]
-        self.animation_mode = animation.RETURN_MODE
+        self.animation_mode = animation.MODE_RETURN
 
         separator = gtk.SeparatorToolItem()
         options_toolbar.insert(separator, -1)
@@ -209,7 +209,7 @@ class AnimateActivity(activity.Activity):
         zfile.write(temp_file_path, 'config')
         frames_path = tempfile.mktemp()
         count = 0
-        for i in self._animation.images:
+        for i in self._animation.frames:
             i.save(frames_path, 'png')
             zfile.write(frames_path, 'frames/%d.png' % count)
             count += 1
@@ -253,7 +253,7 @@ class AnimateActivity(activity.Activity):
         if result == gtk.RESPONSE_ACCEPT:
             jobject = chooser.get_selected_object()
             pixbuf = gtk.gdk.pixbuf_new_from_file(jobject.get_file_path())
-            self._animation.add_image(pixbuf)
+            self._animation.add_frame(pixbuf)
             self._frames_list.add_frame(pixbuf)
         else:
             return
@@ -304,7 +304,7 @@ class AnimateActivity(activity.Activity):
             self._animation = animation.Animation(width, height)
             self._animation.set_mode(self._animation_mode)
             for pixbuf in self._animation_frames:
-                self._animation.add_image(pixbuf)
+                self._animation.add_frame(pixbuf)
                 self._frames_list.add_frame(pixbuf)
             self._animation.set_fps(self._fpsbutton.get_value())
             self._animation.show_all()
diff --git a/animation.py b/animation.py
index 1791533..83d33b1 100644
--- a/animation.py
+++ b/animation.py
@@ -29,9 +29,9 @@ _logger = logging.getLogger('animate-animation')
 _logger.setLevel(logging.DEBUG)
 logging.basicConfig()
 
-NORMAL_MODE = 0
-RETURN_MODE = 1
-PING_PONG_MODE = 2
+MODE_NORMAL = 0
+MODE_RETURN = 1
+MODE_PING_PONG = 2
 
 
 class Animation(gtk.Image):
@@ -41,20 +41,28 @@ class Animation(gtk.Image):
                                 0,
                                 2,
                                 1,
+                                gobject.PARAM_READWRITE),
+                        'fps': (gobject.TYPE_INT,
+                                'framerate',
+                                'frames per second',
+                                1,
+                                24,
+                                2,
                                 gobject.PARAM_READWRITE)}
+
     __gsignals__ = {"current-frame-updated": (gobject.SIGNAL_RUN_LAST,
                                               gobject.TYPE_NONE,
                                               (gobject.TYPE_INT,))}
 
     def __init__(self, width, height, fps=2):
-        gtk.Image.__init__(self)
+        super(Animation, self).__init__()
 
-        self._current_image = 0
+        self._current_frame = 0
         self._timeout = None
-        self._mode = RETURN_MODE
+        self._mode = MODE_RETURN
         self._running = False
-        self.images = []
-        self.images_reverse = False
+        self.frames = []
+        self.frames_reverse = False
 
         self._size = width, height
         self._sleep_time = int(1000 / fps)
@@ -62,25 +70,29 @@ class Animation(gtk.Image):
     def do_set_property(self, pspec, value):
         if pspec.name == 'mode':
             self.set_mode(value)
+        elif pspec.name == 'fps':
+            self.set_fps(value)
 
     def do_get_property(self, pspec):
         if pspec.name == 'mode':
             return self._mode
+        elif pspec.name == 'fps':
+            return self.get_fps()
 
     def move(self, old_pos, new_pos):
-        _object = self.images[old_pos]
-        self.images.remove(old_pos)
-        self.images.insert(new_pos, _object)
+        _object = self.frames[old_pos]
+        self.frames.remove(old_pos)
+        self.frames.insert(new_pos, _object)
 
     def remove_current_frame(self):
-        del(self.images[self._current_image])
-        if len(self.images):
-            self.set_pos(self._current_image
-                         if len(self.images) >= self._current_image
-                         else len(self.images) - 1)
+        del(self.frames[self._current_frame])
+        if len(self.frames):
+            self.set_current_frame(self._current_frame
+                         if len(self.frames) >= self._current_frame
+                         else len(self.frames) - 1)
         else:
             self.set_from_pixbuf(None)
-            self._current_image = 0
+            self._current_frame = 0
 
     def set_fps(self, fps):
         self._sleep_time = int(1000 / fps)
@@ -88,11 +100,14 @@ class Animation(gtk.Image):
             self.pause()
             self.run()
 
+    def get_fps(self):
+        return int(self._sleep_time * 1000.0)
+
     def set_mode(self, mode):
         """Defines animation mode"""
-        if self.images_reverse:
-            self.images.reverse()
-            self.images_reverse = False
+        if self.frames_reverse:
+            self.frames.reverse()
+            self.frames_reverse = False
 
         self._mode = mode
 
@@ -103,18 +118,18 @@ class Animation(gtk.Image):
         """Defines animation size"""
         self._size = width, height
 
-        # Redraw image:
-        self.set_pos(self._current_image)
+        # Redraw frame:
+        self.set_current_frame(self._current_frame)
 
-    def add_image(self, pixbuf):
-        """Appends an image to the list"""
-        self.images.append(pixbuf)
+    def add_frame(self, pixbuf):
+        """Appends an frame to the list"""
+        self.frames.append(pixbuf)
 
     def run(self):
         """Runs the animation"""
         if not self._running:
             try:
-                pixbuf = self.images[self._current_image]
+                pixbuf = self.frames[self._current_frame]
                 width = pixbuf.get_width()
                 height = pixbuf.get_height()
                 max_size = max(width, height)
@@ -134,13 +149,16 @@ class Animation(gtk.Image):
                 _logger.info(" Running!")
 
             except IndexError:
-                _logger.error(" There aren't images")
+                _logger.error(" There aren't frames")
         else:
             _logger.error(" The animation is running already")
 
-    def set_pos(self, pos):
-        self._current_image = pos
-        pixbuf = self.images[self._current_image]
+    def get_current_frame(self):
+        return self._current_frame
+
+    def set_current_frame(self, pos):
+        self._current_frame = pos
+        pixbuf = self.frames[self._current_frame]
         width = pixbuf.get_width()
         height = pixbuf.get_height()
         max_size = max(width, height)
@@ -162,35 +180,35 @@ class Animation(gtk.Image):
 
     def next(self):
         """Shows the next frame"""
-        if self._current_image != len(self.images) - 1:
-            self._current_image += 1
+        if self._current_frame != len(self.frames) - 1:
+            self._current_frame += 1
 
-        elif not self._current_image != len(self.images) - 1 and \
-                                                not self._mode == NORMAL_MODE:
-            if self._mode == PING_PONG_MODE:
-                self.images.reverse()
-                self.images_reverse = not self.images_reverse
-                self._current_image = 1
+        elif not self._current_frame != len(self.frames) - 1 and \
+                                                not self._mode == MODE_NORMAL:
+            if self._mode == MODE_PING_PONG:
+                self.frames.reverse()
+                self.frames_reverse = not self.frames_reverse
+                self._current_frame = 1
 
             else:
-                self._current_image = 0
+                self._current_frame = 0
 
-        self.emit("current-frame-updated", self._current_image)
+        self.emit("current-frame-updated", self._current_frame)
 
         return True
 
     def back(self):
         """Shows the previous frame"""
-        if self._current_image != 0:
-            self._current_image -= 1
+        if self._current_frame != 0:
+            self._current_frame -= 1
 
-        elif self._current_image == 0 and not self._mode == NORMAL_MODE:
-            if self._mode == PING_PONG_MODE:
-                self.images.reverse()
-                self.images_reverse = not self.images_reverse
-                self._current_image = -2
+        elif self._current_frame == 0 and not self._mode == MODE_NORMAL:
+            if self._mode == MODE_PING_PONG:
+                self.frames.reverse()
+                self.frames_reverse = not self.frames_reverse
+                self._current_frame = -2
 
             else:
-                self._current_image = -1
+                self._current_frame = -1
 
-        self.emit("current-frame-updated", self._current_image)
+        self.emit("current-frame-updated", self._current_frame)
diff --git a/frames_list.py b/frames_list.py
index b81a935..cbd5fec 100644
--- a/frames_list.py
+++ b/frames_list.py
@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 #
-# images_list.py
+# frames_list.py
 #
 # Copyright 2012 S. Daniel Francis <francis@sugarlabs.org>,
 #                Agustin Zubiaga <aguz@sugarlabs.org>
@@ -75,8 +75,8 @@ class FramesList(gtk.ScrolledWindow):
         self.frames = count - 1
 
     def _select_frame(self, widget, index):
-        if widget.images_reverse:
-            index = len(widget.images) - 1 - index
+        if widget.frames_reverse:
+            index = len(widget.frames) - 1 - index
         self.selection.select_iter(self.store.get_iter(index))
         selected_path = self.store.get_path(self.selection.get_selected()[1])
         self.treeview.scroll_to_cell(selected_path)
@@ -99,10 +99,12 @@ class FramesList(gtk.ScrolledWindow):
     def _selection_changed_cb(self, selection):
         model, _iter = selection.get_selected()
         try:
-            if self._animation.images_reverse:
-                self._animation.set_pos(self._animation._current_image)
+            if self._animation.frames_reverse:
+                self._animation.set_current_frame(
+                                        self._animation.get_current_frame())
             else:
-                self._animation.set_pos(self.store.get_value(_iter, 0) - 1)
+                self._animation.set_current_frame(self.store.get_value(_iter,
+                                                                        0) - 1)
         except:
             pass