Multiple bug fixes, code comments, simplified MFCMessage object creation, and Enum builder

[Temptin]

Thank you for the well-written project, Kradek, and for open-sourcing it! Here are various code improvements for you.

[KradekMFC]

This is great! Thank you! I'm going to spend some time looking it over and then will likely merge it in the next couple days.

[Temptin]

Happy to help. You saved me loads of time reverse-engineering so the least I can do is give back. ;) Nice to see another Node developer!

[Temptin]

There's a cool new gift for you above (6371f0d). Yep, the new commit includes an epic new MFCEnums.js build system. ;)

If you have any information about the unclassified enum types/groups, I'd like your help naming them. For now, I've only named the enum groups that you had already identified (such as "MFCMessageType", and so on). There are lots more types on the site, and all unknown enum groups are automatically named as "MFCUnknown_######", i.e. "MFCUnknown_FCSERV" until (if ever) a better name is decided.

It's of course possible to read through top.html (at MFC) to try to figure out how those other enum groups are used on the official site, but if you've already done some of that work feel free to contribute better names for those groups.

[Temptin]

Aaaand... added support for enum descriptions in the build process (bd6291c), so that we don't lose your useful "FCVIDEO_TX_IDLE = In public room" note. There we go!

[KradekMFC]

Everything looks good! Nice work on catching a few of my errors and the auto enum builder. Went ahead and merged all of it.

You asked about ParseLine; I believe it lives in top.html (if you're looking at the non-mobile version). MFC uses frames and embeds a lot of their code directly in the pages. It looks like they are taking a more modern approach with the mobile version.

[Temptin]

@KradekMFC Great! Happy to contribute to your excellent project. ;) Oh and remember to tag v0.1.6 for release too; I see you've edited index.js but haven't listed the new version as a github release.

And yeah I edited my comment to remove the question as soon as I discovered ParseLine in top.html. Tip: After running the messy top.html page (and mfccore.js) through jsbeautifier.org, you've got an awesome and readable reference for how the protocol works.

[KradekMFC]

Done.

Re: beautifier, these days I usually just hit the pretty print in Chrome dev tools, but used jsbeautifier myself when I was first figuring out the code.

I’ve been figuring out the enums on an as needed basis for various projects and never went through and tried to figure them all out, but it may be useful at some point. I’ve had in mind possibly creating a set of message classes (beyond the very simple three already there) to abstract away most of the nitty gritty of the protocol, but haven’t really needed it. A lot of it is there to support MFC’s own client interface.

From: Temptin [mailto:notifications@github.com] Sent: Monday, May 11, 2015 1:20 PM To: KradekMFC/MFCSocket Cc: KradekMFC Subject: Re: [MFCSocket] Multiple bug fixes, code comments, simplified MFCMessage object creation, and Enum builder (#1)

@KradekMFC https://github.com/KradekMFC Great! Happy to contribute to your excellent project. ;) Oh and remember to tag v0.1.6 for release too; I see you've edited index.js but haven't listed the new version as a github release.

And yeah I edited my comment to remove the question as soon as I discovered ParseLine in top.html. Tip: After running the messy top.html page (and mfccore.js) through jsbeautifier.org, you've got an awesome and readable reference for how the protocol works.

— Reply to this email directly or view it on GitHub https://github.com/KradekMFC/MFCSocket/pull/1#issuecomment-100982960 . https://github.com/notifications/beacon/ADTguz7DH0dp6gvIFNUMVsCtJUsm0297ks5oINxBgaJpZM4EVyIB.gif

[Temptin]

@KradekMFC Sounds like me then. I don't foresee ever needing all of those other enums since most are related to tipping and joining pvt/grp and stuff. But at least they're there now if we find a need, and the list can easily be looked through. I did consider naming the "bans" enum since it seems to be for notifying a registered user that a model has banned them from their channel; but I didn't touch it since I couldn't find any part of top.html or mfccore.js which actually looked up those enums, so maybe they're unused in the official code, or dealt with in the flash GUI...

Anyway... I agree with abstracting away more of the protocol, but it'd be a lot of work to make it object oriented. The protocol is already pretty simple to deal with using the current library; the hard part is figuring out which combinations of arguments to use and what the flags in the responses mean, but that is where top.html/mfccore.js come in. ;) Let's hold off on any deeper restructuring until our projects actually need it. I might do a little work in this area, though. For example: each object would need outhoing/incoming functions; outgoing takes human readable args like user/password, and incoming parses a received MFCMessage object and checks success and returns a nice object with sessionid, nickname, etc.

One more thing: Have I understood the protocol correctly in that MFCMessageType is always the type ID of the incoming/outgoing message, and that ALL other enums are merely used in the flags fields (arg1/arg2/data) in requests/responses? If so, I have some ideas for restructuring things a bit more to make that even clearer.

[KradekMFC]

Yes, the message type is only used to identify the kind of message being sent. I’ve never run across any other enum used to identify a message type.

From: Temptin [mailto:notifications@github.com] Sent: Monday, May 11, 2015 1:57 PM To: KradekMFC/MFCSocket Cc: KradekMFC Subject: Re: [MFCSocket] Multiple bug fixes, code comments, simplified MFCMessage object creation, and Enum builder (#1)

@KradekMFC https://github.com/KradekMFC Sounds like me then. I don't foresee ever needing all of those other enums since most are related to tipping and joining pvt/grp and stuff. But at least they're there now if we find a need, and the list can easily be looked through. I did consider naming the "bans" enum since it seems to be for notifying registered user that a model has banned them; but I didn't touch it since I couldn't find any part of top.html or mfccore.js which actually looked up those enums, so maybe they're unused in the official code, or dealt with in the flash GUI...

Anyway... I agree with abstracting away more of the protocol, but it'd be a lot of work to make it object oriented. The protocol is already pretty simple to deal with using the current library; the hard part is figuring out which combinations of arguments to use and what the flags in the responses mean, but that is where mfc.html/mfccore.js come in. ;) Let's hold off on any deeper restructuring until our projects actually need it. I might do a little work in this area, though. For example: each object would need outhoing/incoming functions; outgoing takes human readable args like user/password, and incoming parses a received MFCMessage object and checks success and returns a nice object sith sessionid, nickname, etc.

One more thing: Have I understood the protocol correctly in that MFCMessageType is always the type ID of the incoming/outgoing message, and that ALL other enums are merely used in the flags fields (arg1/arg2/data) in requests/responses? If so, I have some ideas for restructuring things a bit more to make that even clearer.

— Reply to this email directly or view it on GitHub https://github.com/KradekMFC/MFCSocket/pull/1#issuecomment-100998614 . https://github.com/notifications/beacon/ADTgu9hT6z2c1l5iM2cotp3PKe1c8piWks5oIOTQgaJpZM4EVyIB.gif

[Temptin]

@KradekMFC Good. I thought so, since the message queue used at MFC uses a switch to determine message type and only seems to use FCTYPE. I will double-check this before any restructuring.

I'll get to work when I have time (maybe a day or two from now), and will make a new pull request when I have something to release.

[Temptin]

@KradekMFC I have to apologize, I've got so much to do and don't have time to work on this anymore. The library works fine as it is now. ;-)

By the way, there was one (and only one) problem with one of my commits: https://github.com/Temptin/MFCSocket/blob/master/lib/MFCSocket.js#L36

That binary check needs to be removed, because I just discovered that the "ws" library doesn't contain the usual WebSocket datatype of Blob.

In fact, it's still possible to check for binary (Blob) in the "ws" library, but it's done this way instead:

ws.on('message', function(data, flags) {
  // flags.binary will be set if a binary data is received. (is undefined if ascii)
}

That argument won't be received when using the "onmessage" method (which is what you're doing), though, since the "onmessage" shorthand emulates the browser API for websockets, which does indeed use "instanceof Blob" to check for binary. But there is no Blob type here, so no way to check.

At least MFC probably never sends binary, heh, so it shouldn't matter much to remove the safety check.

Take care man, and hope you enjoyed the contributions! :-)

[Temptin]

@KradekMFC Well I guess I'll give you one more thing; I've successfully tried the workaround now:

https://github.com/Temptin/MFCSocket/blob/master/lib/MFCSocket.js#L95

Change that line to:

socket.on( 'message', onSocketMessage );

https://github.com/Temptin/MFCSocket/blob/master/lib/MFCSocket.js#L35

Change those three lines to:

function onSocketMessage(data, flags){
    if (!flags.binary){ //make sure the received websocket message is text (not a binary blob)
        var mfcMessageQueue = data; //websocket messages are always complete and don't need to be reconstructed/buffered

(Note the change from "msg.data" to just "data".)

That's all there is to fixing it. ;)

[KradekMFC]

Not a worry :) Thanks for the code and contributions!

From: Temptin [mailto:notifications@github.com] Sent: Tuesday, May 12, 2015 4:56 PM To: KradekMFC/MFCSocket Cc: KradekMFC Subject: Re: [MFCSocket] Multiple bug fixes, code comments, simplified MFCMessage object creation, and Enum builder (#1)

@KradekMFC https://github.com/KradekMFC I have to apologize, I've got so much to do and don't have time to work on this anymore. The library works fine as it is now. ;-)

By the way, there was one (and only one) problem with one of my commits: https://github.com/Temptin/MFCSocket/blob/master/lib/MFCSocket.js#L36

That binary check needs to be removed, because I just discovered that the "ws" library doesn't contain the usual WebSocket datatype of Blob.

In fact, it's still possible to check for binary (Blob) in the "ws" library, but it's done this way instead:

ws.on('message', function(data, flags) { // flags.binary will be set if a binary data is received. }

That argument won't be received when using the "onmessage" method (which is what you're doing), though, since the "onmessage" shorthand emulates the browser API for websockets, which does indeed use "instanceof Blob" to check for binary. But there is no Blob type here, so no way to check.

At least MFC probably never sends binary, heh, so it shouldn't matter much to remove the safety check.

Take care man, and hope you enjoyed the contributions! :-)

— Reply to this email directly or view it on GitHub https://github.com/KradekMFC/MFCSocket/pull/1#issuecomment-101417014 . https://github.com/notifications/beacon/ADTgu7dkqCX_uP0mXU7YahBz_Bf564M2ks5oImBNgaJpZM4EVyIB.gif