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

Feature/on-data #640

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

Feature/on-data #640

wants to merge 5 commits into from

Conversation

poison
Copy link

@poison poison commented Mar 30, 2020

Hi,

Thanks for the amazing library, just discovered it when I'm playing around with WebRTC. I know it's maybe not intended, but I'm actually adding some custom code to the signalling peer-server I'm running (in order to create rooms). For this, I need to intercept messages from the signalling server itself. That's why I modified the code a bit.

It might be handy for other users too, let me know what you think.

Best regards,
Nicolas

@Florrr
Copy link

Florrr commented May 20, 2020

I wanted to create a pull request but noticed that you did the exact same thing.
I've used that message for notifying peerjs that their call-participant disconnected (since I didn't figure out how to do that with peerjs itself..)

I'd recommend creating a new ServerMessageType in lib.enums.ts and adding a new case-statement in lib/peer.ts instead of using the default-statement.
See: Florrr@e8a2c48

@poison
Copy link
Author

poison commented May 20, 2020

Hi Florrr,

I suggest that I cancel my PR and you submit your PR with your suggestions. This one was done rather quick&dirty, and breaks backwards compatibility (the errors), while your suggestion seems the way to go.

I'm currently not using PeerJS in my project anymore since I have too many clients and the mesh became too heavy, so I have to use an SFU (mediasoup).

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.

2 participants