[go: nahoru, domu]

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

Set max-player property on proxies too #419

Merged
merged 5 commits into from
Apr 9, 2021
Merged

Set max-player property on proxies too #419

merged 5 commits into from
Apr 9, 2021

Conversation

0utplay
Copy link
Member
@0utplay 0utplay commented Apr 7, 2021

This pull request includes:

  • breaking changes
  • no breaking changes

Changes made to the repository:

Added the Max-Players property also to proxies

@juliarn
Copy link
Contributor
juliarn commented Apr 7, 2021

This might require a bit more handling, as the max players could be changed in the proxy ping event. We handle that for servers, so it might be good for proxies too. Altough calling the ping event to get the max players is a bit weird.

@Sarsum
Copy link
Contributor
Sarsum commented Apr 7, 2021

Well, we were thinking about the Ping Event. But that would cause problems with the SyncProxy Max Players..
The initial idea was to add this, to support Proxies with the SmartModule (which is not working right now, an the max player count is always 0 in the console)

Copy link
Member
@derklaro derklaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@juliarn
Copy link
Contributor
juliarn commented Apr 7, 2021

Well, we were thinking about the Ping Event. But that would cause problems with the SyncProxy Max Players..
The initial idea was to add this, to support Proxies with the SmartModule (which is not working right now, an the max player count is always 0 in the console)

That would not cause problems with SyncProxy, it's the other way around: CloudNet would detect and use the max player amount set by SyncProxy, which is the correct behavior. The problem with this currently is, that the max players in the bungee config are mostly ignored and replaced via the ping event. If CloudNet would call that event, the same way it does on servers, everything should be fine.

@juliarn
Copy link
Contributor
juliarn commented Apr 7, 2021

What CloudNet could do too is to add an event handler with highest priority to get the max players from the event. That would work if no other plugin modifies the max players in this priority.

@Sarsum
Copy link
Contributor
Sarsum commented Apr 7, 2021

Well, we were thinking about the Ping Event. But that would cause problems with the SyncProxy Max Players..
The initial idea was to add this, to support Proxies with the SmartModule (which is not working right now, an the max player count is always 0 in the console)

That would not cause problems with SyncProxy, it's the other way around: CloudNet would detect and use the max player amount set by SyncProxy, which is the correct behavior. The problem with this currently is, that the max players in the bungee config are mostly ignored and replaced via the ping event. If CloudNet would call that event, the same way it does on servers, everything should be fine.

This would not work in the correct way with the SmartModule. When the amount of the SyncProxy Module is used (e.g. 1000) the SmartModule would use this global amount for starting a new proxy. (And not for example 100 for each Proxy)

@Sarsum
Copy link
Contributor
Sarsum commented Apr 7, 2021

I just got the following idea:
We could handle the "Max-players" like you @juliarn described and add another number to the Smart Configuration.
I would call it something like customUsedMaxPlayerAmountForCalculations (sorry, I'm bad with picking names for variables). If its -1 (default), just ignore it and use "Max-players", but if its not, use this number instead of the "Max-players".

I first thought of adding this new variable to the SyncProxy Config, but then the SmartModule would somehow depend on the SyncProxy Module.

@Sarsum
Copy link
Contributor
Sarsum commented Apr 7, 2021

Nevermind my previous idea, this would prevent the dynamic change of the max players with the API (or the max players used by the smart module)...

@derklaro derklaro changed the title Max-Player property Set max-player property on proxies too Apr 9, 2021
@derklaro derklaro merged commit 1fb9277 into CloudNetService:development Apr 9, 2021
@0utplay 0utplay deleted the development branch April 10, 2021 10:54
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.

None yet

4 participants