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

admin2: automatic scripts #259

Closed
wants to merge 2 commits into from
Closed

admin2: automatic scripts #259

wants to merge 2 commits into from

Conversation

stoneage-mta
Copy link
Contributor

This pull request closes #102.

@jlillis
Copy link
Contributor

jlillis commented Dec 6, 2020

I have a few concerns with this implementation:

  1. we are relying on the client to perform the checks. A malicious client could simply not perform the checks (i.e fail to load the script). I recommend performing these checks server-side and just polling the client for needed information (i.e fps, idletime - ping can be determined serverside with getPlayerPing.
  2. values for minimum fps and maximum idletime and ping should be used as average values rather than absolute values - this will allow for some wiggle-room in the case of temporary network/pc disruptions. The degree of wiggle-room should be configurable via settings. Say we want to kick players who average under 15fps for 60 seconds: minimumFPS = 15fps, period = 60seconds.
  3. the term "automatic scripts" seems vague to me. I would recommend splitting this into two different scripts: "latency checks" for fps/ping, and "idle timeout" for idletime.

@stoneage-mta
Copy link
Contributor Author

Lol, sorry for the delay, I totally forgot about this PR and also thought I had already left an answer.

I think another PR is more reasonable than applying these changes since a lot of code needs to be changed. If no one picks it, maybe another day I do it again, following your ideas.

@stoneage-mta stoneage-mta deleted the admin-automatic-scripts branch July 4, 2021 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

admin2: automatic scripts
4 participants