search

Team-Silver-Sphere / SquadJS

Squad Server Script Framework
Boost Software License 1.0
163 stars 123 forks source link

Improve migration to eosIDs #354

Closed Ulibos closed 2 months ago

Ulibos commented 5 months ago

Summary

This PR completes migration to eosIDs for the tool (except for a couple plugins, more on that below).

Dependencies

This PR depends on #352 and lays foundation for #347

Changes

There are 5 commits:

  1. Cleanup player connection log parsing to ease transition and get rid of clutter.
  2. Rework and extend admin-related methods to make them "omnivore" to different IDs. *
  3. Migrate core code to eosIDs.
  4. Migrate plugins to eosIDs. *
  5. Fix minor issue with uninitialized oldPlayerInfo on player list updates (was annoying me a bit during testing).

* - caveats

Testing

All of this was tested several times with event capture to make sure data is consistent. In summary (but not in 1 last go) I have tested:

connecting, disconnecting, possess, unpossess, create squad, kill (self inflicted), warn, force swap, kick, ban.

Only thing I didn't test for sure is a teamkill. I also am not aware if I had at least 1 run where I didn't have a steamID, most likely not. But so far generalized parsers work correctly, I haven't seen any errors in logs or missing data. I have also tested: getAdminPermsByAnyID(), getAdminsWithPermission() (both were tested for server.admins with steamID or eosID), getPlayerByAnyID() for both types of IDs, rcon methods (warn(), switchTeam(), kick(), ban()).

Caveats

Testing Plugins

I didn't test plugins. We don't use any default ones and we don't have the necessary setup, plus the ones I've edited have mostly trivial fixes. The one I've fixed only partially is discord-killfeed, I didn't touch it's CBL integration. The ones I didn't touch at all are: db-log, awn-api, cbl-info. Some of these are completely unknown to me, some are too dependent on steamID to fix them in this PR. For now they will work as they were before as long as players have steamIDs.

Admins IDs

For now, there is a potential issue with an admin having 2 sets of permissions, one for eosID and another for steamID. Current code won't be able to merge it (at the time of merging it doesn't have persistent mapping of player's IDs). Easily avoidable though by not intermixing two ID formats. Another side-effect is that getAdminsWithPermission() will always return only those admins that have the permission AND are online (for at least 10 seconds). This happens because this method matches all existing IDs against all existing admin entries to convert them all to eosIDs.

fantinodavide commented 5 months ago

Additionally, the "Fix/" prefix is not appropriate in my opinion, and I would change it to "refactor!: " (see https://www.conventionalcommits.org/en/v1.0.0/)

werewolfboy13 commented 5 months ago

Flags added as needed. Let me know when good to push.

Ulibos commented 5 months ago

Status update here: https://github.com/Team-Silver-Sphere/SquadJS/pull/354#discussion_r1566462582 Sorry for bad placement, my github kung-fu is still weak.

fantinodavide commented 3 months ago

New changes look good overall. The error handling of the new getAdminsWithPermission is very clear and easy to understand.

Ulibos commented 3 months ago

I have closed #352 because it is already integrated into this PR so no unsatisfied dependencies left. #347 can be merged in after this PR.