Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi-node capability #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

shadiakiki1986
Copy link

Hi. I added the ability to chat with multiple users in your serverless-webrtc project. It's basically just pulling out code from your js/serverless-webrtc.js file into two classes SWHostConnection.js and SWGuestConnection.js. The main code is in separate html and js files than your original serverless-webrtc.html and js/serverless-webrtc.js. In a nutshell, this is just extending your connection to an array of connections.

Shadi Akiki added 2 commits January 28, 2015 12:38
Added export/import of answer/offers (via FileSaver js library)
Added nicknames (needed for distinguishing different recipients)
Added network display of recipients + individual send/latency/close buttons
Documented in README
@cjb
Copy link
Owner

cjb commented Jan 28, 2015

@shadiakiki1986 Hey, this sounds great, thank you very much! I'll review and test it soon.

@benwiley4000
Copy link

@cjb any update?

@cjb
Copy link
Owner

cjb commented Mar 14, 2017

Ah, I'm sorry I never replied to this. The feature's nice, but it looks like there's lots of code duplication and style issues that it'd be good to avoid. Unless we have a volunteer to do the cleanup perhaps it's best to leave this on a separate branch for people to use..


#### Multi-node capability

serverless-webrtc-multinode.html adds on top of serverless-webrtc.html the functionality of many-to-many connections.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please line wrap to 80 cols to match the rest of the file


serverless-webrtc-multinode.html adds on top of serverless-webrtc.html the functionality of many-to-many connections.

It just does so by making a new WebRTC peer connection for each new pair of users.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can drop "just"


It just does so by making a new WebRTC peer connection for each new pair of users.

Peer connections are either ''Guest'' connections (Bob the recipient), or ''Host'' connections (Alice the sender)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should use real double quotes "

put a period at the end of the sentence

these issues come up below too

/* THIS IS BOB, THE ANSWERER/RECEIVER */

function SWGuestConnection() {
return {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these files/funcs have bad/inconsistent indentation in general

you could try fixing it with something like clang-format -i -style=file

counterparty: null,
lastTimestamp: null,
latency: null,
pc2 : new RTCPeerConnection(cfg, con),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no spaces before the :

guestConnections: null,
p1: null,
init: function(un,pc2a2,p11) {
var self=this;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing spaces around the =

this func has inconsistent indentation

},
close: function() {
this.pc2.close();
if(this.echoId!=null) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing spaces in the right place

you should compare to null using !== instead. or just go with a truthy check like if (this.echoid) {.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants