[Feature & Refactor] Refactored w2g protocol & add additional sync point

[EnergoStalin]

Additions to p2p protocol

Added additional sync point when someone joined ongoing session.

Current algorythm in nutshell by examples

1. Client 1 creates lobby and becomes host automatically
2. Client 2 join lobby with isHost false because code is present in joinLobby call receiving current torrent and shared session state from client 1 on 'peerconnect'
3. Client 3 join lobby the same way as client 2 receiving state from client 1
4. Client 1 leaves no host in the room so no new clients will receive initial state
5. Client 4 joins no state synced to him
6. Client 2 pick new torrent becoming host
7. Now client 2 sends state to all new joined peers until someone else picked torrent acquiring host

Closes #363

[ThaUnknown]

how will this behave if there are >3 people trying to sync joining at the same time tho...

[EnergoStalin]

Didn't test it with 3 clients but only current host(last client sended 'torrent' event will send it on 'peerconnect' to new peers) so theoretically it shouldn't cause problems and will behave the same way as with 2 clients.

The algorythm in nutshell by examples

1. Client 1 creates lobby and becomes host automatically
2. Client 2 join lobby with isHost false because code is present in joinLobby call receiving current torrent and shared session state from client 1 on 'peerconnect'
3. Client 3 join lobby the same way as client 2 receiving state from client 1
4. Client 1 leaves no host in the room so no new clients will receive initial state
5. Client 4 joins no state synced to him
6. Client 2 pick new torrent becoming host
7. Now client 2 sends state to all new joined peers until someone else picked torrent acquiring host

I didn't figure out more adequate way. This is the best way to set trusted node which will send state to new peers. I thinked about doing it based on peer.id but it seems different among clients so it's not an option.

It didn't affect any existing logic just add additional sync point to client directed on initializing onging session on new clients.

[EnergoStalin]

Please also look at this. Whether you like such approach or not.

[ThaUnknown]

this is good code, but it tries to fix a broken system, to put it simply: "Miru's code fucking sucks", and you're trying to build on top of what's fucked, the 2nd PR you linked fixes a LOT of that, but I feel like you take OOP way too far, it's not that required in JS

to my knowledge webrtc guarantees packet order, so I don't think there's much need in creating a "batch" event, as you can simply send the events one by one and it will work the same way

honestly feel free to dump all my code, and write it from scratch like you did in that draft PR, just a little bit less OOP, you dont need a separate class for each event XD

[EnergoStalin]

I feel like you take OOP way too far

Feel the same way that's why it's just draft and i asked someone to look.

Thought a little bit and class approach with events the most declarative way to do it so i against changes here.

to my knowledge webrtc guarantees packet order

Also thought that way when see msgId option in send interface but it was easier to implement it that way instead of assuming. Firstly i added array support on top level making all packets capable of batching but breaks backward compatibility too much. Don't think it's an issue with automatic updates tho 😉 . Will do it like you said then.

honestly feel free to dump all my code

Okay will update it and merge it here soon then.

[EnergoStalin]

Decided to leave excessive OOP approach as is. Thinked about introducing LeaveEvent because during testing with 2 clients 'peerclose' not always called even withing a ~minute leaving 5 different peer instances in the list. Now it can be considered more safe(i think maybe it's my setup's fault noticed faulty internet today so removed it for now). Also i learned that awaiting p2pt.send is a bad idea.

I want to test with 3 clients before merging. Tomorrow will look at it.

[EnergoStalin]

Forgot to mention link to lobby being copied on its creation without extra step of clicking 'Invite to lobby'

[EnergoStalin]

yep 'peerclose not firing for me it also somehow bugged like that. To reproduce quickly rejoin lobby multiple times.

Same issue on master branch withouth backfiring wrong user on init tho.

[ThaUnknown]

yep 'peerclose not firing for me it also somehow bugged like that. To reproduce quickly rejoin lobby multiple times.

Same issue on master branch withouth backfiring wrong user on init tho.

this is likely just local webrtc woes, afaik this works fine on remote connections, timeout is like 30 seconds tho

[EnergoStalin]

Which import style do you like more?

This

// #region Types

/**
 * @typedef {import('./session.js').Magnet} Magnet
 * @typedef {import('./session.js').W2GSession} W2GSession
 * @typedef {import('p2pt').Peer<any>} Peer
 * @typedef {import('./events.js').SyncEventBase} SyncEventBase
 * @typedef {import('./events.js').MsgData} MsgData
 */

/**
 * @template T
 * @typedef {import('./events.js').EventData<T>} EventData<T>
 */

// #endregion

...
/**
   * @param {Peer} peer
   * @param {SyncEventBase} event
   */
  #sendEvent (peer, event) {
    console.log('out W2GMsg', event)
    this.#p2pt?.send(peer, JSON.stringify(event))
  }
...

Or this

/**
   * @param {import('p2pt').Peer<any>} peer
   * @param {import('./events.js').SyncEventBase} event
   */
  #sendEvent (peer, event) {
    console.log('out W2GMsg', event)
    this.#p2pt?.send(peer, JSON.stringify(event))
  }

I did first way for now. But it's harder to maintain cause it's handwritten second import generated by autocomplete.

[EnergoStalin]

Still backfiring player events need to be improved.

[EnergoStalin]

Need to do something with video buffering because if someone joins to lobby now not having torrent downloaded it ignore initial seeking and reset all lobby to the beginning when buffering ends.

[EnergoStalin]

Very simplified schema for counting in bufferization

Common rules

• Do not update session's player state while buffering
• Wait for initial local buffering to complete for seeking to player state time if not already
• Pause and block resuming player on all clients while any peers buffering

[ThaUnknown]

I won't have the time to look at this for the next week, so this is gonna hang for a while, but the stats update schema seems.... overcomplex?

[EnergoStalin]

Actual changes listed below scheme.

How i think it will work

• peer joined seesion all other peer paused
• peer buffered sending buffered event
• when all peers buffered playback resumes

And now i realized that we only need to have info about peer buffering to force waiting before continue. So only additional buffering event needed.

[EnergoStalin]

I know! The most funniest solution is reply to unpause events from buffering client by pause events also sending toasts to all that it's buffering. That way we don't need store extra state among clients. Local buffering still need to be fixed tho.

This will involve excessive traffic but should work excellent since if peer suddenly leaves, no one stuck for 30 seconds until peerclose fired. Pinging that peer also not good idea tho cause all clients need to ping it simultationously. I found old existing WebRTC solution(which don't want to build locally). Will look at it's implementation and think of it. Before rushing to code.

I don't believe i will finish this till the end of the year. Since i sadly also have things to do.

[EnergoStalin]

@ThaUnknown how strong are you against typescript? I can't see any of you repositories using it which is quite surprising for me at least. I'm asking because it can reduce a lot of code in this PR removing large inconvenient jsdoc descriptions and i managed partially enable it without harming anything. I do my best not using anything typescript specific except type definitions in this PR.

Check it out here it has squash merged feat/qol branch for now can be easily removed with interactive rebase.

[ThaUnknown]

it has it's place, not in Miru tho, it's tooling fucking pisses me off, and jsdoc is enough 99% of the time

you're too used to typescript and use a fuckload of OOP, which is why you need so much JSDoc, I usually let it interpret shit with using object literals, which don't have its type but typedoc inherits it's structure

you write good code, don't get me wrong, it's just not my style, I do waaaaaaaaaaay too much insanely experimental and rapid prototyping, and typescript often gets in the way of that slowing me down, and reduces my time to work, which I already have little of, which is why I don't use it, in work or major projects I'd use it tho

I'm considering sticking my fingers into this PR and correcting some stuff more to how I'd write it, but for now I'm more concerned by the "bidirectional bus" shit which has got me confused as to it's high complexity

[ThaUnknown]

@EnergoStalin I've pushed some of my changes, which well, yeet a lot of things

I'm not so afraid of using any, I know it's bad, but I prefer simplicity in many cases like these

Added some config settings for VSCode

about jsdoc, you don't need type declarations everywhere, it's well capable of inheriting them on its own, for example:

class {
  /**
   * @type {number}
   */
  #index = 0
}

you don't need the type number here, it will inherit it, as it's initiated with a number in the first place

feel free to yeet the commit or something, but it's something I'd do anyways after accepting this PR, tho I didn't test the event changes, sorry XD

[EnergoStalin]

jsdoc is enough 99% of the time

It is i'd even say in 100% cause it's just typescript in comments with stupid amount of excessive shit to define something. As i said large and inconvenient for nested typing.

you're too used to typescript

Can't say that is true still cant write complex types by myself and also prefer not to use it in small projects not based on any framework which already has typings

use a fuckload of OOP

That one i must admit is i used to. Cant force myself to try haskell as radical alternative to expand my thinking. Also i tried to write function/module based code after you mentioned it first time but quickly find it inconvinient for approach i took.

typescript often gets in the way of that slowing me down

I can 100% agree on this take but as folks say when project grows it becomes opposite... Personally i don't have such big projects so it's impossible for me to judge you on that but i would take some changes already if it was me.

I just tired a lot to live without typing during this PR at least in code i wrote and i would prefer to have it at least partially i need to jump across files(using TEXT search for god sake cause not all things binded or resolved properly such as events) a lot that's why i rewrote it already to typescript and will maintain ts version for myself while you can feel free to transpile it to js before merging if you considering to tweak it anyway.

As for "bidirectional bus" it just prevents same events from firing twice with optional filtering on top of it. You can easily get what it does when watch on miru logs while player sending events cause some of them firing twice with the same data. I don't want to fix player on top of it so i just did filtering repeating data. I'm currently thinking of splitting and optimizing it on player side so we can get rid of the bus cause it looks like a crutch on top of not well thinked player synchronization code.

[EnergoStalin]

about jsdoc, you don't need type declarations everywhere, it's well capable of inheriting them on its own, for example:

Yeah i go overhead with it wanted to have explicit types and play with it a bit cause i rarely use it

feel free to yeet the commit or something

I wont. I integrate you changes in ts branch and will work on it there for now. It seems like buffering problem now should be fixed only on player side because player ignore seeking while it's buffering i dont think it should be handled in W2G part cause it's part of player api.

Also i finally joined discord i guess it's time to truly become part of community :rocket:

If i dared to spend of my life on this

[EnergoStalin]

Hm when watching player code i noticed other uncovered case. It's when playerAutoplay is enabled it should be temporarily synced between clients for consistency. So playback related settings should belong to session and be shared between clients for example playback speed not synced. It's not particularly useful in my case or why i started this PR but to do things right and avoid further PR's featuring it this would be necessarily. @ThaUnknown I would like to discuss this changes cause it will involve changes in Player component paramaters and MediaHandler.

So in order to sync playback settings and wire player events not instance based(from MediaHandler where's the closest 'module' context):

• Remove all reference to w2g from player exposing api's for management instead.
• Wire that api's from MediaHandler(maybe renaming/decomposing it) to w2g.
• Store playback related settings based on some session provider cause it should be easily revertable to local settings when session done(somewhat strategy pattern use local when no session and use sessions when it persists).

Benefits

• Will reduce complexity of event based state management.
• Will allow syncing playback settings with other peers like playback speed.

Trade-offs

None besides relatively huge feature touching at least 3 components(Player, MediaHandler, W2GSession) and probably out of PR scope without another rebranding.

[EnergoStalin]

TODO

• [ ] Rewrite Player, MediaHandler to allow playback settings sync and generally improve player api controls.
  • [ ] https://github.com/ThaUnknown/miru/pull/390#issuecomment-1858606909
  • [ ] https://github.com/ThaUnknown/miru/pull/390#issuecomment-1902485878
• [ ] Use some sort of consensus algorithm as transport(W2GClient will utilize W2GTransport) for state syncing?
• [ ] Think about better W2GSession implementation based on new player api

[ThaUnknown]

playerAutoplay

it will simply trigger index, and non lobby leader indexs are swallowed aren't they, so non issue?

simply when non-leader calls index, and w2g sees that it goes "hell nah you dont" and emits the current lobby state index, disallowing non-leaders from changing index? it's hacky but it will work

[EnergoStalin]

lobby leader indexs are swallowed

As I remember currently not but should be.

I'm currently more concerned about case when W2GSession will have old player state and will be forced to sync it to new peers cause state synced only on video load or pause/unpause/seek events but not during playback. And no direct way of obtaining it(adding rpc why?). That's one of the main reasons I want to expose function/channel based static api from MediaHandler for controlling it's player since all Player settings is instance based. Don't think it would harm i rather expect it to improve architecture since main player controls can be singleton like thing for easy use from another modules. I'm even more likely want to make offline session thing which exposes player controls itself and automatically syncs calls to controls from across miru when connected to room disallowing "unauthenticated" player access.

Something like this

```js // MediaHandler.svelte Githubissues.

Githubissues is a development platform for aggregating issues.