PActivity activityFinished gets called multiple times

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Create a new PActivity
2. Call action.terminate with a parameter of TERMINATE_AND_FINISH
3. Repeat the same call

What is the expected output? What do you see instead?
the activityFinished handler should only get called once, but will be
called as many times as you call the terminate method (along with a bunch
of activityStarted events)

Here is the proposed fix:
Index: src/edu/umd/cs/piccolo/activities/PActivity.java
===================================================================
--- src/edu/umd/cs/piccolo/activities/PActivity.java    (revision 30)
+++ src/edu/umd/cs/piccolo/activities/PActivity.java    (revision 31)
@@ -57,6 +58,7 @@
    private PActivityDelegate delegate;

    private boolean stepping;
+    private boolean done;
    private long nextStepTime;

    /**
@@ -66,7 +68,9 @@
     */
    public interface PActivityDelegate {
        public void activityStarted(PActivity activity);
+
        public void activityStepped(PActivity activity);
+
        public void activityFinished(PActivity activity);
    }

@@ -103,6 +107,7 @@
        startTime = aStartTime;
        nextStepTime = aStartTime;
        stepping = false;
+        done = false;
    }

    //****************************************************************
@@ -183,6 +188,13 @@
    }

    /**
+     * Return true if this activity is done.
+     */
+    public boolean isDone() {
+        return done;
+    }
+
+    /**
     * Return true if this activity is performing an animation. This is used
     * by the PCanvas to determine if it should set the render quality to
     * PCanvas.animatingRenderQuality or not for each frame it renders.
@@ -197,8 +209,7 @@
     * activity finishes.
     */
    protected void activityStarted() {
-       if (delegate != null)
-           delegate.activityStarted(this);
+        if (delegate != null) delegate.activityStarted(this);
    }

    /**
@@ -208,8 +219,7 @@
     * @param elapsedTime the amount of time that has passed relative to the
activities startTime.
     */ 
    protected void activityStep(long elapsedTime) {
-       if (delegate != null)
-           delegate.activityStepped(this);
+        if (delegate != null) delegate.activityStepped(this);
    }

    /**
@@ -217,8 +227,8 @@
     * activity has been removed from the PActivityScheduler queue.
     */
    protected void activityFinished() {
-       if (delegate != null)
-           delegate.activityFinished(this);
+        if (delegate != null) delegate.activityFinished(this);
+        done = true;
    }

    /**
@@ -262,11 +272,11 @@
    }

    /**
-    * Stop this activity immediately, and remove it from the activity
+     * If not done, stops this activity immediately, and remove it from
the activity
     * scheduler. The termination behavior determines when and if activityStarted
     * and activityFinished get called. The possible termination behaviors are as
     * follow:
-    * 
+     * <p/>
     * TERMINATE_WITHOUT_FINISHING - The method activityFinished will never
get called and
     * so the activity will be terminated midway.
     * TERMINATE_AND_FINISH - The method activityFinished will always get
called. And so the
@@ -276,6 +286,9 @@
     * if the activity has previously started.
     */
    public void terminate(int terminationBehavior) {
+        if (done) {
+            return;
+        }
        if (scheduler != null) {
            scheduler.removeActivity(this);
        }
@@ -283,6 +296,7 @@
        switch (terminationBehavior) {
            case TERMINATE_WITHOUT_FINISHING:
                stepping = false;
+                done = true;
                break;

            case TERMINATE_AND_FINISH:
@@ -293,7 +307,6 @@
                    activityStarted();
                    activityFinished();
                }
-
                break;

            case TERMINATE_AND_FINISH_IF_STEPPING:

Original issue reported on code.google.com by steveonjava on 29 Jun 2008 at 11:16

[GoogleCodeExporter]

Original comment by steveonjava on 2 Jul 2008 at 12:28

• Added labels: Milestone-2.0

[GoogleCodeExporter]

Original comment by steveonjava on 3 Jul 2008 at 5:05

• Added labels: Milestone-1.3

[GoogleCodeExporter]

Should it be possible to reuse an activity after it has been finished?

Original comment by heue...@gmail.com on 7 Oct 2008 at 9:12

[GoogleCodeExporter]

I don't think so.  Since conceptually, they have a start time and duration.

It might be an optimization to allow it, but I don't think it's much of one.

Original comment by allain.lalonde on 18 Feb 2009 at 11:36

[GoogleCodeExporter]

Here is a use case that will no longer work after the proposed fix is committed

http://tinyurl.com/n3xv8o

A new instance of WalkingActivity would need to be created each time walking() 
is
called for every instance of WalkingSprite, rather than just one instance per
WalkingSprite.

Original comment by heue...@gmail.com on 22 Jul 2009 at 9:43

[GoogleCodeExporter]

This can be resolved by adding a reset/reschedule method to the activity that 
would
reschedule the event when called.

Activity could be single serving things (which I like) and then reused when 
needed.

Original comment by allain.lalonde on 31 Jul 2009 at 4:31

[GoogleCodeExporter]

I vote for WontFix.  An activity with a duration of -1 should always keep 
running if
it has not been finished.  Perhaps the possible states and state transitions for
PActivity might be clarified for version 2.0.

Original comment by heue...@gmail.com on 9 Oct 2009 at 4:21

[GoogleCodeExporter]

agree with heuermh.

Original comment by mr0...@mro.name on 9 Oct 2009 at 8:41

[GoogleCodeExporter]

Since most think this shouldn't be fixed, I am marking as such.

Original comment by allain.lalonde on 25 Oct 2009 at 11:32

• Changed state: WontFix

[GoogleCodeExporter]

Since most think this shouldn't be fixed, I am marking as such.

Original comment by allain.lalonde on 25 Oct 2009 at 11:32

[GoogleCodeExporter]

Original comment by allain.lalonde on 30 Oct 2009 at 4:13

• Changed state: Verified