Alts registration per server #248
Conversation
This means that now you can choose whether all servers will register alts or only the selected ones. For proxies, you need to provide the names of the servers that you have in your configs, and for backends just enable or disable a setting in config.
This means that now you can choose whether all servers will register alts or only the selected ones. For proxies, you need to provide the names of the servers that you have in your configs, and for backends just enable or disable a setting in config.
There was a problem hiding this comment.
Thanks for writing up this PR. It looks conceptually complete and contractually correct; however, there's a complicated matter that hasn't been addressed. I would hardly expect you to find it because it requires intricate knowledge of the codebase, although one could conceivably realize it by knowledge of LibertyBans' features.
The problem is that there is a dependency between adding the UUID/IP address and checking for punishments. The punishment selector assumes that the UUID and IP address have already been added, which matters for IP-based punishments. Some modes of IP address enforcement, like NORMAL and STRICT address strictness, only need the UUID of the player to check for punishments assuming the IP address is already registered in the database. That's because the NORMAL and STRICT address strictnesses do not differentiate between the player's current and past IP addresses, so they do not require the current IP address as input information, only the IP address history.
In the current PR, this phenomenon means that some IP-based punishments will not be enforced when the player joins the authentication server. Instead, they will be enforced when the player connects to the game server. We don't strictly have to resolve this matter -- we could allow this behavior and call it an edge-case due to configuring non-standard alts-registry enforcement settings. However, the feature set of LibertyBans should be refined and complete, and I would prefer to avoid unneeded confusion and explanation when this edge-case occurs.
Accounting for this matter means updating the punishment selector to properly consider the current IP address as if it were registered, but without registering it, when selecting for punishments. That's a little bit complicated in itself; more than that, there should be an optimization for servers which don't use this feature to avoid performance regression. If you're up to the challenge, you can write and implement this yourself. If not, just tell me and I will get around to implementing this aspect in my own time. While I always recommend challenging yourself, I can see how this item might require excessively niche knowledge of JOOQ and its usage in this codebase.
All that said, following are some lesser points of consideration. They're easily actionable and hopefully not tedious.
bans-core/src/main/java/space/arim/libertybans/core/selector/EnforcementConfig.java
Outdated
Show resolved
Hide resolved
bans-core/src/main/java/space/arim/libertybans/core/selector/EnforcementConfig.java
Show resolved
Hide resolved
bans-core/src/main/java/space/arim/libertybans/core/selector/EnforcementConfig.java
Outdated
Show resolved
Hide resolved
bans-core/src/main/java/space/arim/libertybans/core/selector/EnforcementConfig.java
Outdated
Show resolved
Hide resolved
bans-core/src/main/java/space/arim/libertybans/core/selector/EnforcementConfig.java
Outdated
Show resolved
Hide resolved
bans-core/src/main/java/space/arim/libertybans/core/selector/EnforcementConfig.java
Outdated
Show resolved
Hide resolved
bans-core/src/main/java/space/arim/libertybans/core/selector/Gatekeeper.java
Outdated
Show resolved
Hide resolved
bans-core/src/main/java/space/arim/libertybans/core/selector/Gatekeeper.java
Outdated
Show resolved
Hide resolved
bans-core/src/main/java/space/arim/libertybans/core/selector/InternalSelector.java
Outdated
Show resolved
Hide resolved
|
Hi! Sorry for the late response. I've been a bit busy the last months. |
Plus a couple tweaks to code style
|
Hi Snake, I've completed the heavy lifting with respect to the database queries that I described above in great detail. This means all you have to do is complete some of the lesser tasks relating to English grammar and code style, and we can merge this PR, and it will enter nonstable development builds. At a later point in time, I will need to write additional tests for the database queries introduced by my commit. However, I don't believe that should block this PR, and I will open a separate issue for that and handle it myself. Unless you want to contribute those tests, then by all means. |
| "If you are running a proxy and don't want to register the IP when players connect, ", | ||
| "set this to false and add the authentication servers' names in the list above.", | ||
| "If this is a backend server, set it to false; if it's an authentication server, set to true."}) | ||
| @ConfKey("should-register-on-connection)") |
There was a problem hiding this comment.
remove ) in should-register-on-connection)
|
is this releasing anytime soon? |
This feature aims to solve a recurrent problem that was stated in the discord server multiples times.
This problem refers to alts registration for players that haven't logged in, which means that only affects non-premium servers.
In order to solve this, I've added a way to disable alts registration per-server. This means that now you can choose which servers are going to register alts for player and which not.
Despite that, this feature hasn't been tested because I don't have the required time and resources to do it, so I'm opening this pull request in order to make more visible this new version.
So, if you are someone that needs this feature and wants to help, reach me through the libertybans discord server and I can send you the compiled version of it (you can clone this rep and compile it by yourself following the instructions).