Support empty team names #381
Conversation
| @@ -183,7 +183,7 @@ export default class SquadRcon extends Rcon { | |||
| const match = line.match( | |||
| /ID: (?<squadID>\d+) \| Name: (?<squadName>.+) \| Size: (?<size>\d+) \| Locked: (?<locked>True|False) \| Creator Name: (?<creatorName>.+) \| Creator Online IDs:([^|]+)/ | |||
| ); | |||
| const matchSide = line.match(/Team ID: (\d) \((.+)\)/); | |||
| const matchSide = line.match(/Team ID: (\d) \((.*)\)/); | |||
There was a problem hiding this comment.
Do we have some log examples for this case? Would like to see this in action.
There was a problem hiding this comment.
I don't have specific log examples available at hand, but it did cause issues with Supermod where there were 3 teams: (Civilians), (), (Armed Forces). This specific entry is from the RCON, not the logs. Let me see if I can reproduce it, but we've since taken down that server.
There was a problem hiding this comment.
To be clear, this does not happen with vanilla Squad, but is possible with modded servers.
There was a problem hiding this comment.
My concern is edge cases where it may break running vanilla servers if that is a possibility.
There was a problem hiding this comment.
I don't have the exact message, but this is what I wrote down in my notes:
Team ID: 0 (Civilians)
Team ID: 1 ()
Team ID: 2 (Armed Forces)
It was incorrectly assigning players to the wrong team. Team 1 being skipped (not matching), they were incorrectly assigned to team 0.
There was a problem hiding this comment.
Oh gotcha. Would still need it to get the regex test out of the way.
There was a problem hiding this comment.
I actually remember such cases in vanilla too about a year ago. It was causing issues with BMs parsing. They maght have fixed it but I'm not sure. My intuition is this happens if ListPlayers is requested while someone has just joined and is still in Spectate state. so a way to repro would be to ask someone join the server while spamming ListPlayers requests.
There was a problem hiding this comment.
My concern is that it won't break normal use. I don't think so but would rather be safe.
There was a problem hiding this comment.
I honestly don't see how it would break it. It's a separate match. In case of current regex we would get an undefined if the field is actually empty. In case of this fix it would work in all cases. But it's worth testing if you are concerned.
There was a problem hiding this comment.
Actually, thinking about it a bit more... Since it's a separate regex that could fail matching - we'd be better off simply handling the non-matching case. Changing to a new regex from this PR would require us to check for an empty group anyways so there is literally no point in applying this change.
Some mods can have empty team names. The current regular expression to enforces at least one character in the team name. This PR removes the requirement, thus supporting empty team names instead of failing to parse the entire row.