Multiple enabled character slots with binary job selection. Make job priorities PLAYER specific.

[Quantum-cross]

About the PR

This is my as of now successful attempt at implementing multi-character selection. I wanted to get a draft on the books so I can start making incremental commits/changes to fix individual issues.

It allows each character slot to be enabled or disabled, as well as having binary job selections similar to how antags are selected.

Job priorities (never -> high) are selected globally on your player. This is represented by a new button above your characters that says "Edit job priorities".

All of these new configuration elements are saved on the server database.

When the round begins, the server will first create a set of enabled jobs on all your characters, and find the intersection of that with your player job priorities > "never". It will then use that resultant list of jobs to consider for your assignment.

After assignments are complete and the players are to be spawned, the server will look at all of your enabled characters who can take that job and pick one randomly for you to play.

Reminder to all ye brave ones who enter for review, Content.Server.Database/Migrations is all automatically generated

Current TODO list

• [x] Fix dummy character rendering
• [x] Fix broken integration tests
• [x] Remove logic for "Selected character" because that doesn't make sense in this new paradigm
• [x] Fix logic for LobbyUIController.GetPreferredJob
• [x] Make UI less bad
• [x] Fix enable button showing state
• [x] Fix loading of job priorities
• [x] Fix dirtying of job priority picker (enable/disable save/reset buttons)
• [x] Fix job priority picker layout
• [x] Investigate how to handle antags (non-crew antags probably should be selected per character so that you can make a wizard only character or nukie only character)
• [x] Probably don't need to pass in character profiles to SpawnPlayers
• [x] Make sure borg and AI spawning works (in the future it might be good to implement something like "SiliconCharacterProfile")
• [ ] Should we enable a one time popup for players to ensure that their job settings are correct after this change happens?
• [ ] Guidebook entry?
• [x] missing JobRolePrototypes breaks things
• [x] Spawn admin verb should be able to select a profile (starlight fixed this, I have permission from the author to use their code as a base for this)
• [x] sort job icons on lobby by department
• [x] dynamic sizing of job prio columns?
• [ ] collapsible job summary and/or server info on lobby gui to ensure chat is usable (the dynamic column things makes this better, can put this off to another pr)
• [x] character preview going urist in some situations (antag profile?) (show clothing off?)
• [x] Loneop profile load not working?
• [x] Quicker editing of job priorities from lobby, drag 'n' drop?
• [x] possible issue - Playtime requirement lock not updating specifically for the job priority editor? (requirement for late join and job preference is still fine?)
• [x] report bug of being able to queue for nukie/wiz with no jobs selected (this pr fixes that I just want to document it as a bug)

Why / Balance

So I can make a stoner janitor character and a rough and tough CE character but also be considered for both jobs/characters at round start.

Technical details

SEE POST BELOW HERE FOR EXTREMELY DETAILED FILE-BY-FILE GUIDE TO CHANGES

https://github.com/space-wizards/space-station-14/pull/36493#issuecomment-2863996411

Media

Screencast showing off the implemented features

https://www.youtube.com/watch?v=h33FhM1jxyE

Pictures showing the new lobby preview UI:

Requirements

• [x] I have read and am following the Pull Request and Changelog Guidelines.
• [x] I have added media to this PR or it does not require an ingame showcase.

Breaking changes

• There is no concept of the "currently selected character" for a player anymore
  • Any code that tries to get the "currently selected character" has been changed to mimic previous functionality as closely as possible
  • Specifically for downstreams where this causes an issue
    • If you are trying to get the "profile" of a player already in the game, instead use SharedHumanAppearanceSystem.GetBaseProfile() to get the profile that the entity was spawned with.
    • Specifically for Starlight: I know that the new life system works by setting the selected character before re-joining. Instead, you must use the new parameter to the joingame command. See the late join GUI code or my earlier port of this PR to starlight in which I just instantiate a copy of the late join GUI for the new life system instead of duplicating the late join GUI code.
  • ~~"Set Outfit" verb probably won't get the player's loadout when setting a job outfit, because there is no well defined "selected character" to pull the loadout from~~
  • "Set Outfit" verb on a mob will use job profiles associated with the HumanoidCharacterProfile that was used to spawn the mob
  • DeathMatchRuleComponent will now choose a random enabled profile to spawn as if no profile is supplied in PlayerBeforeSpawnEvent
  • AntagLoadProfileRuleComponent will now search a player's characters for an enabled character with the Antag selected
  • GameTicker.GetPlayerProfile(ICommonSession) is removed because there is no "selected player profile"
  • Spawning in directly as an observer will use your profile name instead of a character name
• Database changes and migrations
  • Profiles now have a HashSet of JobPreferences, similar to AntagPreferences, indicating that that job is enabled on that profile
  • PlayerPreferences now contain the traditional dictionary of Job -> JobPriority mapping
  • Profiles now have a boolean flag signifying that they are Enabled or Disabled
  • The "PreferenceUnavailable" setting has been removed. This behavior can be restored by setting Passenger job to Low
• Many changes to the HumanoidProfileEditor
• Change Dictionary JobPriorities to HashSet JobPreferences in HumanoidCharacterProfile
• Character export format has been bumped to version 2
• Import of version 1 characters are supported
• Console command "joingame" now takes 3 arguments instead of 2, the first argument now being the slot number of the character you are late joining with
• Changes to the TestPair Helpers to support the new job priority and preference structures
• AntagSelectEntityEvent now includes AntagSelectionDefinition argument to support choosing a profile in AntagLoadProfileRuleComponent
• RulePlayerSpawningEvent and RulePlayerJobsAssignedEvent now takes a Set of NetUserId and does not carry character profiles
• PlayerBeforeSpawnEvent Profile argument is now nullable
• Calling IsAllowed, GetDisallowedJobs, or RemoveDisallowedJobs in PlayTimeTrackingSystem with game.role_timers off will still check age and species requirements.
• calling JobRequirements.TryRequirementsMet with a null playTimes dictionary will not check playtimes (but will still check age and species requirements
• The department named "Specific" has been removed
  • as far as I can tell this was purely categorical to put a section in the job priority selector called "Station-specific jobs".
  • This is kind of unnecessary at this point because besides maybe zookeeper, these jobs tend to be on most maps.
  • As for the why, it just messes with the ordering of the job icons on the lobby screen without adding a hard-coded "ignore the Specific department"

Changelog

:cl:

• add: Job priorities are now bound to your player
• tweak: Job preferences on characters are "yes" or "no" like antag selection
• add: Characters can be individually enabled or disabled
• add: All enabled characters will be considered for round start job assignment (does not affect Antag before job roll)
• add: Antag character previews are shown on character customization (if character has no job selected)
• add: Late join window now has a character picker
• add: You can make profiles for Space Ninja! (They will be used when ghost role is taken)
• add: Show job specific loadout name on character pickers
• tweak: Lobby character summary is overhauled to display all of your selected jobs and characters in a hopefully clean and intuitive way!
• fix: Age, species, and trait job requirements should now work with role timers turned off
• fix: Non-human species will no longer show up in the guidebook with a weirdly human skin tone...
• add: Admin verb - "Spawn as ... " lets you spawn a player using any of their profiles.
• add: You can drag and drop the icons in the lobby to change your job priorities!
• tweak: Better user feedback (clear tooltips) for the "Ready/Not Ready" button.

[Quantum-cross]

I think I've fixed all of the major UI glitches so what the UI displays should be accurate to the internal and server state.

This should be in a good place for some initial local testing if anyone wants to experiment with it.

[github-actions[bot]]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[Quantum-cross]

Showing off the PR

https://www.youtube.com/watch?v=h33FhM1jxyE

[Quantum-cross]

And here I thought I was past this test failure...

[github-actions[bot]]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions[bot]]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[FairlySadPanda]

FYI you don't need to handle merge conflicts yourself, it'll be the maint's job when merging unless the diff becomes catastrophic

[Quantum-cross]

FYI you don't need to handle merge conflicts yourself, it'll be the maint's job when merging unless the diff becomes catastrophic

Sure, but it's not a problem if I do handle it, right?

I'm still planning on adding a couple little bits here and there...

[Quantum-cross]

Added some screenshots of a fixed up lobby character summary -- I need to add some padding values between some elements but I really like how it turned out...

[Quantum-cross]

sorry for the early undrafting I guess, I was kind of excited -- but anything I commit now will be strictly bugfixes, requested changes, and probably padding, minor UI alignment stuff.

[ilovehans10]

I love this feature, and that lobby UI is beautiful. All around top marks for what will be such a major improvement to character creation.

Now all we need is the ability to save/copy loadouts between roles/characters and we'll have a perfectly ergonomic character creation (I know that is out of scope of this PR)!

[Quantum-cross]

I love this feature, and that lobby UI is beautiful. All around top marks for what will be such a major improvement to character creation.

Now all we need is the ability to save/copy loadouts between roles/characters and we'll have a perfectly ergonomic character creation (I know that is out of scope of this PR)!

Yeah I have a major refactor of the profile editor after this gets merged that might begin to make things like that easier.

[Quantum-cross]

Shows loadout override names:

Padding/layout improvements:

[Quantum-cross]

duh, it's supposed to say disabled

[Quantum-cross]

did some work filling out breaking changes, gotta eat then I'll shuffle that information around and put more stuff in technical details

[Quantum-cross]

FILE BY FILE PR REVIEW GUIDE

PLAYER PREFERENCES AND DATABASE CHANGES

Content.Client/Lobby/ClientPreferencesManager.cs

• Changes to the PlayerPreferences class necessitates the changes here to manage the local copy of PlayerPreferences and requesting changes of preferences to the server.
• PlayerPreferences no longer holds SelectedCharacterIndex and therefore MsgSelectCharacter and SelectCharacter(...) has been removed.
• Now, individual characters can be enabled or disabled, so a replacement function has been added SetCharacterEnable(slot, enable) which will enable or disable the character in the slot both locally and sends a request for the change on the server's database.
• PlayerPreferences now contains a Dictionary of Job, JobPriority pairs. Previously, these were held on character profiles.
• A public function UpdateJobPriorities has been added here to set the JobPriorities both locally and sending a request for the change on the server's database.
• The constructor for PlayerPreferences has been changed to remove SelectedCharacterIndex and add JobPriorities

Content.Client/Lobby/IClientPreferencesManager.cs

Explanation for the changes to the interface here is reflected above in ClientPreferencesManager

Content.Server/GameTicking/GameTicker.Player.cs

Remove GetPlayerProfile(ICommonSession) because there is no "selected character".

This function is used in some weird places so replacements will be done carefully...

Content.Server/Preferences/Managers/ServerPreferencesManager.cs

• Remove handler for MsgSelectCharacter
• Add handler for MsgSetCharacterEnable
  • Just read the current character in that slot and set it back with the requested value
• Add Handler for MsgUpdateJobPriorities
  • This will do some sanitization to remove Never jobs and enforce only one High job
• Remove GetSelectedProfilesForPlayers (no "selected" profiles...)

Content.Server/Preferences/Managers/IServerPreferencesManager.cs

Explanation for the changes to the interface here is reflected above in ServerPreferencesManager.cs

Content.Server.Database/Model.cs

This contains the potatoes of the database structure changes.

• Changes have been made to the Profile class, representing a character profile on the database.
  • PreferenceUnavailableMode has been removed here
  • The boolean value "Enabled" has been added.
• The Job class has been changed to simply represent a Job "Preference" on a character profile.
  • This no longer contains a JobPriority column
  • Entries of Job will be connected to a character profile Profile and will denote that a character profile has selected "yes" to a job.
• The JobPriorityEntry class has been added to represent a selected job with priority attached to a Player Preference.
  • This has been named JobPriorityEntry to avoid conflicts with the name JobPriority
  • It's basically what Job used to be, except it contains references (foriegn key?) to a Preference object instead of a Profile object.
• The enum DbPreferenceUnavailableMode has been removed.

Content.Server/Database/ServerDbBase.cs

This contains the meat of the database changes.

• The GetPlayerPreferencesAsync(...) task will also include the JobPriorities of the profile.
  • These are converted properly to a dictionary before being stuffed into the returned PlayerPrefences
• SaveSelectedCharacterSlotAsync is removed, no selected character index anymore.
• The task SaveJobPrioritiesAsync has been added to save a player's job priorities to the database.
  • This task will query the database for the Preference matching the player's userId and also include the JobPriorities into the query.
  • The dictionary is converted to a list of JobPriorityEntry database objects and is saved.
• InitPrefsAsync is used to initialize a Preference object in the database for a brand new player connected to the server.
  • It requires some modifications due to the new structures.
  • A new JobPriority dictionary is created with only Passenger (FallbackOverflowJob) set to High, every other job is Never.
  • The new player's Preference is initialized with this JobPriority dictionary.
• DeleteSlotAndSetSelectedIndex(...) is removed because we don't need to worry about selected slot.
• SetSelectedCharacterSlotAsync(...) is removed because we don't need to worry about selected slot.
• ConvertProfiles(Profile ...) is used to convert a database-side Profile object to a game-side HumanoidCharacterProfile and requires minimal changes.
  • Jobs is now a HashSet instead of a Dictionary and contains no priorities.
  • The returned HumanoidCharacterProfile also includes the Enabled value.
• ConvertProfiles(HumanoidCharacterProfile ...) is used to convert a game-side HumanoidCharacterProfile to a database-side Profile object and requires similar changes in reverse.
  • No longer need PreferenceUnavailable
  • Need to copy over Enabled value.
  • Need to convert humanoid.JobPreferences to a List of Job instead of a JobPriority object.

Content.Server/Database/ServerDbManager.cs

This contains the sprig of parsley on the side of the database changes.

• The IServerDbManager interface is changed to reflect the changes in tasks explained above in ServerDbBase.cs
• The wrapper function SaveSelectedCharacterIndexAsync is removed
• The wrapper function DeleteSlotAndSetSelectedIndex is removed
• The wrapper function DeleteSlotAndSetSelectedIndex is removed
• The wrapper function SaveJobPrioritiesAsync is added

Content.Shared/Preferences/MsgSetCharacter.cs

This NetMessage has been removed -- git has recorded this as a file rename to...

Content.Shared/Preferences/MsgSetCharacterEnable.cs

This is the NetMessage used to ask the server to enable or disable a character. It just contains the index/slot number and a boolean value.

Content.Shared/Preferences/MsgUpdateJobPriorities.cs

This is the NetMessage used to ask the server to update the player's job priorities. It just contains the same Dictionary as in PlayerPrefences.

Content.Shared/Preferences/PlayerPreferences.cs

The PlayerPreferences class has been modified here, and a bunch of helper functions for filtering, managing and combining both the settings of character profiles and player preferences have been added here.

• PlayerPreferences constructor no longer takes selectedCharacterIndex, and now takes the dictionary of job priorities
• SanitizeJobPriorities has been added here, which just removes jobs with the priority set to Never.
• Property SelectedCharacterIndex removed
• Property SelectedCharacter removed
• Property JobPriorities added
• Helper functions added, see documentation for explanation

Content.Shared/Preferences/PreferenceUnavailableMode.cs

Removed

Content.Server.Database/Migrations/***

These database migrations are automatically generated using add-migration.ps1.

Content.Server.Database/Migrations/*/20250428125232_MultiCharacter.Designer.cs

This is kind of unreviewable, I think this is the code to generate the database from scratch? Again -- this was automatically generated.

If you're a database expert feel free to look over this.

Content.Server.Database/Migrations/(Sqlite|Postgres)/(Sqlite|Postgres)ServerDbContextModelSnapshot.cs

Automatically generated, but has a reasonable diff, these may be worth looking by a database expert

Content.Server.Database/Migrations/*/20250428125232_MultiCharacter.cs

Automatically generated, but worth looking at.

This is the code that is generated to perform the steps necessary to "upgrade" the database to the new structure.

• The "one_high_priority" index to the "job" table is dropped, as now job just contains a hash set of requested jobs for a character. It no longer contains the priority, and the "only one job can high priority" restriction here does not matter.
• The "preferences_unavailable" column has been dropped (this is the setting that makes you spawn as Passenger when the server fails to assign you a job)
• The "selected_character_slot" column has been dropped.
• We add a column "enabled" to the table of character profiles.
• We create a table "job_priority_entry", this is kind of what the "job" table USED to be, except that it will be tied to "preference" table instead of "profile"
• The "one_high_priority" index is created on the new "job_priority_entry" table to enforce that only one high priority job is enabled on the new table connected to "profile"

HUMANOID CHARACTER PROFILE

Content.Shared/Preferences/HumanoidCharacterProfile.cs

• DataField _jobPriorities dictionary is changed to _jobPreferences hash set
• DataField Enabled is added to the profile
• Read only property JobPriorities is changed to JobPreferences
• PreferenceUnavailable is removed
• Constructor is changed to reflect the new values.
• Copy constructor is changed to reflect the new values.
• BUGFIX #37282 : DefaultWithSpecies now makes sure that it will get the default appearance of said species.
  • If you don't believe me that this is a bug on master-- open the guidebook to the Species tab and experience the cursed human-skin-toned slime person.
• Removed WithJobPriorities(...)
• Added WithJobPreferences(...) to return a copy of the profile with a given set of job preferences
• Removed WithJobPriority(...)
• Added WithJobPriority(JobPrototype job, bool include) to return a copy of the profile with a specific job included or excluded from the set of job preferences.
• Removed WithPreferenceUnavailable
• Added AsEnabled(...) to return a copy of the profile with the given Enabled value
• EnsureValid required a few changes to validate the new fields
  • Validation of PreferenceUnavailable removed
  • Validation of JobPriorities removed.
  • Validation of JobPreferences added, it just ensures that each job in the HashSet is a valid prototype, and that the prototype sets that it is allowed to be set on the Character Profile Editor (SetPreference field)

Content.Shared/Preferences/Loadouts/RoleLoadout.cs

I changed a PrototypeManager.Index(...) call to TryIndex(...) because there was some case that this failed. I forget the exact details, but there should be no problem if this bails here.

Content.Shared/Humanoid/HumanoidAppearanceComponent.cs

There are many functions that use GetPlayerProfile assuming that a player session had a singular profile assigned to it. This function was removed.

Instead, when a mob is spawned, the HumanoidCharacterProfile that it was spawned with is copied to a field in the HumanoidAppearanceComponent so that it can be cloned, copied, loadouts can be extracted from the original profile, etc etc even if the entity's mind and owner swap.

This is stored in the BaseProfile DataField here.

Content.Shared/Humanoid/HumanoidProfileExport.cs

Increment the version number of character export as the job priorities are now job preferences.

Content.Shared/Humanoid/SharedHumanoidAppearanceSystem.cs

• Handle a version 1 exported character file by converted the priorities into preferences before loading.
• Add a function to save and get the base profile from the HumanoidAppearanceComponent
• Save the profile to the HumanoidAppearanceComponent when calling LoadProfile

UI CHANGES

Please note that I factored a lot of code out of monolithic files here. This is why the diffs are so huge.

I will do my best to describe what code has been moved, and what code has actually changed

Content.Client/Lobby/LobbyUIController.cs

• A new UI class JobPriorityEditor was added, and LobbyUIController has a private handle to an instance.
• In PreferencesDataLoaded and OnStateEntered, setting SelectedCharacterSlot to null will ensure that when the editor UI is constructed, it will automatically select and display the first profile.
• The LobbyCharacterPreviewPanel has been overhauled and its logic has been refactored, it just requires an event invocation to refresh.
• Some local wrapper functions for saving job priorities and reloading the UI have been added for event invocations
• OpenSavePanel now takes an Action argument which is invoked when Confirm is selected.
  • Note: I want to add this "are you sure you want to leave" panel to the case where you select a different character when changes are unsaved, but I need to add an event to RT to do it cleanly. This will be a future project.
• _characterSetup.SelectCharacter is invoked when a profile picker button is clicked.
  • We no longer need to send a message to the server saying "Hey this character is selected"
  • We still use this event to load the clicked profile into the profile editor and change the selected slot in the character setup class.
• Now we add a SetCharacterEnable action which is invoked when the Enabled button is toggled on a character, this will send a message to the server.
• Any function that involves spawning and "dressing" the dummy entity is moved to Content.Client\Lobby\UI\ProfileEditorControls\ProfilePreviewSpriteView.cs
  • GiveDummyJobClothesLoadout
  • GetPreferredJob
  • GiveDummyLoadout
  • GiveDummyJobClothes
  • LoadProfileEntity

Content.Client/Lobby/UI/CharacterPickerButton.xaml

• The raw SpriteView control has been changed to the ProfilePreviewSpriteView control which inherits from SpriteView
• An Enabled button has been added in a box with along with the delete button.
• Some other fun hacky PanelContainer has been also added to give the buttons an outline so that they can be distiguished on top of the button they sit on.

Content.Client/Lobby/UI/CharacterPickerButton.xaml.cs

• An event was added for handling the Enabled toggle
• The constructor passes in a pile more managers (can this be solved with an Inject?)
• The constructor takes an optional simple argument, which disables the Enable and Delete buttons for re-use in the LateJoinGui
• The entity preview is much simpler to set up, you just need to pass in some references to managers with Initialize (again, can this be solved with injection?), and then pass the character profile to LoadPreview(...).
• The ProfilePreviewSpriteView extracts character and loadout names as well, so the description text is just copied from that object once the profile is loaded.
• The rest should be pretty simple to understand hopefully.

Content.Client/Lobby/UI/CharacterSetupGui.xaml

This xaml describes the entire window that pops up when you click Customize on the lobby.

• The Characters BoxContainer is pushed down to make room for the "Edit Job Priorities" button, with a nice horizontal rule between them.
• A box to hold an instance of the JobPriorityEditor is added beside CharEditor
• Only one will be visible at a time

Content.Client/Lobby/UI/CharacterSetupGui.xaml.cs

• SelectedCharacterSlot is added here to track the slot number of the character being edited
• The event SetCharacterEnable is added here to bubble that event back up to the LobbyUIController
• A reference to the HumanoidProfileEditor is added here so that we can set the CharacterPickerButton instances to load a profile into the profile editor when clicked.
• In the constructor, add and initialize the "Edit Job Priorities" button, and set the pressed action to display the job priorities editor.
• Changes to ReloadCharacterPickers
  • When the picker buttons are created, add to the same button group as the edit job priorities button -- they should be tied together as a "radio button" group.
  • Pass the character profile into the CharacterPickerButton constructor instead of slot number.
• ReloadCharacterPickers will dispose and re-create the buttons entirely, I don't like this method and I'll look into improving it in a future PR`

Content.Client/Lobby/UI/HumanoidProfileEditor.xaml

• PreferencesUnavailableButton removed
• The SpriteView profile preview has been changed to a custom control ProfilePreview

Content.Client/Lobby/UI/HumanoidProfileEditor.xaml.cs

• Profile now holds the HumanoidCharacterProfile that is currently being edited and will reflect the current values of the controls in the editor.
• _savedProfile holds the "loaded" character profile, which reflects the profile before any edits were made. This should also reflect the profile that is stored on the server for this slot.
  • This is "loaded" by SetProfile(...)
• Code related to the preview dummy has been factored out to re-use the ProfilePreviewSpriteView and ProfilePreview
• With the meaning of these two values I hope that the changes to most of this file should make reasonable sense.
• Code for PreferenceUnavailableButton is removed.
• Code to select job priorities are changed to set job "preferences" (yes or no), identical to antag selection.

Content.Client/Lobby/UI/ProfileEditorControls/ProfilePreview.xaml

This is just a little wrapper for ProfilePreviewSpriteView that includes the rotate buttons and handles image export

Content.Client/Lobby/UI/ProfileEditorControls/ProfilePreview.xaml.cs

This is simple enough hopefully...

Content.Client/Lobby/UI/ProfileEditorControls/ProfilePreviewSpriteView.cs

This control inheriting SpriteView provides an easy way to display a preview of and and extract name, job, and loadout name overrides from a HumanoidCharacterProfile.

This file is a partial and provides the overall logic, serving as an entry point for the real meat of the class in ProfilePreviewSpriteView.Humanoid.cs

Content.Client/Lobby/UI/ProfileEditorControls/ProfilePreviewSpriteView.Humanoid.cs

Nearly ALL of this code is moved directly from the Helpers region of LobbyUIController.cs

I'm pretty sure there's really not much if anything changed here at all.

• in LoadHumanoidEntity, if a profile has all jobs set to Never, it will attempt to preview the profile as an antag with the new DataField PreviewStartingGear.

Content.Client/Lobby/UI/JobPriorityEditor.xaml

This is the simple main control for editing job priorities

Content.Client/Lobby/UI/JobPriorityEditor.xaml.cs

Nearly ALL of this code is copied from the job priority selector code from HumanoidProfileEditor.xaml.cs before I changed it to yes or no.

So most of this code is unchanged, except for the glue to connect the UI events to the CharacterSetupGui and Lobby/LobbyUIController

Content.Client/Lobby/UI/LateJoinGui.xaml

I've factored out the late join gui to a xaml file instead of being purely code generated.

It contains a CharacterList BoxContainer on the left and the JobList BoxContainer as before on the right.

Content.Client/Lobby/UI/LateJoinGui.xaml.cs

• The SelectedId event (when you click on a job to join) now includes the selected character slot to join as.
• Basically just use RebuildCharacterList to generate CharacterPickerButton elements with simple: true
  • When a picker button is pressed, set the private _selectedSlot for use when joining
  • Also rebuild the job list every time a character is selected in case there are job requirements blockers

Content.Client/Lobby/UI/LobbyCharacterPreviewPanel.xaml

This control used to display the preview of your selected character with that text "This is <NAME>, they are <XYZ> years old."

Now, it must present much more data about your entire set of selected job priorities and characters.

I tried to create most of the structure here in xaml.

There are four main boxes for each priority which contain GridContainers that will hold small icons of each job, except for the last one for High which just has one larger icon.

Content.Client/Lobby/UI/LobbyCharacterPreviewPanel.xaml.cs

• Load the preferences, and for each job:

  • Don't show the icon for a locked role
  • Put the icon into a TextureRect and dump it into the appropriate iconContainer mapped to priority
  • If there are no valid enabled profiles for a job, darken the icon to show this.
  • MouseFilter must be Pass so that tooltips will work...

• CreateJobTooltip will show a tooltip box that shows every character with that job enabled. These characters are shown with full names using the ProfilePreviewSpriteView control.

ROLE SELECTION AND ROUND START

Content.Server/GameTicking/GameTicker.RoundFlow.cs

• In StartRound, call SpawnPlayers with just the player net IDs instead of profiles.
  • SpawnPlayers called in this way will handle job assignment
• Remove character profiles from RulePlayerSpawningEvent
• Remove character profiles from RulePlayerJobsAssignedEvent

Content.Server/Station/Systems/StationJobsSystem.Roundstart.cs

• AssignJobs will take a set of NetUserId instead of a dictionary with profiles.
• AssignOverflowJobs is removed
• GetPlayersJobCandidates is where the meat is
  • It takes a set of NetUserId instead of dictionary with profiles
  • For each player it will aggregate all of the available jobs taking account the player priorities and enabled characters.
  • The StationJobsGetCandidatesEvent will further restrict the options as it did before
• Actual HumanoidCharacterProfiles will be selected later right before spawn time

Content.Server/GameTicking/GameTicker.Spawning.cs

• Remove character profiles from SpawnPlayers argument
• Remove call to AssignOverflowJobs
• In SpawnPlayers, after jobs are assigned, a HumanoidCharacterProfile will be chosen for the player right before actually spawning it.
• A call to SpawnPlayer override with a null Profile and no job selected will try to pick the "Best available job", and then pick a profile with that job.
• An override was added for MakeJoinGame that specifies the profile that player will be joining with.
• If a player observes from the lobby, the name of their ghost will tbe their player username, as there is no "selected character" to pull the name from.

Content.Shared/Roles/JobRequirements.cs

For some reason, job requirements like AgeRequirement and SpeciesRequirement are tightly coupled to the playtime tracking system.

If role timers aren't on globally via cvar, you can't check other job requirements.

I've changed the abstract class JobRequirements to accept null for the playTimes dictionary. If this is null, it will not check playtime counters, but still check other job requirements.

Content.Server/Players/PlayTimeTracking/PlayTimeTrackingSystem.cs

All of these functions have been changed to check playtimes optionally only if playTimes is not null.

• IsAllowed will return if the player has ANY enabled character that meets all requirements
• GetDisallowedJobs will return all jobs that the player does NOT have ANY enabled character that can play that job.

Content.Shared/Roles/JobRequirement/AgeRequirement.cs

use nullable playtimes

Content.Shared/Roles/JobRequirement/DepartmentTimeRequirement.cs

use nullable playtimes and return true if null

Content.Shared/Roles/JobRequirement/OverallPlaytimeRequirement.cs

use nullable playtimes and return true if null

Content.Shared/Roles/JobRequirement/RoleTimeRequirement.cs

use nullable playtimes and return true if null

Content.Shared/Roles/JobRequirement/SpeciesRequirement.cs

use nullable playtimes

Content.Shared/Roles/JobRequirement/TraitsRequirement.cs

use nullable playtimes

Content.Server/Antag/AntagSelectionSystem.API.cs

No selected character, so just ask if the player has any character enabled with these antag roles selected.

Content.Server/Antag/AntagSelectionSystem.cs

Add the AntagSelectionDefinition to the AntagSelectEntityEvent, this is used in AntagLoadProfileRuleSystem

Content.Server/GameTicking/Rules/AntagLoadProfileRuleSystem.cs

This will take and attempt to find an enabled profile on the player's session to use for this Antag. If it can't find one, it will use a random profile as it did before. This even works for ghost rules like Space Ninja!!!

Resources/Prototypes/GameRules/events.yml

Add prefRoles to the space ninja rule def so that AntagLoadProfileRuleSystem will work.

Resources/Prototypes/Roles/Antags/ninja.yml

Allow the user to set space ninja preference so that they can make a profile that will only be used for space ninja ghost role.

Add a starting gear preset for UI character editor antag preview

Resources/Prototypes/Roles/Antags/nukeops.yml

Add a starting gear preset for UI character editor antag preview

Resources/Prototypes/Roles/Antags/wizard.yml

Add a starting gear preset for UI character editor antag preview

INTEGRATION TESTS

Content.IntegrationTests/Pair/TestPair.Helpers.cs

• All job and antag helper functions are moved to TestPair.Jobs.cs
• AddDummyPlayers added which can add one or more players with a default job priority list. This just sets up the player with one character that will work like the old job priorities

Content.IntegrationTests/Pair/TestPair.Jobs.cs

This file now contains functions to set job priorities, job preferences, and antag preferences for a given ICommonSession returned by AddDummyPlayers

AssertJob was also moved here from Content.IntegrationTests/Tests/Round/JobTest.cs for re-use in other tests.

Content.IntegrationTests/Pair/TestPair.Recycle.cs

I work a little bit more carefully to clean up modified PlayerPrefences here

Basically delete every profile except slot 0, then set the profile 0 to a default humanoid, and set the job priorities to default.

Also catch an exception here and print out its info because oh god if there was a throw here it was impossible to debug

Content.IntegrationTests/Pair/TestPair.cs

Remove null suppression and return null for Player property if it doesn't exist, what use is it nullable otherwise? (This did cause a problem at some point somewhere somehow)

Content.IntegrationTests/Tests/GameRules/AntagPreferenceTest.cs

Use new signature for SetAntagPreference

Content.IntegrationTests/Tests/GameRules/NukeOpsTest.cs

Use new signature for SetAntagPreference

Content.IntegrationTests/Tests/GameRules/TraitorRuleTest.cs

Use new signature for SetAntagPreference

Content.IntegrationTests/Tests/Lobby/CharacterCreationTest.cs

No "selected character"

Content.IntegrationTests/Tests/Preferences/ServerDbSqliteTests.cs

No "selected character"

Content.IntegrationTests/Tests/Round/JobTest.cs

• Move AssertJob to TestPair.Jobs.cs
• Update these tests to use the new helpers.

Content.IntegrationTests/Tests/Station/StationJobsTest.cs

Update these tests to use the new helpers.

Content.IntegrationTests/Tests/Round/JobRequirementsTest.cs

• Add tests for AgeRequirement
• Add tests for SpeciesRequirement

MISCELLANY

Content.Client/Lobby/LobbyState.cs

I guess I removed an unused using directive.

Content.Shared/Roles/AntagPrototype.cs

Add a new DataField PreviewStartingGear to define a loadout to use when a character profile is previewed as an antag in the character editor

Content.Server/Administration/Commands/SetOutfitCommand.cs

This command assumed that a session had a "selected profile" to use the player's selected loadout when a job outfit is selected. This will now pull the loadout from the saved BaseProfile of the entity.

Content.Server/Administration/Systems/AdminVerbSystem.cs

"Spawn" and "Clone" admin verbs will use the saved BaseProfile on the entity.

Content.Server/GameTicking/Commands/JoinGameCommand.cs

The joingame console command now takes the character slot number. This command is directly used in the late join gui.

Content.Shared/GameTicking/PlayerBeforeSpawnEvent.cs

Allow null for Profile, used by deathmatch rule

Content.Server/GameTicking/Rules/DeathMatchRuleSystem.cs

If no Profile is given in the OnBeforeSpawn event here, it will randomly select any enabled profile for the player.

Content.Server/GameTicking/Rules/RespawnRuleSystem.cs

Respawn the player with the HumanoidAppearanceComponent.

[LaCumbiaDelCoronavirus]

Good rework, however the UI could use improvement:

• Huge box of characters that appears when you (assumedly) hover over jobs is unwieldy
• Feels like the job and antag priority should stay localised to each character in the setup menu and an overview of all character job priorities (as shown in lobby screen sidebar) isn't really necessary, it's more distracting than every other piece of UI among other reasons
• No need to show characters that are disabled for a certain job, just only show characters that have that job selected
• IMO buttons for enabling/deleting characters should keep the colors of the old button for deleting them (the general grey UI color that turns red when hovered over iirc)

[Quantum-cross]

* Huge box of characters that appears when you (assumedly) hover over jobs is unwieldy

1. The screenshots are showing me performing stress tests with large amounts of characters selected for a job
2. One of my biggest goals is ensuring that it is absolutely clear which jobs you are being considered for, and why.

Character creation settings have already been confusing enough for some new players, so seeing all of the jobs at a glance with priorities on the lobby screen is great. If an icon is darkened, you can hover over and see exactly why you won't be considered for that job (oh shit I forgot to ENABLE the character!)

* Feels like the job and antag priority should stay localised to each character in the setup menu and an overview of all character job priorities (as shown in lobby screen sidebar) isn't really necessary, it's more distracting than every other piece of UI among other reasons

I'm really not sure what you mean here.

* No need to show characters that are disabled for a certain job, just only show characters that have that job selected

I specifically did this to ensure that you can easily tell why a job isn't being selected.

* IMO buttons for enabling/deleting characters should keep the colors of the old button for deleting them (the general grey UI color that turns red when hovered over iirc)

I had difficulty with this because there are buttons on buttons. The "Enabled" button will be green when on, and the character picker "button" will also be green, and then they will blend into each other perfectly. This is why I created an outline for the buttons.

[Absotively]

So far I've mostly looked at the database changes at a high level and at how old and empty database states are handled. I have looked at the lobby changes a bit while checking database handling. I have a few suggestions:

• When an existing database is migrated, all characters end up with the same jobs on as before the migration, but all characters are enabled and all job priorities are set to "Never". I think it might be better to set the job priorities for each preference to match the profile that was selected for that preference before the migration, and enable only that profile, so that the effective state of each player's settings is unchanged by the migration.

• It might be worth considering changing the name of JobPriorityEntry to something along the lines of PreferenceJobPriority or PreferenceJob, since being directly associated with a PlayerPreference is its distinguishing new feature. "Entry" doesn't really add much here in my opinion.

• It is possible to ready up with all jobs on "Never", or with all non-"Never" jobs having no characters enabled. I think it would be clearer for the ready/not ready button to be disabled in this circumstance, since the game can't actually spawn the player at round start. Probably some kind of message or tooltip would also be needed to tell the player why they can't ready up.

• It would be helpful if the tooltip with characters that shows when you hover over a job icon also included the job name, for the benefit of players who don't know all the icons. It does help that it shows the characters in the relevant loadout, though, I like that.

• The heading for the lobby character/job preview is still "Characters" even though the jobs are now more prominently displayed than the characters are. I think a different heading would be clearer. I'll suggest "Round start settings" for now but it's a bit unwieldy and hopefully someone who isn't me can suggest something better.

• It may be worth considering changing Enabled and Disabled to other terms such as Active and Inactive in user visible text. There's a general trend towards avoiding enabled/disabled for software features outside of the sort of technical contexts where it's the most widely understood option. I do think enable/disable are fine in code here, that's certainly one such technical context.

• It is probably worth noting that setting Passenger to Low is not quite equivalent to the old PreferenceUnavailable setting. The old setting would only take effect if all the player's other selected jobs were unavailable; setting Passenger to Low instead puts it on an even footing with other Low priority jobs. On the other hand, I'm not sure how often this is likely to be relevant in practice.

For the database stuff, I have only looked at SQLite so far, but if you haven't made modifications to the auto generated migrations then I don't think there should be any relevant differences in Postgres. If logic is added to the migrations then they will definitely need to both be tested. I'd be happy to take a swing at modifying the migrations to preserve preferences if you'd rather avoid mucking with SQL, but I admit I'm not as fast at getting to things as I'd like.

[Quantum-cross]

So far I've mostly looked at the database changes at a high level and at how old and empty database states are handled. I have looked at the lobby changes a bit while checking database handling. I have a few suggestions:

Thank you for looking over this, I'll address your comments individually.

* When an existing database is migrated, all characters end up with the same jobs on as before the migration, but all characters are enabled and all job priorities are set to "Never". I think it might be better to set the job priorities for each preference to match the profile that was selected for that preference before the migration, and enable only that profile, so that the effective state of each player's settings is unchanged by the migration.

I went back and forth on refining the database migration. I eventually ended up not touching it because "it worked".

After thinking about it now and now that the database structure has settled, I think I will look into disabling all the characters except for the one that was currently selected before the migration to closely mimic previous behavior upon migration. I'll do the same for the job priorities.

I think it would be interesting to try and implement a one time pop-up for the first time a player is logged in after the change. I think there's a "time last logged in" field which is probably used for things like this (rule changes, changelogs, etc) that so I could do this without introducing more data into the database.

* It might be worth considering changing the name of JobPriorityEntry to something along the lines of PreferenceJobPriority or PreferenceJob, since being directly associated with a PlayerPreference is its distinguishing new feature. "Entry" doesn't really add much here in my opinion.

I understand where you're coming from here. I originally used the words "Preference" "Priority" "Job" kind of without regard to what they mean and things got very confusing.

I will first define my thought process on my definitions here, and then address your comments further

Before my changes, Antag settings are called AntagPreference, these represent yes or no, "I want to be considered for this antagonist". I am following this pattern and precisely defining the following terms:

• XYZPreference: Boolean setting that says that something wants to be considered for or allowed to be XYZ. I'm defining this term with regards to the fact that a Preference might be assigned to a player instead of a character for some future setting.
  • AntagPreference: A character wants to be considered for a certain antagonist.
  • JobPreference: A character wants to be considered for a certain Job.
• XYZPriority: A range from Never to High (1 to 4) as the the level that something wants to be considered for XYZ.
  • JobPriority: The priority level that a Player wants to be considered to be assigned a certain Job.

Previously, on the database side in Model.cs, we had:

• class Job
  • Represents a job and its priority setting on a character profile
  • Connected to Profile (character profile)
  • Contains
    • string JobName
    • enum DbJobPriority

Now we have:

• class Job (I didn't want to change the name of this to minimize database migration changes)

  • Represents a job preference on a character profile
  • Connected to Profile (character profile)
  • Contains
    • string JobName

• class JobPriorityEntry

  • Represents a job and its priority setting on a PlayerPreferences object
  • Connected to Preference (a player's full set of preferences)
  • Contains
    • string JobName
    • enum DbJobPriority

It is unfortunate that the player preferences class on the database side is just called Preference. I believe that it would be more clear if it mirrored the game side object PlayerPreferences.

Additionally, it would be nice if the database side classes that mirrored game side objects be prefixed with Db like DbJobPriority.

It feels like the code style Model.cs code is probably a little stale compared to the rest of the repository, with good reason, as database changes should probably only be made when required.

I feel like my naming schemes here are kind of the minimal set of changes I need to represent the new structures, with reasonable clarity, without touching things that don't need to be touched.

As for the Entry suffix on JobPriorityEntry, I wanted to ensure that the name of JobPriority in Model.cs did not clash with the name Content.Shared.Preferences.JobPriority, which caused the need to specify the entire namespace in many places of the code in order to resolve the correct symbol.

* It is possible to ready up with all jobs on "Never", or with all non-"Never" jobs having no characters enabled. I think it would be clearer for the ready/not ready button to be disabled in this circumstance, since the game can't actually spawn the player at round start. Probably some kind of message or tooltip would also be needed to tell the player why they can't ready up.

I agree with this. I want round start character creation and selection issues to be as clear as possible. I will implement this.

* It would be helpful if the tooltip with characters that shows when you hover over a job icon also included the job name, for the benefit of players who don't know all the icons. It does help that it shows the characters in the relevant loadout, though, I like that.

I 95% agree with this, there are three cases in the tooltip:

1. There are no characters with the job preference period. 👍 This will result in a popup that says something to the effect of "You have no characters with {JOB}!"
2. There is at least one character with the job preference, and at least one of them are enabled 👍 This will result in the character listing, and the ones that are enabled will have their job title listed next to the character
3. There is at least one character with the job preference, and ALL of them are DISABLED 👎 This will result in the character listing, but all of the job titles will be replaced with "Disabled", leaving a new player still unable to tell which job it is.

Solution: A small heading on the tooltip. I will implement this

* The heading for the lobby character/job preview is still "Characters" even though the jobs are now more prominently displayed than the characters are. I think a different heading would be clearer. I'll suggest "Round start settings" for now but it's a bit unwieldy and hopefully someone who isn't me can suggest something better.

It was previously "Character", which I changed to "Characters" because the summary is a full amalgamation of all of your character settings combined. If anyone has an idea to change it to something more clear (especially for new players) that isn't so wordy I'm open for it.

* It may be worth considering changing Enabled and Disabled to other terms such as Active and Inactive in user visible text. There's a general trend towards avoiding enabled/disabled for software features outside of the sort of technical contexts where it's the most widely understood option. I do think enable/disable are fine in code here, that's certainly one such technical context.

Never thought of this, Active and Inactive are just as precise and I can change this in the localizations, but I'm hesitant to change it in code as it touches so much. I will implement this in localization.

* It is probably worth noting that setting Passenger to Low is not quite equivalent to the old PreferenceUnavailable setting. The old setting would only take effect if all the player's other selected jobs were unavailable; setting Passenger to Low instead puts it on an even footing with other Low priority jobs. On the other hand, I'm not sure how often this is likely to be relevant in practice.

I was aware of this and I may have lied a little when I said it's equivalent. The previous PreferenceUnavailable setting is basically a setting between "never" and "low" that can only be applied to passenger. After discussion in discord, I got the feeling that I could just remove it as it added a lot of weird edge case complexity.

If this causes a problem, an argument could be made for one or two more levels of priority to be added to the JobPriority enumerator in the future to compensate for this.

For the database stuff, I have only looked at SQLite so far, but if you haven't made modifications to the auto generated migrations then I don't think there should be any relevant differences in Postgres. If logic is added to the migrations then they will definitely need to both be tested. I'd be happy to take a swing at modifying the migrations to preserve preferences if you'd rather avoid mucking with SQL, but I admit I'm not as fast at getting to things as I'd like.

I will take a stab at migrating the currently selected character settings to the new structure. I'll let you know if I have any problems.

If you could help test it on postgres when I push it I would appreciate it.

Thank you so much for the feedback.

[github-actions[bot]]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions[bot]]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[BigfootBravo]

Blessed PR. This would incentivize me playing my other characters more often, and to even make more of em!

[Quantum-cross]

* It has now occurred to me that it may actually have been possible for a player who readies with no jobs available to be spawned, if they happen to roll nukie or wizard. But I think it's still probably for the best that this is now disallowed, as it would essentially be antag rolling.

Now you can't click on the ready button unless you have an enabled character with a job enabled compatible with your job priorities.

For sure this is enforced client side, it might be prudent to enforce that a player can't be in the ready state with this configurationon the server.

[Quantum-cross]

* The lobby preview panel can in some cases get quite tall and leave the chat box rather short. I don't know the UI system well enough to suggest any fixes off the top of my head.

This is worse if you're compiling in debug -- as all job requirements are lifted.

Generally your assistant roles will become locked after some time, well before your command roles, which keeps the total number of roles displayed on this panel somewhat reasonable.

[northerndakotas]

How does it work when one character has antags enabled, but also only jobs that can't roll antag, and another one of your characters doesn't have antags enabled but does have jobs that can be antags?

[Absotively]

Noting here before I forget that I was wrong about the down migrations still being only auto-generated, and I didn't test them when I tested the up migrations.

[superjj18]

How does it work when one character has antags enabled, but also only jobs that can't roll antag, and another one of your characters doesn't have antags enabled but does have jobs that can be antags?

Assuming antag rolling is implemented correctly(haven't seen any evidence or personal experience with it yet), antags roll first and send you to a antag-friendly role if rolled, followed by command, followed by standard crew.

Again not entirely convinced this system is working as intended according to past PRs

[northerndakotas]

How does it work when one character has antags enabled, but also only jobs that can't roll antag, and another one of your characters doesn't have antags enabled but does have jobs that can be antags?

Assuming antag rolling is implemented correctly(haven't seen any evidence or personal experience with it yet), antags roll first and send you to a antag-friendly role if rolled, followed by command, followed by standard crew.

Again not entirely convinced this system is working as intended according to past PRs

I'm pretty sure it is working. There have been many times I have ended up as a engi, or bartender even though I had a bunch of command rolls enabled and I ended up being an antag.

That aside, my question was this: Let's say you queue up and the game chooses you to be a syndicate agent, but the only character you have with syndicate agent enabled can only spawn as Captain or HoS, and has every other job disabled. What will the game do in that situation?

[Quantum-cross]

That aside, my question was this: Let's say you queue up and the game chooses you to be a syndicate agent, but the only character you have with syndicate agent enabled can only spawn as Captain or HoS, and has every other job disabled. What will the game do in that situation?

Just tested this, I had one character with passenger syndicate on low, one character with CE syndicate on high.

Forced traitor and started the round like 10 times, always made me passenger syndicate (set the traitor gamerule delay to like 5 seconds)

Then I disabled the passenger character, and it just fails to join you into the game (you stay in the lobby). I'm not sure that's ideal but at the very least it doesn't give you command antags or anything.