Player Manager fixes and useful tweaks #72
Conversation
- Changed Permissions to set Default to a "Read-Only" role - Allowed a standard players permission the ability to research - Fixed a bug where inventory would error during sync if an item/entity name was modified between updating a mod and joining the server again.
This causes confusion with server admins, where permissions were modified by the user in-game and then reset.
|
Reviewed the changes, except the permission settings change, I do agree with everything else. The reason I'm singling out permission changes is that they basically allow nothing and they require people to change the code to be able to give new users at least some rights, etc, etc. Basically, I think, the change is not suited to be in the overall mod repo. |
|
Personally, I think we should stick to as close as vanilla behaviour as possible. Public by default, enable permission groups by setting. |
|
I too think these permissions should be opt-in rather than forced upon you. The other changes looks okay. |
* RPG System (https://mods.factorio.com/mod/RPGsystem) is the first mod to support this.
Hi all,
Here is my PR for PlayerManager fixes and tweaks, i will go through what is changed below, and why imo it was done.
Note: All errors that i fixed actually happened during game play, most of these errors caused InvSync to fail outright.
Inventory sync would break if a mod was removed or an item name was altered when a player had an item from said mod in their inventory
Because the way the PlayerManager works, it just assumes that all the data it gets is correct and does not attempt to keep syncing the inventory when factorio throws an error. With my change the errors are logged to console and inventory sync still proceeds bypassing the items that no longer exist in the game.
Check for the existence of a grid, before attempting to modify it
Sometimes grids are modified or just outright removed from entities that had it before, I added a check to ensure grids exist before attempting to modify them.
Permissions are now different
The Default permissions group in my view should not allow the player to interact with the world until the admin has allowed the player to do so, this allows private servers to remain public without password. While this change is Marmite i would recommend that a configuration value could be created to define if the server is a private or public, and switch the default permission node accordingly. (Note: I changed this for my own server, it's private - which is why i explain about the Marmite side of things when Clustorio is used on public events)
Permissions should never be overwritten every time the server loads
Currently permission groups are reset to their hard coded values as defined at the top of the file every time the server starts, this in my view is wrong as if an admin were to change the permission system in game using /admin, their modifications would be reset.
Not only does this both stop confusion, but it also allows the admin to re-configure the permissions in game instead of having to go through this Lua script.