Skip to content
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

fix antag selection being evil #28197

Merged
merged 15 commits into from
May 26, 2024

Conversation

deltanedas
Copy link
Contributor

About the PR

remove the shitcode that made it pick people IN THE LOBBY and that dont meet requirements for any listed antag role in the rule

Why / Balance

idk being a traitor in ooc sounds bad, if it even worked and didnt just crash the server
fixes #27425 and fixes #28196

Media

wyci

  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

no

Changelog
🆑

  • fix: Fixed being picked for antag roles while in the lobby or not opted in.

}

return new AntagSelectionPlayerPool(new() { preferredList, fallbackList, unwantedList, invalidList });
return new AntagSelectionPlayerPool(new() { preferredList, fallbackList });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When i was trying to fix this myself i basically did the same thing. My only consairn and why i never made a pr was cause. What happens if we got no one in the prefferedlist or fallbacklist? Are we starting the gamemode without any antags. This resulting in a broken gamerule/greenshift.

Yes i agree this should be intended behavior (#24292). But i don't think we currently have a solution to prevent broken gamerules.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok this doesnt cancel it, but nukies should be changed to make spawners if theres not enough players like in my issue
traitors zombies etc doesnt really matter as theres no unused map lying around doing nothing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Afaik it also picks people at random in game not just in the lobby (look at sleeper agents. There are reports that players get sleeper agent given to them when they never opted in)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes this fixes that since it wont pick anyone that didnt opt in

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep the unwanted list but not the invalid one at least until we have ways of setting up ghost roles and other rules properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the majority of the issue is people that arent opted in getting picked, invalid is only really picked in a dev environment. nukies making ghost roles for missing players wyci

@ElectroJr
Copy link
Contributor

You probably have to update the tests so that you can specify preferences for clients, so that they can opt into the nukie role?

VasilisThePikachu added a commit to Axolotl-MRP/axolotl-mrp-14 that referenced this pull request May 23, 2024
@ElectroJr
Copy link
Contributor

ElectroJr commented May 24, 2024

I pushed a few commits, most notably:

Allow tests to modify antag preferences
makes the nukie test change the antag preferences to opt into nuke ops

Fix antag selection
Fixes what I think was actually going wrong, because it was causing all players in the lobby to be considered ineligible for antag roles at round start. So everyone was considered ineligible, which would've made it much more likely for people to get picked with the role disabled, as usually it should've only happened on low pop servers or on a server full of pacifists. Though I still think forcing people to be antags against their will is bad so the unwanted & invalid lists should still get removed

Add AntagPreferenceTest
Adds a test to check that GetPlayerPool() can actually return valid players in the lobby.

edit:
and this should fix people in the lobby from being given mid-round antags?
Try stop players in lobbies from being assigned mid-round antags

@github-actions github-actions bot added the Status: Needs Review This PR requires new reviews before it can be merged. label May 24, 2024
@Plykiya
Copy link
Contributor

Plykiya commented May 25, 2024

not only can antag selection pick people who are just waiting in the lobby, it also has a chance to make ghosts antags as well

}

return new AntagSelectionPlayerPool(new() { preferredList, fallbackList, unwantedList, invalidList });
return new AntagSelectionPlayerPool(new() { preferredList, fallbackList });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep the unwanted list but not the invalid one at least until we have ways of setting up ghost roles and other rules properly.

@GroggleG
Copy link

Evil antag selection: I will work properly and not run like shit.
On a serious note is there actually good progress on the fix?

@ElectroJr
Copy link
Contributor

On a serious note is there actually good progress on the fix?

I think it fixes the selection bugs? but its hard to know because its pretty hard to test properly.

keep the unwanted list but not the invalid one at least until we have ways of setting up ghost roles and other rules properly.

I don't think there's any situation where you want to force someone to be an antag if they explicitly opted out of that role. That is probably just going to lead to people leaving the server.

@EmoGarbage404
Copy link
Member

I don't think there's any situation where you want to force someone to be an antag if they explicitly opted out of that role. That is probably just going to lead to people leaving the server.

realistically it doesnt happen much and i think its preferable when we dont have a system that resolves not having players for a given antag.

@deltanedas
Copy link
Contributor Author

it happens enough to have like 4 issues opened
this is especially an issue on lowpop where theres less players to begin with, much more likely to have people that arent opted in

@EmoGarbage404
Copy link
Member

it happens enough to have like 4 issues opened

that's because the selection has been bugged. that's been the old behavior for literal ages and the amount of times it happens normally is extremely low

@ElectroJr
Copy link
Contributor

realistically it doesnt happen much and i think its preferable when we dont have a system that resolves not having players for a given antag.

I'd still rather it not pick anyone than force an unwilling player that will probably just leave. Eventually the game rule system should be reworked so that at bare minimum the rule should notify any active admins somehow that it failed to select players and they can sort it out. Though ideally the rule just wouldn't even be picked.

@EmoGarbage404
Copy link
Member

fairnuff i dont care to push on it

@EmoGarbage404 EmoGarbage404 merged commit 492ccc9 into space-wizards:master May 26, 2024
11 checks passed
@deltanedas deltanedas deleted the skull-emoji-ops branch May 26, 2024 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Review This PR requires new reviews before it can be merged.
Projects
None yet
7 participants