emberjs-addons / sproutcore-statechart

(Ember Only) Repository for the ported SproutCore Statechart package
www.sproutcore.com
50 stars 11 forks source link

sendAction() calls the passed function more then necessary when sent from concurrent substates' enterState() #5

Closed zeppelin closed 12 years ago

zeppelin commented 13 years ago

The number of times the action is called is multiplied by order the substate was defined.

Demonstration: http://shellshaper.com/sproutcore/sendaction-bug/

Code:

var App = SC.Application.create();

App.statechart = SC.Object.extend(SC.StatechartManager, {
  rootState: SC.State.extend({

    substatesAreConcurrent: true,

    testAction: function(sender) {
      var output = "testAction sent from: "+sender;
      $('body').append('<div>'+output+'</div>');
      console.log(output);
    },

    stateA: SC.State.extend({
      enterState: function() {
        this.statechart.sendAction('testAction', this.get('stateName'));
      }
    }),
    stateB: SC.State.extend({
      enterState: function() {
        this.statechart.sendAction('testAction', this.get('stateName'));
      }
    }),
    stateC: SC.State.extend({
      enterState: function() {
        this.statechart.sendAction('testAction', this.get('stateName'));
      }
    })
  })
});

App.statechart = App.statechart.create();

Output:

testAction sent from: stateA
testAction sent from: stateB
testAction sent from: stateB
testAction sent from: stateC
testAction sent from: stateC
testAction sent from: stateC
FrozenCanuck commented 13 years ago

This is a flaw that was fixed in the SproutCore's 1.x version of the statechart framework; however, looks like the fix was not applied to the port in SC 2.0. Thanks for the notice. Will get fixed.

robmonie commented 13 years ago

Thanks Mike. I'm keen on this fix as well.

ColinCampbell commented 13 years ago

@frozencanuck when you get this fixed, can you close this issue? Thanks mate!

zeppelin commented 12 years ago

@FrozenCanuck Are there any updates regarding this issue? I'd fix this if necessary, but have no idea where to start...

FrozenCanuck commented 12 years ago

@zeppelin My apologies for the delayed response. Unfortunately I did not get a chance to fix the problem earlier. The problem is within the sendAction method where it does not keep track of what states have been checked when trying to handle an action. This is the current logic causing the problem:

for (; i < len; i += 1) {
  actionHandled = false;
  state = currentStates[i];
  if (!get(state, 'isCurrentState')) continue;
  while (!actionHandled && state) {
    actionHandled = state.tryToHandleAction(action, arg1, arg2);
    if (!actionHandled) state = get(state, 'parentState');
    else statechartHandledAction = true;
  }
}

This is the fix that has been applied to the statechart framework in SproutCore 1.x (currently residing in the merge_public branch):

var checkedStates = {};
for (; i < len; i += 1) {
  eventHandled = NO;
  state = currentStates[i];
  if (!state.get('isCurrentState')) continue;
  while (!eventHandled && state) {
    if (!checkedStates[state.get('fullPath')]) {
     eventHandled = state.tryToHandleEvent(event, arg1, arg2);
     checkedStates[state.get('fullPath')] = YES;
    }
    if (!eventHandled) state = state.get('parentState');
    else statechartHandledEvent = YES;
  }
}

I have created the fix and there is now a pending pull request to bring it in:

https://github.com/emberjs-addons/sproutcore-statechart/pull/13

zeppelin commented 12 years ago

@FrozenCanuck thank you vm! :)