-
Notifications
You must be signed in to change notification settings - Fork 118
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
Adding back player limits to streamers in the Signalling Server #427
Adding back player limits to streamers in the Signalling Server #427
Conversation
@@ -10,7 +10,7 @@ | |||
"start": "node ./dist/index.js --serve --console_messages verbose --log_config --https_redirect", | |||
"lint": "eslint ./src --ext .js,.jsx,.ts,.tsx", | |||
"test": "echo \"Error: no test specified\" && exit 1", | |||
"watch": "nodemon -V -d 3 --watch src --watch ../Signalling/dist -e 'ts,js,mjs,cjs,json' --exec 'npm run build && node ./dist/index.js --serve --console_messages verbose --log_config --https_redirect --player_port 1025' --http_root www", | |||
"watch": "nodemon -V -d 3 --watch src --watch ../Signalling/dist -e 'ts,js,mjs,cjs,json' --exec 'npm run build && node ./dist/index.js --serve --console_messages verbose --log_config --https_redirect --player_port 1025' --http_root www --max_players 1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think don't set this during watch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor changes requested.
I am also thinking we should bump signalling protocol version and add a changelog to the top of that md we have the explains the protocol to mention what was modified in each signalling version. If you agree, we may as well start now? |
Co-authored-by: Luke Bermingham <[email protected]>
…r limit on npm script.
MD we have? I've added a new changelog in the protocol folder but we cant really make manual modifications to the files under docs/ because it's all auto generated and cleaned out in the process of building the docs. |
@mcottontensor https://github.com/EpicGamesExt/PixelStreamingInfrastructure/blob/master/Signalling/docs/Protocol.md
That works for me! |
I find it slightly frustrating that the signalling messages were moved into common but we have the signalling lib as its own package. Feels like things are very separated. I don't know the best solution though. |
Hello there, I would like to ask you a question, when should I expect a new bumped version of @epicgames-ps/lib-pixelstreamingcommon-ue5.5, so I can use subscribeFailed event on my streaming page ? Thanks in advance! |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
[UE5.5] Merge pull request #427 from mcottontensor/player_limits
Relevant components:
Problem statement:
#270
The player limits on the signalling server side were removed when multi streamers came in. We would like to bring back that functionality.
Solution
This change adds --max_players to the signalling server arguments. This will limit the number of players that can connect to any streamer registered with the signalling server. For eg. with a limit of 10, any streamer connected to the signalling server will be limited to 10 players at a time. This is per streamer, not per signalling server.
The frontend has also been updated to properly signal to the user when a subscription fails like when the player limit is hit.
Documentation
Updated the usage text of the signalling server to show the new argument.
Updated the Common library documentation to document the new signalling message and usage.