обновление whitelist логики в ui, обновление требований у цк ролей #933
Conversation
WalkthroughThe pull request introduces significant modifications across several classes and configuration files related to ghost roles and job management. Key changes include the addition of a Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
S: Untriaged
Changes: Localization
github-actions
bot
added
the
Changes: Map
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (10)
Content.Server/Backmen/Reinforcement/ReinforcementSystem.cs (1)
226-243: Refactor repeated permission checks into a helper methodWithin the
OnTakeoverTakeRolemethod, there are several permission checks that follow a similar pattern: checking a condition, settingent.Comp.Used = false;, sending a chat message usingSendChatMsg, and returning. Consider refactoring these into a helper method to reduce code duplication and improve maintainability.Content.Server/Ghost/Roles/GhostRoleSystem.cs (1)
58-59: Consider renaming_wlRoleto_jobWhitelistManagerfor clarityThe field
_wlRolemight be more clearly named_jobWhitelistManagerto improve code readability and maintainability.Resources/Locale/ru-RU/backmen/playtime.ftl (1)
2-2: LGTM with a minor suggestion for consistency.The new localization string aligns well with the PR objectives and maintains consistent formatting with existing messages.
Consider adding a period at the end of the message to match the punctuation style of other messages:
-playtime-deny-reason-discord = Вам нужно пройти [color=yellow]Discord[/color] авторизацию, чтобы взять эту роль +playtime-deny-reason-discord = Вам нужно пройти [color=yellow]Discord[/color] авторизацию, чтобы взять эту роль.Resources/Prototypes/_Backmen/Loadouts/Jobs/CentCom/BKCCCargo.yml (1)
Line range hint
1-7: Remove deprecatedCCCargoconfigurationThe old
CCCargostarting gear configuration should be removed since it's being replaced byCCCargoFull.Apply this diff:
-- type: startingGear - id: CCCargo - equipment: - id: CCCargoPDA - ears: ClothingHeadsetCentCom - pocket1: AppraisalTool - shoes: ClothingShoesColorBlack - head: ClothingHeadHatCargosoftResources/Prototypes/_Backmen/CentComm/BKCCOperator.yml (1)
Line range hint
7-21: Add Discord requirement to GhostRole componentAccording to the PR objectives, all CC roles should require Discord authorization. The GhostRole component should include this requirement.
Add the Discord requirement to the GhostRole component:
requirements: - !type:RoleTimeRequirement role: JobCentralCommandAssistant time: 7200 - !type:DepartmentTimeRequirement department: CentCom time: 14400 - !type:DepartmentTimeRequirement department: Silicons time: 6000 + - !type:DiscordRequirement job: BKCCOperator whitelistRequired: trueResources/Prototypes/_Backmen/CentComm/BKCCCargo.yml (1)
44-44: Add a meaningful description for the entity.The empty description string should be replaced with a proper description, similar to how SpawnPointCMBKCCCargo has a detailed description of the role's responsibilities.
- description: "" + description: "A Central Command Cargo Officer responsible for managing cargo operations and coordinating with the station's cargo department."Resources/Prototypes/_Backmen/CentComm/BKCCAssistant.yml (2)
Line range hint
1-38: Add Discord authorization requirementAccording to the PR objectives, all ЦК roles should require Discord authorization. The
SpawnPointCMBKCCAssistantentity'sGhostRolecomponent should include this requirement.Add the Discord requirement to align with the PR objectives:
- type: GhostRole allowMovement: true allowSpeech: true makeSentient: true name: Ассистент ЦК description: Разнорабочий призванный для поддержки экипажа ЦК и для поддержки самой станции цк в текущем секторе. rules: Вы обязаны выполнять поручения экипажа станции цк, опеспечивать станцию цк в рабочем состоянии, также запрещено покидать станцию цк! raffle: settings: short requirements: + - !type:DiscordRequirement - !type:DepartmentTimeRequirement
42-42: Add a meaningful descriptionAn empty description string might affect documentation and UI displays. Consider adding a descriptive text that explains the role's purpose and responsibilities.
- description: "" + description: "Разнорабочий призванный для поддержки экипажа ЦК и для поддержки самой станции цк в текущем секторе."Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/intern.yml (1)
Armed loadout for intern role is inconsistent with role expectations and security hierarchy
Based on the comparison with other CentCom roles, the intern's armed loadout (including Mosin sniper rifle, combat knife, and basic armor) is inappropriate for several reasons:
- Security equipment hierarchy: CentCom security roles (Private Officer, Head of Security) are properly equipped with role-appropriate gear matching their security responsibilities.
- Role expectations: The intern is an administrative/assistant role, yet has access to high-powered weapons without proper security clearance or training implications.
- Security risk: The loadout provides significant combat capabilities to what should be an entry-level position.
Recommendations:
- Remove combat weapons (sniper rifle, combat knife) from intern loadout
- Limit equipment to administrative tools and basic protective gear
- If armed variant is necessary, document specific use cases and add access restrictions
🔗 Analysis chain
Line range hint
67-82: Security Review: Armed Gear LoadoutThe
CentComInternGearArmedconfiguration provides significant combat equipment including:
- Mosin sniper rifle
- Combat knife
- Basic armor
- Ammunition
- Medical supplies
Please verify if this level of equipment is appropriate for an intern role, as it could pose security risks if the role falls into the wrong hands.
Let's check if other intern/assistant roles have similar armed variants:
Consider:
- Adding role-specific restrictions or conditions for accessing the armed variant
- Documenting when/how this loadout should be used
- Adding additional security checks or logging when this loadout is assigned
Let me check the full context of this armed loadout and compare it with other CentCom roles:
Let me modify the search pattern to avoid using look-around assertions:
Let me check if there are any other CentCom roles to compare their equipment levels:
Let me check the equipment loadouts of other high-security CentCom roles for comparison:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other armed gear configurations for intern/assistant roles # Expected: Understand if this is a common pattern or an outlier rg -U "type: startingGear\n.*[Ii]ntern.*[Aa]rmed[\s\S]*?equipment:" "Resources/Prototypes/"Length of output: 355
Script:
#!/bin/bash # Get full context of the intern armed loadout rg -U "CentComInternGearArmed[\s\S]*?(?=\n\n|\z)" Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/intern.yml # Check other CentCom roles' armed loadouts for comparison rg -U "type: startingGear\n.*CentCom.*[Aa]rmed[\s\S]*?(?=\n\n|\z)" Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/Length of output: 814
Script:
#!/bin/bash # Get full context of the intern armed loadout and other CentCom armed loadouts rg -U "type: startingGear\n.*CentCom.*[Aa]rmed.*\n.*equipment:.*(\n.*)*" Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/ # Also check for any role restrictions or conditions rg "restrict|condition|access" Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/intern.ymlLength of output: 4831
Script:
#!/bin/bash # List all CentCom role files fd -e yml -e yaml "." Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/ # Check for other armed variants in CentCom roles rg -l "type: startingGear" Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/ | xargs rg -l "[Aa]rmed"Length of output: 762
Script:
#!/bin/bash # Check equipment loadouts of other CentCom roles, particularly focusing on security-related ones rg -U "equipment:.*(\n.*)*" Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/operator.yml Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/blueshield.yml Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/head_of_security.yml Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/private_officer.ymlLength of output: 13470
Content.Client/UserInterface/Systems/Ghost/Controls/Roles/GhostRolesEui.cs (1)
113-122: Consider adding guidance for users when access is denied.While the access control logic is well-structured, users might benefit from additional guidance when their access is denied. Consider:
- Adding tooltips or help text explaining how to get whitelisted
- Including instructions for Discord authorization
- Adding links to relevant documentation or Discord server
Example improvement:
if (group.Key.WhitelistRequired) { hasAccess = false; - reason = FormattedMessage.FromMarkupOrThrow(Loc.GetString("playtime-deny-reason-not-whitelisted")); + reason = FormattedMessage.FromMarkupOrThrow(Loc.GetString("playtime-deny-reason-not-whitelisted") + "\n" + + Loc.GetString("whitelist-instructions")); } else if (group.Key.DiscordRequired) { hasAccess = false; - reason = FormattedMessage.FromMarkupOrThrow(Loc.GetString("playtime-deny-reason-discord")); + reason = FormattedMessage.FromMarkupOrThrow(Loc.GetString("playtime-deny-reason-discord") + "\n" + + Loc.GetString("discord-auth-instructions")); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (17)
Content.Client/UserInterface/Systems/Ghost/Controls/Roles/GhostRolesEui.cs(3 hunks)Content.Server/Backmen/Reinforcement/ReinforcementSystem.cs(6 hunks)Content.Server/Ghost/Roles/GhostRoleSystem.cs(4 hunks)Content.Shared/Ghost/Roles/GhostRolesEuiMessages.cs(1 hunks)Resources/Locale/en-US/backmen/playtime.ftl(1 hunks)Resources/Locale/ru-RU/backmen/playtime.ftl(1 hunks)Resources/Maps/centcomm.yml(0 hunks)Resources/Prototypes/_Backmen/CentComm/BKCCAssistant.yml(1 hunks)Resources/Prototypes/_Backmen/CentComm/BKCCCargo.yml(2 hunks)Resources/Prototypes/_Backmen/CentComm/BKCCOperator.yml(1 hunks)Resources/Prototypes/_Backmen/Loadouts/Jobs/CentCom/BKCCCargo.yml(1 hunks)Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/cargotech.yml(1 hunks)Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/centcom_official.yml(1 hunks)Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/head_of_security.yml(1 hunks)Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/intern.yml(1 hunks)Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/operator.yml(1 hunks)Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/private_officer.yml(1 hunks)
🔥 Files not summarized due to errors (1)
- Resources/Maps/centcomm.yml: Error: Server error: no LLM provider could handle the message
💤 Files with no reviewable changes (1)
- Resources/Maps/centcomm.yml
✅ Files skipped from review due to trivial changes (1)
- Resources/Locale/en-US/backmen/playtime.ftl
🔇 Additional comments (27)
Content.Server/Backmen/Reinforcement/ReinforcementSystem.cs (3)
159-160: Character generation now uses player's profile with random name
The change to use the player's character profile and assign a random name based on species and gender aligns with the PR objectives to use the player's character but with a randomly generated name.
419-429: Verify consistency in user notification methods
The SendChatMsg method sends both a popup and a chat message to the player. Verify that this approach is consistent with the intended user experience and existing notification patterns in the codebase to prevent duplicate or overwhelming notifications.
Run the following script to check how user notifications are typically handled in similar contexts:
✅ Verification successful
Based on the search results, I can now generate the final response:
Sending both popup and chat message is appropriate for critical notifications
The SendChatMsg method's approach of using both popup and chat message is consistent with the codebase's notification patterns for important system messages. This dual-notification approach is specifically used in scenarios where:
- Critical information needs player attention (reinforcement system messages, ghost rejoin notifications)
- System state changes that affect gameplay (economy notifications, oracle messages)
- Important status updates (cryostorage messages)
The implementation matches other critical notification patterns in the codebase, particularly in:
ReinforcementSystem.csGhostReJoinSystem.csOracleSystem.cs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the standard patterns for user notifications
# Search for methods where both _popup.PopupCursor and _chatManager.ChatMessageToOne are used together
rg '_popup\.PopupCursor.*\n.*_chatManager\.ChatMessageToOne' -A 5
# Search for methods that use only one of these notifications
rg '_popup\.PopupCursor' -A 5
rg '_chatManager\.ChatMessageToOne' -A 5
Length of output: 43489
320-322: Ensure ghost role whitelisting functions correctly after changes
By setting ghost.JobProto = job.ID; and commenting out ghost.WhitelistRequired = job.Whitelisted;, verify that the ghost role whitelisting still functions as intended, as the job whitelist is now checked elsewhere in the code.
Run the following script to find usages of ghost.WhitelistRequired and confirm that removing this assignment does not impact whitelisting:
✅ Verification successful
Let me gather more information about the whitelisting functionality to understand where it's being checked now.
Let me check one more thing to understand how the whitelisting is handled in the reinforcement system.
Commenting out ghost.WhitelistRequired is safe as whitelisting is properly handled elsewhere
The whitelisting check has been moved to a more appropriate location. The ReinforcementSystem now uses _jobWhitelistManager.IsAllowed() to verify whitelist requirements before allowing role assignment, making the ghost.WhitelistRequired property redundant. This is evidenced by:
- Whitelist verification happens in
ReinforcementSystem.csusing_jobWhitelistManager.IsAllowed(args.Player, proto.Job) - Job whitelist checks are also implemented in
StationJobsSystemandJobWhitelistSystem - The system uses proper dependency injection with
JobWhitelistManagerand maintains whitelist state in_whitelistscollection
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find usages of ghost.WhitelistRequired
# Search for assignments and usages of WhitelistRequired property in GhostRoleComponent
rg 'ghost\.WhitelistRequired' -A 5
Length of output: 505
Script:
#!/bin/bash
# Search for whitelisting related code in the Reinforcement system
rg -i 'whitelist' Content.Server/Backmen/Reinforcement/ReinforcementSystem.cs -A 5
# Search for job whitelist checks in the codebase
ast-grep --pattern 'job.Whitelisted'
Length of output: 1377
Script:
#!/bin/bash
# Check JobWhitelistManager implementation to understand how IsAllowed works
rg -A 10 'class JobWhitelistManager' Content.Server/Players/JobWhitelist/
# Check WhitelistSystem implementation
rg -A 10 'class WhitelistSystem' Content.Server/Backmen/RoleWhitelist/
Length of output: 2389
Content.Server/Ghost/Roles/GhostRoleSystem.cs (2)
584-603: Logic for whitelist and Discord requirements is correctly implemented
The conditional checks for job whitelisting and Discord authentication are properly implemented, ensuring that only authorized players can access certain ghost roles.
615-616: WhitelistRequired and DiscordRequired properties are appropriately assigned
The properties WhitelistRequired and DiscordRequired are correctly set on the GhostRoleInfo object, which will facilitate proper role access control in the UI.
Resources/Prototypes/_Backmen/Loadouts/Jobs/CentCom/BKCCCargo.yml (2)
Line range hint 23-71: Well-structured loadout definitions
The existing loadout definitions align well with the new CCCargoFull configuration, providing good modularity and flexibility for equipment customization.
10-20: Verify new equipment prototypes
The new configuration adds additional equipment items. Please ensure that these prototypes are properly defined:
ClothingUniformJumpsuitCargoClothingNeckCloakCapClothingBackpack
✅ Verification successful
Based on the search results, I can generate the final response as we have found all the necessary prototype definitions:
All equipment prototypes are properly defined
The verification confirms that all required prototypes exist in the codebase:
ClothingUniformJumpsuitCargoin Resources/Prototypes/Entities/Clothing/Uniforms/jumpsuits.ymlClothingNeckCloakCapin Resources/Prototypes/Entities/Clothing/Neck/cloaks.ymlClothingBackpackin Resources/Prototypes/Entities/Clothing/Back/backpacks.yml
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the new equipment prototypes are defined
# Search for prototype definitions in Resources/Prototypes
rg -t yaml "id: (ClothingUniformJumpsuitCargo|ClothingNeckCloakCap|ClothingBackpack)" Resources/Prototypes/
Length of output: 19912
Resources/Prototypes/_Backmen/CentComm/BKCCOperator.yml (2)
41-41: Verify text formatting consistency
The period has been removed from the description. Ensure this aligns with the text formatting guidelines for entity descriptions.
Line range hint 58-59: Confirm random name generation aligns with PR objectives
The PR mentions "console for calling officials has been modified to use the player's character but with a randomly generated name". The RandomHumanoidAppearance component with randomizeName: true appears to implement this requirement.
✅ Verification successful
The previous ast-grep pattern didn't work well with YAML, so let's try with ripgrep and also look for similar CC-related files to understand the context better.
Random name generation is consistently implemented across CentComm roles
The search results confirm that all CentComm roles in the Resources/Prototypes/_Backmen/CentComm/ directory consistently use RandomHumanoidAppearance with randomizeName: true:
- BKCCOperator.yml
- BKCCCargo.yml
- BKCCSecure.yml
- BKCCAssistant.yml
This implementation aligns with the PR's objective to use randomly generated names for CentComm officials, and the configuration is uniform across all related roles.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify similar configuration in other CC roles
ast-grep --pattern 'RandomHumanoidAppearance:
randomizeName: true'
Length of output: 70
Script:
#!/bin/bash
# Search for RandomHumanoidAppearance component configurations in YAML files
rg -U "RandomHumanoidAppearance[\s\S]*?randomizeName" --type yaml
# Also check for similar files in the CentComm directory
fd -e yml -e yaml "CC.*\.yml" Resources/Prototypes
Length of output: 26853
Resources/Prototypes/_Backmen/CentComm/BKCCCargo.yml (1)
59-59: Verify the CCCargoFull loadout prototype.
The loadout has been changed from CCCargo to CCCargoFull. Let's verify that this new prototype exists and contains the intended equipment changes.
✅ Verification successful
The CCCargoFull prototype exists and contains the expected equipment loadout
The prototype change from CCCargo to CCCargoFull is valid. The CCCargoFull prototype is properly defined in Resources/Prototypes/_Backmen/Loadouts/Jobs/CentCom/BKCCCargo.yml and includes all the base equipment from CCCargo plus additional items:
jumpsuit: ClothingUniformJumpsuitCargoneck: ClothingNeckCloakCapback: ClothingBackpack
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the CCCargoFull loadout prototype exists and check its contents
# Search for the CCCargoFull prototype definition
echo "Searching for CCCargoFull prototype definition:"
rg -A 10 "CCCargoFull:" Resources/Prototypes/
# Also search for any references to ensure it's properly used
echo -e "\nSearching for CCCargoFull references:"
rg "CCCargoFull"
Length of output: 425
Script:
#!/bin/bash
# Let's check the contents of the CCCargoFull loadout prototype
echo "Contents of BKCCCargo.yml file:"
cat Resources/Prototypes/_Backmen/Loadouts/Jobs/CentCom/BKCCCargo.yml
Length of output: 1408
Resources/Prototypes/_Backmen/CentComm/BKCCAssistant.yml (1)
Line range hint 44-60: LGTM! Component configuration looks good
The new component configuration properly sets up the Central Command Assistant role with:
- Appropriate security implants
- Correct faction membership
- Random name generation (aligning with PR objectives)
- Required protections and permissions
Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/operator.yml (2)
15-15: LGTM! Verify consistency across other Central Command roles.
The addition of discordRequired: true aligns with the PR objective to require Discord authorization for all ЦК roles.
Let's verify that this requirement is consistently applied across all Central Command roles:
✅ Verification successful
Discord requirement is consistently implemented across all Central Command roles
The verification confirms that discordRequired: true is properly set for all Central Command roles:
- intern.yml
- head_of_security.yml
- cargotech.yml
- private_officer.yml
- operator.yml
- centcom_official.yml
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all Central Command role configurations have the Discord requirement
# Search for all Central Command job definitions and check for discordRequired
rg -U "type: job\n.*CC.*\n[\s\S]*?discordRequired:" "Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/"
Length of output: 8169
15-15: Verify the Discord requirement implementation and error handling.
Since this is a new authorization requirement, we should ensure proper implementation of the Discord verification logic and error handling.
Let's check the implementation:
✅ Verification successful
Based on the search results, I can now provide a verified response about the Discord requirement implementation:
Discord requirement is properly implemented with error handling and verification
The codebase shows comprehensive implementation of Discord verification:
- Client-side verification in
JobRequirementsManager.cschecks the requirement and provides localized error messages - Server-side handling in
GhostRoleSystem.csandDiscordAuthManager.csenforces the requirement - Error messages are properly localized through the
role-required-discordstring - The feature integrates with both job selection and ghost role systems
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the Discord requirement implementation and error handling
# Search for Discord requirement checks in job assignment logic
ast-grep --pattern 'if ($_.discordRequired) {
$$$
}'
# Search for related error messages or localization strings
rg "discord.*required" "Resources/Locale/"
Length of output: 56
Script:
#!/bin/bash
# Let's try different approaches to find Discord-related implementations
# Search for Discord-related code patterns
rg -i "discord.*required" -A 5
# Search for files containing "discord" to understand the implementation context
fd -i discord
# Look for job requirement checks
rg -i "job.*require" -A 5
Length of output: 107024
Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/cargotech.yml (3)
14-14: LGTM: Discord requirement addition aligns with PR objectives
The addition of discordRequired: true correctly implements the requirement for Discord authorization for CentCom roles.
Line range hint 67-78: Verify sprite assets existence
Please ensure that the following sprite assets exist:
/Textures/Backmen/Icons/jobicon.rsiBackmen/Objects/Devices/pda.rsiwith statepda-centcomcargo
✅ Verification successful
All sprite assets verified successfully
The verification confirms that both required sprite assets exist and are properly configured:
Resources/Textures/Backmen/Icons/jobicon.rsiis presentResources/Textures/Backmen/Objects/Devices/pda.rsiis present and contains the required statepda-centcomcargowith animation delays configured
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the required sprite assets exist
fd -e rsi 'jobicon.rsi$' Resources/Textures/Backmen/Icons
fd -e rsi 'pda.rsi$' Resources/Textures/Backmen/Objects/Devices
# If found, verify the states
if [ -d "Resources/Textures/Backmen/Objects/Devices/pda.rsi" ]; then
cat "Resources/Textures/Backmen/Objects/Devices/pda.rsi/meta.json" | jq '.states[] | select(.name == "pda-centcomcargo")'
fi
Length of output: 516
Line range hint 80-97: Review inheritance from CentcomIDCardDeathsquad
The ID card inherits from CentcomIDCardDeathsquad which seems overly privileged for a cargo role. Consider creating a more appropriate base class for CentCom civilian roles.
Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/centcom_official.yml (3)
15-15: LGTM: Discord requirement aligns with PR objectives.
The addition of discordRequired: true aligns with the PR objective to require Discord authorization for all Central Command roles. This adds an extra layer of verification for this high-privilege role.
Line range hint 42-45: Verify the wage and bank balance ranges.
The role has:
- Minimum bank balance: 400
- Maximum bank balance: 600
- Wage: 75 (CentCom department)
Please confirm if these monetary values are balanced relative to other roles.
✅ Verification successful
The monetary values for CentCom Official role are balanced relative to other roles
Looking at the wage and bank balance data across all roles:
- Command roles (Captain, HoP, etc.) have 400-600 balance range and 100 wage
- Most regular roles have 150-250 balance range and 50 wage
- CentCom roles consistently have:
- 400-600 balance range for senior positions (Official, Admiral, HoS)
- 100-250 balance range for junior positions (Intern, Private Officer)
- 75 wage across most positions
The CentCom Official's values (400-600 balance, 75 wage) align perfectly with other senior CentCom positions and are appropriately positioned between regular roles and Command roles.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check wage and bank balance ranges across different roles
# Search for wage definitions
echo "Wage definitions across roles:"
rg "wage:|wageDepartment:|minBankBalance:|maxBankBalance:" "Resources/Prototypes"
Length of output: 16342
Line range hint 19-41: Verify the extensive privileges granted to this role.
This role has significant privileges including:
- Full access through
AllAccessgroup - Special abilities (SpecForce, Psionic, DispelPower, AntagImmune)
- Mind shield implant
Please verify if all these privileges are necessary and appropriate for this role.
✅ Verification successful
Let me analyze the specific privileges by checking the content of these files to understand the context better.
Let me try a different approach to examine the role privileges.
Based on the gathered information, I can now provide a final response:
The extensive privileges granted to the CentCom Official role are appropriate and consistent with its position
The role's privileges align with its high-ranking position within Central Command, as evidenced by:
- The role requires admin notification, whitelist, and Discord verification
- It has department time requirements for CentCom
- The privileges are part of a structured hierarchy with specific access levels (CCCargo, CCSecurity, CentralCommand, CCOfficer)
- The special components (SpecForce, Psionic, DispelPower, AntagImmune) are security measures appropriate for a high-ranking CentCom official
- The role includes standard security measures like the MindShieldImplant
The role's configuration shows careful access control with multiple verification layers, making these privileges justified for its intended function.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other roles with similar privileges to ensure consistency
# Search for roles with AllAccess
echo "Roles with AllAccess:"
rg -l "AllAccess" "Resources/Prototypes"
# Search for roles with similar special components
echo "\nRoles with SpecForce or Psionic components:"
rg -l "SpecForce|Psionic" "Resources/Prototypes"
Length of output: 5985
Script:
#!/bin/bash
# Check the content of CentCom roles to compare privileges
echo "=== CentCom Official privileges ==="
rg -A 10 "CentCom/centcom_official.yml" Resources/Prototypes
echo "\n=== CentCom Admiral privileges ==="
rg -A 10 "CentCom/centcom_admiral.yml" Resources/Prototypes
echo "\n=== Operator privileges ==="
rg -A 10 "CentCom/operator.yml" Resources/Prototypes
# Check if there are any role-specific restrictions or requirements
echo "\n=== Special requirements or restrictions for CentCom roles ==="
rg "CentCom.*special:" Resources/Prototypes
Length of output: 591
Script:
#!/bin/bash
# Use cat to directly read the content of relevant files
echo "=== CentCom Official (Backmen) ==="
cat "Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/centcom_official.yml"
echo -e "\n=== CentCom Official (Main) ==="
cat "Resources/Prototypes/Roles/Jobs/CentComm/official.yml"
echo -e "\n=== Looking for other CentCom roles with similar components ==="
# Search for other roles with similar component combinations
rg -C 5 "components:.*SpecForce.*Psionic.*DispelPower.*AntagImmune" "Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/"
Length of output: 3948
Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/intern.yml (1)
14-14: LGTM: Discord requirement aligns with PR objectives.
The addition of discordRequired: true correctly implements the requirement for Discord authorization for CentCom roles as specified in the PR objectives.
Let's verify that this requirement is consistently applied across all CentCom roles:
Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/private_officer.yml (2)
Line range hint 41-77: Verify if equipment changes are needed for this role
The PR mentions equipment changes for cargo CentCom roles, but this file defines a security officer role. While the current equipment configuration looks appropriate for a security role (combat gear, weapons, security access), please verify if any equipment adjustments were intended for this role as well.
Let's check if there are any pending equipment changes for this role:
✅ Verification successful
No equipment changes needed for CentCom Private Officer role
After comparing the equipment loadouts across different CentCom security roles (Private Officer, Head of Security, and Blueshield) and checking the cargo-related changes, the current Private Officer equipment is appropriate and consistent with its security role. The equipment includes standard CentCom combat gear, weapons, and security-specific items that align with the role's responsibilities. The PR's cargo-related changes don't affect this security role's equipment needs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Look for related equipment changes in other commits or discussions
# Expected: Find any mentions of equipment changes for CentCom security roles
# Search for equipment-related changes in CentCom configurations
rg -g "*.yml" "CentComPrivateOfficerGear" Resources/Prototypes/
Length of output: 373
Script:
#!/bin/bash
# Let's check the content of BKCCSecure.yml to understand the context
cat Resources/Prototypes/_Backmen/CentComm/BKCCSecure.yml
# Also check for any other CentCom security gear patterns
rg -g "*.yml" "CentCom.*(?i)security|(?i)security.*CentCom" Resources/Prototypes/
# Look for any recent changes or related equipment in the same directory
ls -la Resources/Prototypes/_Backmen/CentComm/
Length of output: 3066
Script:
#!/bin/bash
# Let's check the head of security gear for comparison
cat Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/head_of_security.yml
# And check the blueshield gear
cat Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/blueshield.yml
# Also check if there are any recent changes to cargo roles' equipment
cat Resources/Prototypes/_Backmen/CentComm/BKCCCargo.yml
Length of output: 6505
Line range hint 16-39: Verify role requirements and permissions
The role has significant requirements and permissions:
- 108000 time units in Security department
- 3600 time units as CentCom Assistant
- Access to sensitive areas (CentralCommand, Armory, etc.)
- Special abilities (mind shield, psionic bonus)
Please confirm these configurations align with the intended role balance.
Let's check for consistency with other similar roles:
✅ Verification successful
Requirements and permissions are consistent with role hierarchy
Based on the verification results, the private officer's requirements and permissions align with other CentCom roles:
- The 108,000 time requirement for Security department is comparable to other specialized CentCom roles (e.g., intern requires 108,000 in their respective departments)
- The 3,600 time requirement as CentCom Assistant is actually lower than other CentCom roles (operator requires 7,200, HoS requires 21,600)
- The access permissions and special abilities (mind shield, SpecForce component) are consistent with the security-focused nature of the role
- The configuration follows the same pattern as other CentCom security roles like blueshield and head of security
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Compare requirements and permissions across CentCom security roles
# Expected: Find similar patterns in requirements and access levels
# Search for department time requirements in CentCom roles
rg -U "type: job\n.*BKCC.*\n[^-]*requirements:" Resources/Prototypes/
# Search for access permissions in CentCom roles
rg -U "type: job\n.*BKCC.*\n[^-]*access:" Resources/Prototypes/
Length of output: 137
Script:
#!/bin/bash
# Let's try a broader search for CentCom roles and their configurations
# Search for files containing CentCom roles
fd "\.yml$" Resources/Prototypes/ --exec rg -l "CentralCommand|CCSecurity" {}
# Then let's examine the specific requirements and access patterns
rg -U "requirements:(?s).*?access:" Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/
# Also search for similar security roles
rg -U "department: Security.*?time: \d+" Resources/Prototypes/
Length of output: 12487
Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/head_of_security.yml (2)
15-15: LGTM: Discord requirement addition aligns with PR objectives.
The addition of discordRequired: true is consistent with the PR's goal of requiring Discord authorization for all ЦК roles.
Line range hint 1-90: Verify if equipment adjustments are needed for this role.
While the PR mentions equipment adjustments for cargo ЦК roles, please verify if any equipment changes are also needed for the Head of Security role to maintain consistency with the broader changes.
✅ Verification successful
No equipment adjustments needed for CentCom Head of Security role
The current equipment setup for the Head of Security role is consistent with other CentCom security/military roles. The role uses standard CentCom military equipment including:
ClothingUniformJumpsuitCentcomMilitary- Standard military uniformClothingOuterArmorCentcom- Standard CentCom armorClothingCentcomBackpackandClothingShoesBootsCentcom- Standard CentCom gear
The equipment aligns with the Private Officer role which uses similar protective gear, and maintains appropriate distinction from civilian CentCom roles (Official, Intern) which use different uniform variants. No adjustments are needed to maintain consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for equipment changes in other CentCom roles to ensure consistency
# Search for equipment changes in other CentCom role definitions
rg -A 5 "ClothingUniformJumpsuitCentcom|ClothingCentcomBackpack|ClothingShoesBootsCentcom" Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/
# Search for any modified CentCom gear definitions in this PR
rg "type: startingGear.*CentCom" Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/
Length of output: 3777
Script:
#!/bin/bash
# Let's check for any recent changes to CentCom gear and related equipment
# Search for all starting gear definitions in CentCom roles
rg "type: startingGear" Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/ -B 1 -A 10
# Search for any cargo-related CentCom roles and their equipment
rg "department: CentCom.*Cargo|Cargo.*department: CentCom" Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/ -A 20
# Check for any other CentCom military/security uniforms
rg "ClothingUniformJumpsuitCentcomMilitary|ClothingOuterArmorCentcom" Resources/Prototypes/
Length of output: 7865
Content.Shared/Ghost/Roles/GhostRolesEuiMessages.cs (1)
15-15: Consider updating the comment to be more specific.
The implementation looks good, but the comment // backmen: Whitelist could be more specific since this property is specifically for Discord authorization requirements.
- public bool DiscordRequired { get; set; } // backmen: Whitelist
+ public bool DiscordRequired { get; set; } // Requires Discord authorizationLet's verify the implementation of this property in related files:
✅ Verification successful
Property implementation is correctly integrated across the codebase
The DiscordRequired property is properly implemented and used throughout the codebase:
- Server-side enforcement in
GhostRoleSystem.cs - Client-side UI checks in
GhostRolesEui.csandJobRequirementsManager.cs - Proper configuration in job prototypes under
Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/ - Integration with antag roles in
Resources/Prototypes/_Backmen/Roles/Antags/
The comment style is consistent with other similar properties in the codebase (e.g., // backmen: antag, // backmen: whitelist), so the current comment can remain as is.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check how DiscordRequired is used across the codebase
# Expected: Find usage in GhostRoleSystem for enforcement and in UI code for display
# Check for property usage in ghost role system
rg -A 5 "DiscordRequired" --type cs
# Check for Discord-related localization strings that might be used for denial messages
rg "discord" Resources/Locale/ --type ftl
Length of output: 5341
Script:
#!/bin/bash
# Let's check for localization strings in a different way
rg "discord" Resources/ --type yaml
# Let's also check for any UI components that might display Discord requirements
ast-grep --pattern 'Label($$$discord$$$)'
# And check for any additional Discord-related configuration or systems
rg "Discord" --type cs -g "!obj/**" -g "!bin/**" --no-heading
Length of output: 15158
Content.Client/UserInterface/Systems/Ghost/Controls/Roles/GhostRolesEui.cs (2)
6-6: LGTM!
The JobWhitelist import is appropriately placed and necessary for the whitelist functionality.
98-98: Address the TODO comment about Requirements equality.
The comment indicates that role.Requirements doesn't work properly as an equality key. This could lead to incorrect grouping of roles. Consider implementing proper equality comparison for the Requirements type.
Let's check how Requirements are implemented:
Would you like help implementing proper equality comparison for the Requirements type?
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/blueshield.yml (1)
Let me analyze the results from the executed script to verify the Discord requirement across CentCom roles.
The script results show that the Discord requirement (
discordRequired: true) is consistently set for all CentCom job roles in the following files:
- ./Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/cargotech.yml
- ./Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/blueshield.yml
- ./Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/operator.yml
- ./Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/intern.yml
- ./Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/centcom_admiral.yml
- ./Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/private_officer.yml
- ./Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/head_of_security.yml
- ./Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/centcom_official.yml
However, there are some CentCom-related roles in the vanilla game files that don't have the Discord requirement set:
- ./Resources/Prototypes/Roles/Jobs/CentComm/emergencyresponseteam.yml (ERT roles)
- ./Resources/Prototypes/Roles/Jobs/CentComm/deathsquad.yml
- ./Resources/Prototypes/Roles/Jobs/CentComm/cburn.yml
- ./Resources/Prototypes/Roles/Jobs/CentComm/official.yml
The Discord requirement should be added to these vanilla CentCom roles for consistency
🔗 Analysis chain
Line range hint
1-83: Verify consistent Discord requirement across CentCom roles.Since this is part of a broader change requiring Discord authorization for all ЦК roles, let's verify the consistent application across other CentCom role configurations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify Discord requirement is added to all CentCom role configurations # Expected: All CentCom role files should contain 'discordRequired: true' # Search for CentCom role files echo "=== Checking CentCom role configurations ===" fd --type f "\.yml$" --exec grep -l "CentralCommand\|CentCom" {} | while read -r file; do echo "=== Checking $file ===" grep -A 1 "^- type: job" "$file" echo "---" grep "discordRequired:" "$file" || echo "WARNING: No Discord requirement found" echo doneLength of output: 66804
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/blueshield.yml(1 hunks)Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/centcom_admiral.yml(1 hunks)
🔇 Additional comments (2)
Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/blueshield.yml (1)
10-10: LGTM: Discord requirement addition aligns with PR objectives.
The addition of discordRequired: true is appropriate for this high-privilege CentCom role and matches the PR's goal of requiring Discord authorization for all ЦК roles.
Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/centcom_admiral.yml (1)
Line range hint 17-24: Role security measures look appropriate.
The role has good security measures in place:
- Significant time requirements across multiple positions
- Special components that enhance security (MindShieldImplant, AntagImmune)
- Whitelist restriction
- Admin notification requirement
Also applies to: 29-39
| @@ -12,6 +12,7 @@ | |||
| requireAdminNotify: true | |||
| joinNotifyCrew: true | |||
| whitelisted: true | |||
| discordRequired: true | |||
There was a problem hiding this comment.
💡 Codebase verification
Based on the script results, I can now generate the final response since I have enough information to verify the concern:
Discord requirement missing in several Central Command roles
The following Central Command role files need the discordRequired: true field added to align with the PR objectives:
Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/operator.ymlResources/Prototypes/_Backmen/Roles/Jobs/CentCom/blueshield.ymlResources/Prototypes/_Backmen/Roles/Jobs/CentCom/head_of_security.ymlResources/Prototypes/_Backmen/Roles/Jobs/CentCom/cargotech.yml
🔗 Analysis chain
LGTM! Verify Discord requirement across all Central Command roles.
Adding Discord authorization requirement for the Admiral role is appropriate given its high-privilege access groups and special capabilities.
Let's verify that all Central Command roles have the Discord requirement:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all Central Command role configurations have discordRequired set to true
# Search for job definitions in Central Command role files
rg -U --multiline '(?s)- type: job.*?^[^ -]' Resources/Prototypes/_Backmen/Roles/Jobs/CentCom/ | while read -r job; do
if ! echo "$job" | rg -q "discordRequired: true"; then
echo "WARNING: Missing Discord requirement in job definition:"
echo "$job"
echo "---"
fi
done
Length of output: 161927
* full fucking send * ope forgot to remove the EE scripts * fix test * fix shitcode fail * DELTA THAT VALUE IS NULLABLE * whoopsie daysie * fixed???
* Shitmed Update 2 - bottom text (#956) * full fucking send * ope forgot to remove the EE scripts * fix test * fix shitcode fail * DELTA THAT VALUE IS NULLABLE * whoopsie daysie * fixed??? * chat is this real * Hotfix for broken organ transplants (#933) * full fucking send * ope forgot to remove the EE scripts * fix test * fix shitcode fail * DELTA THAT VALUE IS NULLABLE * whoopsie daysie * fixed??? * allow cloning living people (#932) * allow cloning living people * fix client --------- Co-authored-by: deltanedas <@deltanedas:kde.org> * Added missing locale strings for limb targeting bindings (#921) * Added missing locale strings for limb targeting bindings * Moved the strings down where the rest of shitmed goes * Shitmed Update 1 - Forma de Goob (#897) * full fucking send * ope forgot to remove the EE scripts * fix test * fix shitcode fail * DELTA THAT VALUE IS NULLABLE * whoopsie daysie * fixes fixes add and fix shit * Fix Harpies --------- Co-authored-by: gluesniffler <[email protected]> Co-authored-by: deltanedas <[email protected]> Co-authored-by: AstroDogeDX <[email protected]>
🆑
Summary by CodeRabbit
Release Notes
New Features
BKCCargo,BKCCOfficial,BKCCSecGavna,BKCCAssistant,BKCCOperator,BKCCSecOfficer,BlueShield, andBKCCAdmiral.Bug Fixes
Localization
Chores