RanvierMUD / core

Core engine code for Ranvier
https://ranviermud.com
MIT License
45 stars 40 forks source link

Set active properties before emitting events #104

Open Sakeran opened 5 years ago

Sakeran commented 5 years ago

This changes Effect#activate and Effect#deactivate such that the Effect's active property is set to the proper value before their corresponding events are emitted. Most importantly, this prevents a case where an infinite loop might be created.

Example (some debugEffect.js):

module.exports = {
  config: {
    duration: 1000
  },
  listeners: state => ({
    effectDeactivated: function() {
      this.target.emit('badNews'); // infinite loop
      Broadcast.sayAt(this.target, "Effect deactivated.");
    }
  })
};

In this case, EffectList#validateEffects and the listener function will be stuck invoking one another until the stack overflows. While less likely to occur practice, we could engineer a similar loop with Effect#activate and its related event.

Setting the active property before any events are called ensures that each method will short-circuit before this becomes an issue.

nelsonsbrian commented 5 years ago

I think I have experienced this as well. I have had to gate the deactivate listener to only fire once in a few things. But never looked into it.

azigler commented 4 years ago

Really nice catch!