Manager of Assistants - Part 2 & ID card computer QOL - #5546
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (16)
📒 Files selected for processing (21)
🚧 Files skipped from review as they are similar to previous changes (18)
Included review availability: Your plan includes up to 2 reviews per rolling hour; 1 remains after this review. 📝 WalkthroughSummaryAdds Assistant Manager tools for changing assistant job titles and HUD icons. User impactAssistant Managers can assign assistant job titles and HUD icons without changing ID card access. ID card computers filter icons by console tags. Emagged consoles unlock all icons. Universal consoles show all icons. Notable upstream changes
Risk areas
Guideline gaps
WalkthroughThe PR adds selectable job icons to ID card consoles with tag filtering and EMAG unlocking. It adds a privileged job identity console for changing titles and icons. It adds Assistant Manager job icons, localization, prototypes, and clipboard assets. ChangesJob identity and icon management
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to This change adds Assistant Manager ID-card tooling and related job icons; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Operator
participant JobIdentityConsoleWindow
participant JobIdentityConsoleBoundUserInterface
participant JobIdentityConsoleSystem
participant TargetIdCard
Operator->>JobIdentityConsoleWindow: Select title and job icon
JobIdentityConsoleWindow->>JobIdentityConsoleBoundUserInterface: Submit console request
JobIdentityConsoleBoundUserInterface->>JobIdentityConsoleSystem: WriteJobIdentityMessage
JobIdentityConsoleSystem->>TargetIdCard: Apply validated title and icon
JobIdentityConsoleSystem-->>JobIdentityConsoleWindow: Publish updated identity state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
Content.Shared/_Starlight/Access/Components/JobIdentityConsoleComponent.cs (1)
28-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCorrect the
RequiredTagsdocumentation and use an XML doc comment.The comment names
JobIconPrototype.AllowSelectionandallIconsUnlocked. Neither applies to this component: the filter here isnewJobIcon.Tags.Overlaps(component.RequiredTags)inContent.Server/_Starlight/Access/Systems/JobIdentityConsoleSystem.csLines 62-63, and there is no unlock override onJobIdentityConsoleComponent. Also state the fail-closed behavior, because an empty set rejects every icon.📝 Proposed doc
- /// Through "JobIconPrototype.AllowSelection" a console will only display icons that match the tag set in job.yml with the tag on the console, unless overided by allIconsUnlocked - + /// <summary> + /// Icons are selectable on this console only if <see cref="JobIconPrototype.Tags"/> overlaps this set. + /// An empty set rejects every icon. + /// </summary> [DataField, AutoNetworkedField] public HashSet<ProtoId<TagPrototype>> RequiredTags = new();As per path instructions: "Add accurate XML documentation to public/reusable methods, component fields, and parameters when it materially helps callers."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Content.Shared/_Starlight/Access/Components/JobIdentityConsoleComponent.cs` around lines 28 - 31, Replace the existing documentation above JobIdentityConsoleComponent.RequiredTags with an accurate XML doc comment. Describe that icons are accepted when newJobIcon.Tags overlaps the required tags, and explicitly state that an empty RequiredTags set rejects every icon; remove references to JobIconPrototype.AllowSelection and allIconsUnlocked.Source: Path instructions
Content.Shared/_Starlight/Access/Systems/SharedJobIdentityConsoleSystem.cs (1)
12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
Sawmillconstant.Nothing resolves a sawmill from this name. The system has no
ISawmillfield, andJobIdentityConsoleSystemlogs only through_adminLogger.🧹 Proposed removal
- public const string Sawmill = "jobidentityconsole"; -As per path instructions: "Flag ... dead code, stale test code, unused dependencies".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Content.Shared/_Starlight/Access/Systems/SharedJobIdentityConsoleSystem.cs` at line 12, Remove the unused Sawmill constant from the job identity console system; no replacement is needed because logging uses _adminLogger and no ISawmill resolves this name.Source: Path instructions
Content.Shared/Access/Systems/SharedIdCardConsoleSystem.cs (1)
21-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a per-console tag list instead of one hardcoded tag.
CrewJobIconTaghardcodesJobIconCrewfor everyIdCardConsoleComponent. The newJobIdentityConsoleComponent.RequiredTags(Content.Shared/_Starlight/Access/Components/JobIdentityConsoleComponent.csLines 30-31) already models the same concept as a configurableHashSet<ProtoId<TagPrototype>>. Two divergent mechanisms for one rule will drift, and department-restricted ID consoles cannot be expressed with the constant.Reusing a single
requiredTagsdata field on both components keeps the filter behavior in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Content.Shared/Access/Systems/SharedIdCardConsoleSystem.cs` around lines 21 - 23, Replace the hardcoded CrewJobIconTag usage in SharedIdCardConsoleSystem with a configurable requiredTags collection shared by IdCardConsoleComponent and JobIdentityConsoleComponent. Reuse the existing RequiredTags field pattern so each console can define allowed icon tags, including department-specific restrictions, while keeping the filtering behavior centralized.Content.Shared/Access/Components/IdCardConsoleComponent.cs (1)
86-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix the malformed doc comment on
AllIconsUnlocked.Line 87 contains a stray
<and a missing space ("IdCardConsoleComponent.AllIconsUnlocked"is). The two lines also duplicate the "True if" phrasing.📝 Proposed comment cleanup
- // True if every job icon (not just those tagged for that console) are unlocked for selection. - // True if <"IdCardConsoleComponent.AllIconsUnlocked"is set or the console has been emagged. + // True if every job icon is selectable, not only the icons tagged for this console. + // Set when IdCardConsoleComponent.AllIconsUnlocked is true or the console has been emagged. public readonly bool AllIconsUnlocked;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Content.Shared/Access/Components/IdCardConsoleComponent.cs` around lines 86 - 88, Clean up the documentation comment for IdCardConsoleComponent.AllIconsUnlocked by removing the stray “<”, adding the missing space before “is”, and consolidating the duplicated “True if” wording into a clear description of the field’s two conditions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Content.Client/Access/UI/IdCardConsoleWindow.xaml.cs`:
- Around line 239-243: The icon selection state retains the previous target’s
icon when the new target has none. In
Content.Client/Access/UI/IdCardConsoleWindow.xaml.cs lines 239-243, reset
_currentJobIcon when TargetIdJobIcon is null and prevent submission until the
current target has an explicit selection; in
Content.Client/_Starlight/Access/UI/JobIdentityConsoleWindow.xaml.cs lines
83-87, apply the same reset and no-selection behavior, updating the message
contract if needed to represent no icon selected.
In `@Content.Server/Access/Systems/IdCardConsoleSystem.cs`:
- Around line 70-84: Update OnGotEmagged and the related UpdateUserInterface
flow so unlocked job icons are computed with the interaction emag state already
applied, even before the standard completion flow stores the flag. Use an
explicit unlock override or adjust the emag check in UpdateUserInterface, while
preserving the existing UI refresh and logging behavior.
- Around line 269-287: Update the default-icon fallback in the job icon
selection logic around IconAllowed and JobPrototype.Icon so IconAllowed is
applied only to the explicitly selected icon. Resolve and use job?.Icon without
the access check, preserving the existing restriction for newJobIcon and
ensuring the resulting jobIcon is passed to TryChangeJobIcon and
UpdateStationRecord.
In `@Content.Shared/_Starlight/Access/Components/JobIdentityConsoleComponent.cs`:
- Around line 33-44: Update WriteJobIdentityMessage so JobIcon uses nullable
ProtoId<JobIconPrototype>?, allowing title-only updates to omit an icon. Ensure
the server handling this message treats null as “keep the current icon” while
still applying JobTitle, matching IdCardConsoleComponent.WriteToTargetIdMessage
behavior.
In `@Resources/Prototypes/_Starlight/Entities/Objects/Misc/folders.yml`:
- Around line 298-303: Update the assistant manager board entity’s StealTarget
configuration to use a distinct stealGroup from HoPBoardSteal, while preserving
the existing HoPBoardSteal assignment for BoxFolderHoPClipboard. Review the
board’s inheritance and tags alongside BaseGrandTheftContraband and confirm the
intended contraband tier, adding or removing the parent as needed for
consistency.
- Around line 238-240: Update the JobIdentityConsole prototype using the
JobIconAssistantManager required tag: add JobIconAssistantManager to the Tags
collection of an existing jobIcon prototype, or define a matching jobIcon
prototype, so JobIdentityConsoleSystem can resolve a valid assistant manager
icon.
In `@Resources/Prototypes/StatusIcon/job.yml`:
- Around line 17-18: Replace each inline “# Starlight-edit” marker on the added
JobIconCrew tag blocks with # Starlight-start before the multi-line tags block
and # Starlight-end after it, applying this consistently to every listed
occurrence in the file.
---
Nitpick comments:
In `@Content.Shared/_Starlight/Access/Components/JobIdentityConsoleComponent.cs`:
- Around line 28-31: Replace the existing documentation above
JobIdentityConsoleComponent.RequiredTags with an accurate XML doc comment.
Describe that icons are accepted when newJobIcon.Tags overlaps the required
tags, and explicitly state that an empty RequiredTags set rejects every icon;
remove references to JobIconPrototype.AllowSelection and allIconsUnlocked.
In `@Content.Shared/_Starlight/Access/Systems/SharedJobIdentityConsoleSystem.cs`:
- Line 12: Remove the unused Sawmill constant from the job identity console
system; no replacement is needed because logging uses _adminLogger and no
ISawmill resolves this name.
In `@Content.Shared/Access/Components/IdCardConsoleComponent.cs`:
- Around line 86-88: Clean up the documentation comment for
IdCardConsoleComponent.AllIconsUnlocked by removing the stray “<”, adding the
missing space before “is”, and consolidating the duplicated “True if” wording
into a clear description of the field’s two conditions.
In `@Content.Shared/Access/Systems/SharedIdCardConsoleSystem.cs`:
- Around line 21-23: Replace the hardcoded CrewJobIconTag usage in
SharedIdCardConsoleSystem with a configurable requiredTags collection shared by
IdCardConsoleComponent and JobIdentityConsoleComponent. Reuse the existing
RequiredTags field pattern so each console can define allowed icon tags,
including department-specific restrictions, while keeping the filtering behavior
centralized.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ce75c999-0368-4f00-909b-fe66fbd953a3
⛔ Files ignored due to path filters (16)
Resources/Textures/_Starlight/Interface/Misc/job_icons.rsi/ClericalAssistant.pngis excluded by!**/*.pngResources/Textures/_Starlight/Interface/Misc/job_icons.rsi/CookAssistant.pngis excluded by!**/*.pngResources/Textures/_Starlight/Interface/Misc/job_icons.rsi/DeliveryAssistant.pngis excluded by!**/*.pngResources/Textures/_Starlight/Interface/Misc/job_icons.rsi/GardenerAssistant.pngis excluded by!**/*.pngResources/Textures/_Starlight/Interface/Misc/job_icons.rsi/HandymanAssistant.pngis excluded by!**/*.pngResources/Textures/_Starlight/Interface/Misc/job_icons.rsi/MedicalAssistant.pngis excluded by!**/*.pngResources/Textures/_Starlight/Interface/Misc/job_icons.rsi/ReligiousAssistant.pngis excluded by!**/*.pngResources/Textures/_Starlight/Interface/Misc/job_icons.rsi/ShopkeeperAssistant.pngis excluded by!**/*.pngResources/Textures/_Starlight/Objects/Misc/assistantmanager_clipboard.rsi/am_clipboard.pngis excluded by!**/*.pngResources/Textures/_Starlight/Objects/Misc/assistantmanager_clipboard.rsi/am_clipboard_over.pngis excluded by!**/*.pngResources/Textures/_Starlight/Objects/Misc/assistantmanager_clipboard.rsi/hop_clipboard_paper.pngis excluded by!**/*.pngResources/Textures/_Starlight/Objects/Misc/assistantmanager_clipboard.rsi/hop_clipboard_pen.pngis excluded by!**/*.pngResources/Textures/_Starlight/Objects/Misc/assistantmanager_clipboard.rsi/hop_clipboard_priv_id.pngis excluded by!**/*.pngResources/Textures/_Starlight/Objects/Misc/assistantmanager_clipboard.rsi/hop_clipboard_target_id.pngis excluded by!**/*.pngResources/Textures/_Starlight/Objects/Misc/assistantmanager_clipboard.rsi/inhand-hopboard-left.pngis excluded by!**/*.pngResources/Textures/_Starlight/Objects/Misc/assistantmanager_clipboard.rsi/inhand-hopboard-right.pngis excluded by!**/*.png
📒 Files selected for processing (21)
Content.Client/Access/UI/IdCardConsoleBoundUserInterface.csContent.Client/Access/UI/IdCardConsoleWindow.xamlContent.Client/Access/UI/IdCardConsoleWindow.xaml.csContent.Client/_Starlight/Access/UI/JobIdentityConsoleBoundUserInterface.csContent.Client/_Starlight/Access/UI/JobIdentityConsoleWindow.xamlContent.Client/_Starlight/Access/UI/JobIdentityConsoleWindow.xaml.csContent.Server/Access/Systems/IdCardConsoleSystem.csContent.Server/_Starlight/Access/Systems/JobIdentityConsoleSystem.csContent.Shared/Access/Components/IdCardConsoleComponent.csContent.Shared/Access/Systems/SharedIdCardConsoleSystem.csContent.Shared/StatusIcon/StatusIconPrototype.csContent.Shared/_Starlight/Access/Components/JobIdentityConsoleComponent.csContent.Shared/_Starlight/Access/Systems/SharedJobIdentityConsoleSystem.csResources/Locale/en-US/_Starlight/job/job-names.ftlResources/Prototypes/StatusIcon/job.ymlResources/Prototypes/_Starlight/Entities/Objects/Misc/folders.ymlResources/Prototypes/_Starlight/Entities/Structures/Machines/Computers/computers.ymlResources/Prototypes/_Starlight/StatusIcon/job.ymlResources/Prototypes/_Starlight/tags.ymlResources/Textures/_Starlight/Interface/Misc/job_icons.rsi/meta.jsonResources/Textures/_Starlight/Objects/Misc/assistantmanager_clipboard.rsi/meta.json
|
The bot talked about getting rid of hardcoding tags from consoles, which is probably a good idea. Going to figure out how I want to do that and then this should be in a serviceable state. |
|
Please tell me you removed all the syndicate job icons and stuff |
Yes, that's what the tagging does - see the screenshots. |
|
@bynddark8 the other PR got merged, can you update this one to make sure Assistant Managers spawn with the board? |
|
probably just in their pocket slot |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
done, added to the loadout in the belt slot |
Content + Integration Test ResultsFailed Tests
Github Test Reporter by CTRF 💚 |
60f3c6c
Short description
Part 2 of #5528, as requested in #4183.
This PR purely implements the Assistant Manager's task master digi-board and the code required to make it work. The job configuration, clothing sprites, and loadout are all in part 1.
This PR does (4) things to provide implementation for the Assistant Manager:
IdCardConsoleSystemand the newly createdJobIdentityConsoleSystem.JobIdentityConsoleSystem, which is effectively an ID card computer that can change an ID card's job title and HUD icon without reading or writing access tags. This interface is accessible through a new item: the task master digi-board, which allows Assistant Managers to set the job titles and HUD icons of their subordinates for more effective task delegation.Why we need to add this
Brief summary, reasoning explained more in depth in the original issue.
Highpop/Alpha consistently struggles with shittery and idleness with a regular population of 50+ assistants as a result of limited job slots. This kind of behavior among assistants places additional strain on command/security players alongside the administrative team to maintain adequate levels of roleplay standards. By placing trusted (whitelisted) players in a position to organize and coordinate this population, assistants will hopefully actually assist the crew and contribute more meaningfully to roleplay and create less friction in the course of a round.
Media (Video/Screenshots)
fig 1. task master digi-board sprites
fig 2. Assistant "Job" icons (from left to right): shopkeeper, delivery boy, gardener, cook assistant, handyman, clerical assistant, religious assistant, medical assistant.
fig 3. the task master digi-board
fig 4. the ID card computer
fig 5. an emagged ID card computer
Checks
Changelog
🆑 bynddark8