Various Skill Bugfixes (#22533)

This PR fixes a bunch of skills related bugs, the biggest of which were
the result of the system being overly trusting of the database, when in
reality due to a bunch of unpredictable edge cases, the database is not
guaranteed to always have what I think it has. To fix these bugs, I've
had to slightly refactor how skills are generated on player characters
and antagonists, such that the burden of proof for skills is with the
Skills Subsystem rather than the Database.

Skills generated for a fresh character that has NO preferences saved
(Worst case scenario):

<img width="1491" height="849" alt="image"
src="https://github.com/user-attachments/assets/683fb538-106a-4679-8e2a-30dd45f456a1"
/>

Promoting that same character to Antagonist now increases certain skills
to a minimum baseline:

<img width="1315" height="830" alt="image"
src="https://github.com/user-attachments/assets/94bcc69e-cbbe-45fa-956e-f53b7f5d2779"
/>

By Mel's request, Bluespace Technicians spawn with all skills fully
maxed out for debugging purposes:

<img width="1909" height="985" alt="image"
src="https://github.com/user-attachments/assets/db4f3f65-3d87-47fc-907e-c68eae6d4cdb"
/>
This commit is contained in:
VMSolidus
2026-05-29 20:37:57 +00:00
committed by GitHub
parent 3aa4942572
commit 13f87e4a26
9 changed files with 70 additions and 21 deletions
+18 -3
View File
@@ -5,7 +5,16 @@ SUBSYSTEM_DEF(skills)
/// This is essentially the list we use to read skills in the character setup.
var/list/skill_tree = list()
/// The set of all skills that are "forced" in order to guarantee necessary components are applied.
/**
* The set of all known skill singletons.
* These are always typed as /singleton/skill and as such are safe to for(var/singleton/skill/skill as anything in SSskills.all_skills)
*/
var/list/all_skills = list()
/**
* The set of all skills that are "forced" in order to guarantee necessary components are applied.
* These are always typed as /singleton/skill and as such are safe to for(var/singleton/skill/skill as anything in SSskills.required_skills)
*/
var/list/required_skills = list()
/datum/controller/subsystem/skills/Initialize()
@@ -20,8 +29,9 @@ SUBSYSTEM_DEF(skills)
// Next, we add the empty subcategory lists if they're not present. At this point, the tree would look like "Combat" -> "Melee" -> empty list
// After that's done, if our skill is not present, add it to the empty list of the subcategory.
for(var/singleton/skill/skill as anything in GET_SINGLETON_SUBTYPE_LIST(/singleton/skill))
if (skill.required)
required_skills += skill.type
all_skills += skill
if (skill.required && skill.component_type)
required_skills += skill
var/singleton/skill_category/skill_category = GET_SINGLETON(skill.category)
if(!(skill.subcategory in skill_tree[skill_category]))
skill_tree[skill_category] |= skill.subcategory
@@ -30,3 +40,8 @@ SUBSYSTEM_DEF(skills)
if(!(skill in skill_tree[skill_category][skill.subcategory]))
skill_tree[skill_category][skill.subcategory] |= skill
return SS_INIT_SUCCESS
/datum/controller/subsystem/skills/Destroy()
all_skills.Cut()
required_skills.Cut()
return ..()
@@ -22,13 +22,16 @@
/datum/action/leadership/proc/get_target(owner, atom/target, modifiers)
SIGNAL_HANDLER
UnregisterSignal(owner, COMSIG_MOB_CLICKON)
if (. == COMSIG_MOB_CANCEL_CLICKON)
return . // Another signal-handler already got to it.
// Both forms of deliver speech will immediately return control to the caller without blocking.
// Unfortunately StrongDMM is apparently incapable of reading that.
if (owner == target)
UNLINT(deliver_speech_area(owner))
else
UNLINT(deliver_speech_target(owner, target))
UnregisterSignal(owner, COMSIG_MOB_CLICKON)
return COMSIG_MOB_CANCEL_CLICKON
/datum/action/leadership/proc/deliver_speech_area(mob/owner)
@@ -27,22 +27,26 @@
/datum/action/ministry/proc/get_target(owner, atom/target, modifiers)
SIGNAL_HANDLER
UnregisterSignal(owner, COMSIG_MOB_CLICKON)
if (. == COMSIG_MOB_CANCEL_CLICKON)
return . // Another signal-handler already got to it somehow.
. = COMSIG_MOB_CANCEL_CLICKON
if (owner == target)
to_chat(owner, SPAN_NOTICE("You cannot offer a blessing to yourself."))
return COMSIG_MOB_CANCEL_CLICKON
return .
if (!astype(target, /mob)?.client)
to_chat(owner, SPAN_NOTICE("[target] cannot receive a blessing."))
return COMSIG_MOB_CANCEL_CLICKON
return .
if (get_dist(owner, target) >= 2)
to_chat(owner, SPAN_NOTICE("You must be adjacent to [target] to offer them a blessing."))
return COMSIG_MOB_CANCEL_CLICKON
return .
// StrongDMM for whatever ungodly reason can't tell that this proc won't block the caller.
UNLINT(try_give_blessing(target))
UnregisterSignal(owner, COMSIG_MOB_CLICKON)
return COMSIG_MOB_CANCEL_CLICKON
return .
/datum/action/ministry/proc/try_give_blessing(mob/target)
set waitfor = FALSE
+11 -3
View File
@@ -27,10 +27,18 @@
/// The sub-category of this skill. Used to better sort skills.
var/subcategory
/**
* Required skills are always included in the user's saved skills preference, even if it's at the lowest rank.
* This is needed for skills that rely on components.
* Required skills are always loaded at a minimum skill level of 1 regardless of if they were saved to the preferences.
* This is useful for skills that provide a penalty below a certain skill level.
*/
var/required = FALSE
/**
* For required skills on humanoid antags, they are always guaranteed a minimum of this skill level.
* If they have the skill previously at a lower level, their skill level is increased to this upon being promoted to antagonist.
* If they don't have the skill at all, they are given the skill at this level.
*/
var/antag_level = SKILL_LEVEL_TRAINED
/// The component datum that this skill will add during character spawning
var/component_type = null
/// Map of skill levels to level costs. How many skill points it takes to purchase this rank in a skill.
@@ -74,7 +82,7 @@
*/
/singleton/skill/proc/on_spawn(mob/owner, skill_level)
SHOULD_CALL_PARENT(TRUE)
if (!owner || !component_type)
if (!owner || !component_type || (!required && skill_level == SKILL_LEVEL_UNFAMILIAR))
return
owner.AddComponent(component_type, skill_level)
+7 -3
View File
@@ -20,9 +20,13 @@
announce_antagonist_spawn()
LAZYDISTINCTADD(SSticker.mode.antag_templates, src)
// Antags wipe skill components so that they can bypass skill restrictions entirely.
for(var/skill in target.current.GetComponents(/datum/component/skill))
qdel(skill)
// Antags are always guaranteed certain minimum skill levels.
// Such that if a player character is promoted to an antagonist, they are always boosted up to a minimum competence required for antagging.
for(var/singleton/skill/skill as anything in SSskills.required_skills)
var/antag_skill_rank = skill.antag_level
var/datum/component/skill/skill_comp = target.current.LoadComponent(skill.component_type, antag_skill_rank)
if (skill_comp.skill_level < antag_skill_rank)
skill_comp.skill_level = antag_skill_rank
/datum/antagonist/proc/create_default(var/mob/source)
var/mob/living/M
+5 -1
View File
@@ -127,6 +127,10 @@
feedback_add_details("admin_verb","BST")
// Load all skills as max for a bst
for (var/singleton/skill/skill as anything in SSskills.all_skills)
skill.on_spawn(bst, skill.maximum_level)
return 1
/client/proc/bst_post_spawn(mob/living/carbon/human/bst/bst)
@@ -141,7 +145,7 @@
var/datum/weakref/original_mob
/mob/living/carbon/human/bst/Destroy(force)
original_mob = null
QDEL_NULL(original_mob)
return ..()
/mob/living/carbon/human/bst/can_inject(var/mob/user, var/error_msg, var/target_zone)
@@ -68,11 +68,6 @@
loaded_skills = list()
pref.skills = list()
for(var/key in SSskills.required_skills)
var/singleton/skill/skill = GET_SINGLETON(key)
if (istype(skill))
pref.skills[skill.type] = SKILL_LEVEL_UNFAMILIAR
for(var/key,value in loaded_skills)
if (!key)
continue
+6
View File
@@ -528,10 +528,16 @@ GLOBAL_LIST_EMPTY_TYPED(preferences_datums, /datum/preferences)
if(istype(P) && (P.ability_flags & PSI_FLAG_CANON))
P.apply(character)
// Load all of the player-set skills first.
for(var/skill_type in skills)
var/singleton/skill/skill = GET_SINGLETON(skill_type)
skill.on_spawn(character, skills[skill.type])
// Attempt to load all the "required" skills.
// Player-set skills won't be overwritten here as LoadComponent will never re-initialize a component that already exists.
for(var/singleton/skill/required_skill as anything in SSskills.required_skills)
character.LoadComponent(required_skill.component_type, SKILL_LEVEL_UNFAMILIAR)
if(icon_updates)
character.force_update_limbs()
character.update_mutations(0)
@@ -0,0 +1,10 @@
author: Hellfirejag
delete-after: True
changes:
- bugfix: "Hopefully fixed a hard delete with Bluespace Techs."
- rscadd: "Bluespace Techs now spawn with all skills fully maxed out"
- rscadd: "Upon being promoted to Antagonist, player characters now have certain skills (such as Firearms) always increased to at least a minimum level of competence."
- bugfix: "Fixed a bug where both Leadership and Ministry could apply to the same mouse click."
- bugfix: "Fixed Ministry not cancelling if you fail to click on a valid target."
- bugfix: "Fixed player characters who have never visited the Skills menu failing to generate required skills."
- bugfix: "Fixed player characters spawning with non-required skills (like Ministry) at rank 1."