aframevr / a-painter

🎨 Paint in VR in your browser.
https://aframe.io/a-painter/
MIT License
679 stars 200 forks source link

Multiplayer Implementation - How to Merge? #152

Open mrfrase3 opened 8 years ago

mrfrase3 commented 8 years ago

Hey All,

As mentioned in #151 me and @avrljk are working the multi-user aspect of A-Painter.

I've gotten it working seamlessly between the users (besides the memory leak in 151) and the code is current feature complete and almost ready to commit. I just have a few questions with regards to merging it, but first, a quick run down.

The method is fairly simple, the calls in system/brush.js add an event to a buffer, there is then a new system which I called socket, that runs the connection to a socket.io server. Every x ticks, the buffer is sent up to the server. The server then caches the list of events and pushes it to the rest of the clients, which the socket system just executes.

Each drawing session has an unique ID, you can when you join the session, the cached stroke events are sent to you. You can have several sessions occurring at once. (kinda jsfiddle style)

To get the server working, I created a new server file, which runs an express server with web-pack-middleware, the socket.io server then runs on that.

The questions I have are do people feel strongly for/against having a server in the main repo? Should the multi-user functionality even be merged to the master thread, be an actively updated fork, or uploaded to completely separate repo?

Maybe the client should only go into multi-user "mode" if there is a socket server detected, and then develop just the server in another repo? If a person decides to have multi-user, they just download the server which includes this repo as a dependancy?? idk

I also had to build the functionality on an older version due to issue #147, so the multi-user functionality wouldn't be very visible until the current version can be used with a headset again.

Also, TODO/possibilities:

fernandojsg commented 8 years ago

Hi @mrfrase3 @avrljk! So nice to hear that this features is moving on! Re merge here or not, I believe it could be nice to create a PR including the whole source code of the server + modifications on the client so we could take a look and discuss there if it's more suitable to include it here or maybe in a separate project instead of having the discussion here without knowing what is going on really on the backend, how big is it, how many modifications it will need in the client and so on.

Re the TODO/possibilities, It's a great list of features we could add! I'm specially interested on the users headests/controllers to really feel like you're painting with someone else there. Once the multiplayer code will be merged we could create new issues with these features so we could keep there the discussion about what to do with them.

Thanks again for taking care of this cool feature! Waiting for that PR ;)

dmarcos commented 8 years ago

What about creating eventually an a-painter-server repo that contains the server and the component/systems necessary for broadcasting?

fernandojsg commented 8 years ago

@dmarcos Yes, I was thinking if it's small enough we may want to have everything together for easy setup? idk really. In any case it could be nice to take a look at the code it doesn't matter if it's on a PR here or creating the other repo and have two PR one for each one. @mrfrase3 @avrljk do you have it uploaded somewhere where we could take a look so we could decide after how to proceed, before you spend time arranging it to do the PR?

cvan commented 8 years ago

I would recommend not standing up a separate server (at least not initially) using Firebase: (a) it'll be error-prone and expensive to handle many concurrent connections using WebSockets, and (b) all the WebVR-enabled builds of the browsers already support WebRTC data channels, so you really just need a WebSocket signalling server + WebRTC. And, Firebase or Signalhub should do you just fine. I'd recommend using Firebase for both WebSocket signalling and pub-sub for multiplayer (and upgrade to WebRTC where supported).

Check out the following:

mrfrase3 commented 8 years ago

Heyo,

Sorry, I've been bogged down with uni and other projects. That and waiting for #151.

Anyway, you can find the multi user version on my fork Here, the commit specific to the implementation can be found Here

As to the talk of firebase, I may have drunk the kool-aid before looking at the production pricing :S The main idea behind having the server that the data has to pass through is reliability, persistence and scalability.

I like to use this scenario to describe what I'm going for; Say two artists want to do a collaboration on an a-painter art piece, one of the artists has a large fan base on say twitter, they set the link permissions to view only and post it on twitter. Hundreds if not thousands of people then join in to watch these artists work in real time, being able to look around the environment with any device, from vive to phone, without making edits or disturbing the stream.

In that scenario, webRTC would not work, as the artists computer would catch fire from handling the workload, unless you did some crazy tor network level signaling. Also, the painting would not persist between drawing sessions, unless a binary was saved and uploaded every time. (which is annoying)

This leaves either a firebase or a socket server solutions, which are both similar, it would really boil down to cost, preference, time and ones trust in storing data on google servers. But in the true spirit of tacos:

Porque no los dos?

Currently the stroke data read/writes into a buffer for the socket connection, we could just make a system called say Multiuser which contains the read/write buffers/handling, and then have components for that system (socket, firebase, webRTC, etc) that broadcasts/receives data based on what the setup defines or maybe what the user requires. (I am a a-frame noob, so correct me if I'm wrong)

mrfrase3 commented 7 years ago

So I've recently revisited this and re-coded it in a much less impactful way to the main repo.

In commit #205 I've added the concept of an owner of a stroke, this wont affect those using this as singleplayer in the slightest yet having it allows a multiplayer implementation to inject whatever other code it needs during runtime without affecting the main repo.

Basically others can implement multiplayer servers however they like relatively easily.

An example of a socket implementation can be found here: https://github.com/mrfrase3/a-painter-socket-server

cvan commented 7 years ago

thanks for working on this! check out @haydenjameslee's https://github.com/haydenjameslee/networked-aframe, which should help.

I have a paid Firebase plan for Mozilla VR, which we could use. if there's a Heroku server you want us to deploy for signalling (and potentially WebSockets fallback), let me know, and I'll talk to our IT folks to see about deploying a paid, stable instance of a backend server for this realtime, multi-user support.

fernandojsg commented 7 years ago

After checking both implementations I tend to like this implementation as it doesn't modify almost anything on the core and let the developers to create their own custom backend so we don't rely on (and maintain) a complex implementation but leave it up to the user. I believe this is the way to go for the MVP of the multiplayer support and then we could evolve from there. I've already merged https://github.com/aframevr/a-painter/pull/205 although I was taking a look at the socket server but I didn't go deep on it, I'll review and we could keep the discussion on how to follow from here.