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

AXE - change necessary to work webrtc. #737

Merged
merged 1 commit into from
Apr 21, 2019
Merged

Conversation

rogowski
Copy link
Collaborator

The condition in line https://github.com/rogowski/gun/blob/master/gun.js#L2065 must be true only if Websocket. wire instanceof opt.WebSocket was added to condition.

This change is necessary to WEBRTC because 'open' == wire.readyState and not 1 like Websocket.

Changes in adapters/mesh.js are to sync the changes with gun.js.

@amark
Copy link
Owner

amark commented Apr 21, 2019

@rogowski yeah, I apologize, I was worried about this when it was added when we were trying to get multicast working (we think some crashes/bugs were due to socket not being open).

I'll probably just delete that line moving forward, since I assume other transports might have the same problem (I was hoping if wire.readyState would isolate it, but lol, why on earth do multiple transports have to use the same property readyState but have different codes for it!!???), which is why I peer.say( lets people building transport plugins for DAM handle custom send commands if they are not WebSocket compatible.

Thanks for catching this so fast! It'll be going out in the next release.

When I get around to it, I'll also be switching AXE to be a DAM hook/plugin, so that way we don't have to do a special Gun.AXE check (AXE will just attach itself to DAM, without DAM having to check if AXE is on GUN), I need to think about how to do this cleanly.

I'm SO SUPER EXCITED ABOUT AXE!!!! and your PANIC test for it is EPIC. :D :D :D

@amark amark merged commit e7e9c35 into amark:master Apr 21, 2019
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