search

node-dmx / dmx

DMX controller library for node.js
MIT License
297 stars 96 forks source link

Changed update event so it also catches animations #76

Closed williamoverton closed 5 years ago

williamoverton commented 5 years ago

Currently the update event is only sent when individual calls are made the library. this means animations do not trigger events. I have passed the event handling down a layer into the driver to cover this.

I was not able to change the event handling in the main library class as animations are run against the driver's universe object and I didn't want to break existing functionality with animations.

williamoverton commented 5 years ago

Any update on this?

Cheers

Fensterbank commented 5 years ago

Thank you for your PR and sorry for the inactivity. While I self-assigned myself I was quite busy the last two weeks. I try to review, test and merge it this weekend!

williamoverton commented 5 years ago

Great news!

if possible I'd be very interested in being part of the node-dmx org as I plan to do lots of work on this in the future! Please let me know if you're interested :)

Thanks,

Will

Fensterbank commented 5 years ago

Seems to be a legit and useful change. Thanks for your work! 🍪

I noticed your commits are made as woverton wi***uk@***.com and it seems to be another mail address than the one you are using on Github. This means the commits cannot be referenced with your Github account, which is the reason we don't see your profile picture here: grafik

Since you plan to make more commits in the future, you may want to have this work linked to your Github.

If this was by mistake I'll wait and give you the change to make a fix commit (--amend) and a force push with your Github mail adress before merging. 🙂

williamoverton commented 5 years ago

@Fensterbank Good spot! Simply added my new email to my github account and that seems to have sorted it. Cheers