-
-
Notifications
You must be signed in to change notification settings - Fork 109
#1466(make all recruiting roles recruit players to their current team) #1520
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
base: dev_2.3.0
Are you sure you want to change the base?
#1466(make all recruiting roles recruit players to their current team) #1520
Conversation
and global method to get a player's betrayal addon
wait...how do I add it to the project |
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.
PR Overview
This PR adjusts the recruiting logic so that all recruiting roles recruit players to their current team by using a new betrayal add-on assignment mechanism. Key changes include adding new using directives (e.g. for Coven and Double roles), introducing and standardizing the GetBetrayalAddon method and associated recruitment checks, and modifying role conversion logic across multiple role files (Jackal, Cultist, Godfather, Infectious, Gangster, etc.) to use the new mechanism.
Reviewed Changes
File | Description |
---|---|
Modules/CustomRolesHelper.cs | New GetBetrayalAddon method and additional using directives |
Roles/Neutral/Jackal.cs | Updated recruiting role logic and role conversion (e.g. Sheriff) |
Roles/Neutral/CursedSoul.cs | Recruitment logic now uses betrayal add-ons for soulless conversion |
Roles/Neutral/Cultist.cs | Revised charmed recruiting using the new add-on mechanism |
Roles/Impostor/Godfather.cs | Updated role conversion on dead body checks using betrayal add-ons |
Roles/Neutral/Infectious.cs | Modified biting/recruiting logic to incorporate the new add-on |
Roles/Neutral/Virus.cs | Adjustments to recruitment logic for virus role in game end checks |
Roles/Coven/Ritualist.cs | Revised conversion method to use recruitment eligibility via the new method |
Roles/Crewmate/Admirer.cs | Updated recruitment conversion and RPC syncing using add-ons |
Roles/Impostor/Gangster.cs | Updated recruitment logic to consistently use the betrayal add-on |
Patches/CheckGameEndPatch.cs | Revised winner checks to account for new role types and add-ons |
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (3)
Roles/Neutral/Jackal.cs:226
- [nitpick] Consider clarifying the naming between the 'addon' and 'role' variables here to clearly indicate when the default recruiting add-on is used versus a transformed role assignment.
if (role is CustomRoles.Sheriff)
Roles/Impostor/Gangster.cs:80
- Ensure that target.CanBeRecruitedBy(killer) correctly handles all edge cases (such as age and loyalty restrictions) so that the subsequent assignment using the 'addon' variable does not lead to unintended role conversions.
if (target.CanBeRecruitedBy(killer))
Modules/CustomRolesHelper.cs:429
- Add unit tests to cover various scenarios of the GetBetrayalAddon logic—for both recruiter and non-recruiter flows—to ensure that the default and conditional role assignments behave as expected.
public static CustomRoles GetBetrayalAddon(this PlayerControl pc, bool forRecruiter = false)
still trying to understand what CoPilot is saying |
also made it that CL gives betrayal addon on retrain
why did the conflicts come back... |
idk why the conflicts reappeared
No description provided.