Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removes redundant and misleading alt-titles, adds title_blurbs to a lot that remain, adds one or two more alt-titles #6056

Merged
merged 10 commits into from
Oct 20, 2023

Conversation

SingingSpock
Copy link
Contributor

About The Pull Request

Trims out a handful of alt-titles that were misleading and/or redundant, and adds title_blurb text where missing to most of the ones that remain in files I touched, to help establish what each alt-title is for. (I then realized that nothing shows the title_blurb in-game, but that's a problem for another PR). The departmental senior titles were also given a desc that indicates their actual role in our current SOP (including Senior Researcher, whose desc was previously literally "Lorem Ipsum." There's also a couple alt-titles, most notably Trainee EMT, since there wasn't a good role for that previously. Exact details of the changes are too long to put in here, but it's pretty easily readable. The most significant alteration was changing almost all the titles for the senior medical position, removing "Head Nurse" (because that makes no sense whatsoever as a role of authority that doctors would look towards for advice) and making "Senior Physician" the primary title.

Why It's Good For The Game

Alt-titles were often just a dozen ways to say kinda the same thing. In the worst cases, they were actively confusing (e.g. Counselor as an alt-title for both Chaplain and Psychiatrist) or not indicative at all of the original job (e.g. Director of Operations for Captain). This leaves each alt-title in non-service departments having a distinct role that they fill, indicated by title_blurb text (which I will work on fixing next since I noticed it isn't actually shown in-game). Additionally, a couple of niches weren't previously covered by alt-titles (such as Trainee EMT), so those were added.

Changelog

🆑 SingingSpock
tweak: Alt-titles were trimmed down and more description given to many remaining ones. A couple more were added.
/:cl:

@github-actions github-actions bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 5, 2023
@SingingSpock
Copy link
Contributor Author

I don't know why the linter is still complaining, I re-added the trailing newlines to those files

silicons pushed a commit that referenced this pull request Oct 8, 2023
Copy link
Contributor

@silicons silicons left a comment

Choose a reason for hiding this comment

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

blah blah subjective nitpicks

"Crew Resources Officer" = /datum/prototype/struct/alt_title/cro,
"First Officer" = /datum/prototype/struct/alt_title/fo,
Copy link
Contributor

Choose a reason for hiding this comment

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

don't remove CRO

@@ -1,6 +1,6 @@
/datum/role/job/station/engineer
id = JOB_ID_STATION_ENGINEER
title = "Station Engineer"
title = "Engineer"
Copy link
Contributor

Choose a reason for hiding this comment

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

i don't really see the need to shorten this to engineer

code/modules/jobs/job_types/station/medical/chemist.dm Outdated Show resolved Hide resolved
code/modules/jobs/job_types/station/science/roboticist.dm Outdated Show resolved Hide resolved
"Prosthetists" = /datum/prototype/struct/alt_title/prosthetists,
"Artificer-Specialist" = /datum/prototype/struct/alt_title/artificer_specialist
"Biomechanical Technician" = /datum/prototype/struct/alt_title/biomech,
"Mechatronic Technician" = /datum/prototype/struct/alt_title/mech_tech
Copy link
Contributor

Choose a reason for hiding this comment

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

engineer sounds cooler for both tbh; why are we changing it?

@@ -34,15 +34,16 @@
Armoury gear in a crisis, and retrieving it when the crisis has passed. In an emergency, the Warden may be called upon to direct the \
Security Department as a whole."
alt_titles = list(
"Jailor" = /datum/prototype/struct/alt_title/warden/jailor,
"Brig Overseer" = /datum/prototype/struct/alt_title/warden/overseer,
Copy link
Contributor

Choose a reason for hiding this comment

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

ngl this is such a silly title but whatever idc

Copy link
Collaborator

@BlueWildrose BlueWildrose left a comment

Choose a reason for hiding this comment

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

Also, it'd be neat if the shaft miner had some alt titles to do with salvaging. Chances are that might become their default titles eventually anyways.
Salvage Technician, Salvage Specialist, or something. Maybe both.

@@ -1,5 +1,5 @@
/datum/role/job/station/head_nurse
title = "Head Nurse"
/datum/role/job/station/senior_physician
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like changing from head_nurse to this might mess with those who have this as their job, among a few other things like job icons. Maybe - I can't be completely sure. Changing this should not be treated lightly.

@silicons
Copy link
Contributor

@timothyteakettle

@Captain277 Captain277 dismissed timothyteakettle’s stale review October 20, 2023 09:01

Teak approved the changes, but is unavailable. 10/20/2023 2 AM PST

@Captain277 Captain277 merged commit 3c1e91b into Citadel-Station-13:master Oct 20, 2023
silicons pushed a commit that referenced this pull request Oct 29, 2023
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may
not be viewable. -->
<!-- You can view Contributing.MD for a detailed description of the pull
request process. -->

## About The Pull Request

This PR directly reverts #6056's changes to the Facility Director (i.e.
renaming the default title to Captain, among a couple other things).
This fixes a bug where none of the FD-specific loadout items could be
equipped even as the Captain, as those items were checking for the
"Facility Director" title, not the "Captain" title. (Alt-titles don't
count toward this.) This same bug may be present in other jobs that have
had their default titles changed, but that is beyond the scope of this
PR.

This PR has been tested locally.

## Why It's Good For The Game

Nothing else in the community (i.e. the wiki or other parts of the code)
have been updated to reflect this sudden change to the default title of
the head of the facility, so I think that should be taken care of
first/at the same time. In addition, the bug that I am directly fixing
is rather egregious and probably should have been caught earlier.

In any case, there is nothing wrong with continuing to call the head of
the installation the Facility Director, and players who wish to continue
being called Captain can simply change their alt title.

## Changelog

<!-- If your PR modifies aspects of the game that can be concretely
observed by players or admins you should add a changelog. If your change
does NOT meet this description, remove this section. Be sure to properly
mark your PRs to prevent unnecessary GBP loss. You can read up on GBP
and it's effects on PRs in the tgstation guides for contributors. Please
note that maintainers freely reserve the right to remove and add tags
should they deem it appropriate. You can attempt to finagle the system
all you want, but it's best to shoot for clear communication right off
the bat. -->

:cl:
tweak: Renamed Captain to Facility Director by default, which should fix
a bug where FD-specific items could not be equipped even as the Captain.
/:cl:

<!-- Both :cl:'s are required for the changelog to work! You can put
your name to the right of the first :cl: if you want to overwrite your
GitHub username as author ingame. -->
<!-- You can use multiple of the same prefix (they're only used for the
icon ingame) and delete the unneeded ones. Despite some of the tags,
changelogs should generally represent how a player might be affected by
the changes rather than a summary of the PR's contents. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants