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

Fix global list lookups by name for 3 GLOBs #6421

Merged
merged 3 commits into from
Jun 18, 2024

Conversation

Drulikar
Copy link
Contributor

@Drulikar Drulikar commented Jun 10, 2024

About the pull request

This PR adds key uniqueness checks for GLOB.gear_name_presets_list, GLOB.all_yautja_capes, and GLOB.all_species and rectifies various /datum/equipment_preset that offended this check. Any that conflicted like this would just not get displayed or looked up correctly. This change applies to more than just the create_humans list. Species and predator cloaks had no offenders to correct.

This does make inheritance a bit weird (see changes needed for /datum/equipment_preset/synth/survivor/wy that wanted to share an access list), but should we want to address that we need to define when a type is abstract so setup_gear_name_presets knows to skip it and ensure all inherited types unset the abstract setting. This relies on the equipment flags being set to EQUIPMENT_PRESET_STUB when abstract, and then unset for non-abstract types. I also applied this change to predator cloaks and species which do the same thing, but they had no offenders.

Explain why it's good for the game

Fixes all these key collisions (and more if any had multiple collisions):

[22:39:26]RUNTIME: USCM Surgeon from /datum/equipment_preset/uscm_ship/uscm_medical/doctor/surgeon overlaps with /datum/equipment_preset/uscm_ship/uscm_medical/doctor! It must have a unique name for lookup! - code/__HELPERS/unsorted.dm@1467

[22:39:26]RUNTIME: Combat Correspondent from /datum/equipment_preset/uscm_ship/reporter_uscm overlaps with /datum/equipment_preset/uscm_ship/reporter! It must have a unique name for lookup! - code/__HELPERS/unsorted.dm@1467

[22:39:26]RUNTIME: USCM Senior Enlisted Advisor (SEA) from /datum/equipment_preset/uscm_ship/sea/access overlaps with /datum/equipment_preset/uscm_ship/sea! It must have a unique name for lookup! - code/__HELPERS/unsorted.dm@1467

[22:39:26]RUNTIME: USCM Squad Smartgunner from /datum/equipment_preset/uscm/sg/full overlaps with /datum/equipment_preset/uscm/sg! It must have a unique name for lookup! - code/__HELPERS/unsorted.dm@1467

[22:39:26]RUNTIME: Survivor - PMC from /datum/equipment_preset/survivor/pmc/miner overlaps with /datum/equipment_preset/survivor/pmc! It must have a unique name for lookup! - code/__HELPERS/unsorted.dm@1467

[22:39:26]RUNTIME: CLF Survivor from /datum/equipment_preset/survivor/clf/cold overlaps with /datum/equipment_preset/survivor/clf! It must have a unique name for lookup! - code/__HELPERS/unsorted.dm@1467

[22:39:26]RUNTIME: Corpse from /datum/equipment_preset/corpse/wy overlaps with /datum/equipment_preset/corpse! It must have a unique name for lookup! - code/__HELPERS/unsorted.dm@1467

[22:39:26]RUNTIME: Tutorial from /datum/equipment_preset/tutorial/fed overlaps with /datum/equipment_preset/tutorial! It must have a unique name for lookup! - code/__HELPERS/unsorted.dm@1467

[22:39:26]RUNTIME: Whiteout Team Medic from /datum/equipment_preset/pmc/w_y_whiteout/low_threat/medic overlaps with /datum/equipment_preset/pmc/w_y_whiteout/medic! It must have a unique name for lookup! - code/__HELPERS/unsorted.dm@1467

[22:39:26]RUNTIME: Survivor - Synthetic - Classic Joe from /datum/equipment_preset/synth/survivor/wy overlaps with /datum/equipment_preset/synth/survivor! It must have a unique name for lookup! - code/__HELPERS/unsorted.dm@1467

[22:39:26]RUNTIME: Survivor - Synthetic - Classic Joe from /datum/equipment_preset/synth/survivor/corporate_synth overlaps with /datum/equipment_preset/synth/survivor/wy! It must have a unique name for lookup! - code/__HELPERS/unsorted.dm@1467

[22:39:26]RUNTIME: Survivor - Synthetic - Classic Joe from /datum/equipment_preset/synth/survivor/cmb overlaps with /datum/equipment_preset/synth/survivor/corporate_synth! It must have a unique name for lookup! - code/__HELPERS/unsorted.dm@1467

[22:39:26]RUNTIME: USCM O-7 - Brigadier General (High Command) from /datum/equipment_preset/uscm_event/general/o7 overlaps with /datum/equipment_preset/uscm_event/general! It must have a unique name for lookup! - code/__HELPERS/unsorted.dm@1467

Testing Photographs and Procedure

Screenshots & Videos

Put screenshots and videos here with an empty line between the screenshots and the <details> tags.

Changelog

🆑 Drathek
fix: Fixed various equipment_presets not getting cached or looked up correctly
/:cl:

@github-actions github-actions bot added the Fix Fix one bug, make ten more label Jun 10, 2024
Copy link
Contributor

This PR has been inactive for long enough to be automatically marked as stale. This means it is at risk of being auto closed in ~ 7 days, please address any outstanding review items and ensure your PR is finished, if these are all true and you are auto-staled anyway, you need to actively ask maintainers if your PR will be merged. Once you have done any of the previous actions then you should request a maintainer remove the stale label on your PR, to reset the stale timer. If you feel no maintainer will respond in that time, you may wish to close this PR youself, while you seek maintainer comment, as you will then be able to reopen the PR yourself

@github-actions github-actions bot added the Stale beg a maintainer to review your PR label Jun 18, 2024
@Drulikar Drulikar added Stale Exempt PR can't go stale and removed Stale beg a maintainer to review your PR labels Jun 18, 2024
@harryob
Copy link
Member

harryob commented Jun 18, 2024

This does make inheritance a bit weird (see changes needed for /datum/equipment_preset/synth/survivor/wy that wanted to share an access list), but should we want to address that we need to define when a type is abstract so setup_gear_name_presets knows to skip it and ensure all inherited types unset the abstract setting

typically, this could be done with a new var on /equipment_preset like var/abstract_type, and for an abstract type, you set it equal to the typepath of the type itself. so you can check if something's abstract with

if(abstract_type == type)

do you want to implement that here? the inheritance here does feel a bit odd

@Drulikar Drulikar marked this pull request as draft June 18, 2024 21:04
@Drulikar
Copy link
Contributor Author

This does make inheritance a bit weird (see changes needed for /datum/equipment_preset/synth/survivor/wy that wanted to share an access list), but should we want to address that we need to define when a type is abstract so setup_gear_name_presets knows to skip it and ensure all inherited types unset the abstract setting

typically, this could be done with a new var on /equipment_preset like var/abstract_type, and for an abstract type, you set it equal to the typepath of the type itself. so you can check if something's abstract with

if(abstract_type == type)

do you want to implement that here? the inheritance here does feel a bit odd

That's not a bad idea, however its already got a flag its checking for EQUIPMENT_PRESET_STUB so I will just opt to use that.

@Drulikar Drulikar marked this pull request as ready for review June 18, 2024 21:54
@harryob harryob added this pull request to the merge queue Jun 18, 2024
Merged via the queue into cmss13-devs:master with commit ed2fdac Jun 18, 2024
28 checks passed
cm13-github added a commit that referenced this pull request Jun 18, 2024
@Drulikar Drulikar deleted the Fix_Global_List_Name_Lookups branch June 18, 2024 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Fix one bug, make ten more Stale Exempt PR can't go stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants