-
-
Notifications
You must be signed in to change notification settings - Fork 79
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
Choose Antags Before Job Selection #1059
base: master
Are you sure you want to change the base?
Choose Antags Before Job Selection #1059
Conversation
…into antag-selection-rework
…into antag-selection-rework
…into antag-selection-rework
…into antag-selection-rework
…into antag-selection-rework
…nto antag-selection-rework
…into antag-selection-rework
…into antag-selection-rework
…into antag-selection-rework
…into antag-selection-rework
…into antag-selection-rework
…into antag-selection-rework
…into antag-selection-rework
…into antag-selection-rework
Alright this passes all of the applicable unit tests for game rules (except the one that enables every rule at once but that's because there's a fucked up UID in the listening post map), adding reviewers and moving to Not Draft. Then going to come back tomorrow and see if there's additional cleanup to do. Should probably add something to indicate that antags are rolled before job selection in the job select menu. That's all I can think of for now. There's like 30 out of 352 unit tests that fail for other reasons that I want to look into probably but that is outside of the scope of things for now. I'll make an issue about it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea if doing this as a review does anything over leaving these as individual comments but here's an incredibly minor thing I noticed and a potentially bigger problem
return; | ||
if (component.SelectionsComplete) // Imp edit start | ||
{ | ||
if (QueuedAntags.Count != 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should probably be > 0 for sanity, even if it never intentionally goes negative?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might as well yeah
@@ -361,6 +363,9 @@ private Dictionary<NetUserId, List<string>> GetPlayersJobCandidates(int? weight, | |||
if (!_prototypeManager.TryIndex(jobId, out var job)) | |||
continue; | |||
|
|||
if (!job.CanBeAntag && _antag.QueuedAntags.ContainsKey(player)) | |||
continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any protection for if a player only has antagimmune jobs enabled for their chosen character? or can they sometimes be dumped into passenger / held in the lobby by this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pretty sure they will just get put out into the lobby, but I also think this is handled when the antag pool is selected initially. I'll go back and verify before merging though. I vaguely remember testing this when I first wrote the initial portion a month ago, but I'd like to see it Actually Work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it be possible to, if they're selected as an antag, just...make them spawn as a passenger? missing out on the job slot i want because i only queue command/sec but don't have antags disabled sounds rough. and as we've always discussed, more people might play sec if they could still get lucky and roll antag passenger as a surprise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more people might play sec if they could still get lucky and roll antag passenger as a surprise
the natural solution to this feels like just putting passenger on 'low'
Essentially makes it so all antag gamerules choose which player will be an antag before job selection runs. It gets all added gamerules, including pending ones, and then selects based on the player pool at the start of the round. There's still a LOT to test with this and I will probably make a bunch of changes as I revisit things but this seems to be a Technically Working Prototype. I wanted to get this draft up so other code knowledge havers could take a look and tell me if there's things I'm being stupid about. I know at the very least I want to add a bit that double checks the counts and adds more antags when each rule actually starts if the ratios are off.
🆑