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 fluff type check for map overrides #4400

Merged
merged 2 commits into from
Sep 14, 2023

Conversation

Drulikar
Copy link
Contributor

@Drulikar Drulikar commented Sep 12, 2023

About the pull request

This PR fixes an issue caused by a path remapping in #4245 where a typecheck for an old fluff value no longer triggered to prevent map specific camos on the fluff gear.

Note: NO_SNOW_TYPE is more than just snow, but also not all item types even perform a test on that flag to prevent calling select_gamemode_skin.

Explain why it's good for the game

Fixes #4398 (Or any /obj/item/clothing/suit/storage/marine/light/fluff)

Changelog

🆑 Drathek
fix: Fix some donor items breaking on maps with camos.
fix: Fix donor items not having a fingerprint atom flag and donor armors not having conductive atom flag.
/:cl:

@github-actions github-actions bot added the Fix Fix one bug, make ten more label Sep 12, 2023
Copy link
Member

@morrowwolf morrowwolf left a comment

Choose a reason for hiding this comment

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

The base fluff items should have the NO_SNOW_TYPE flag on flags_atom so it doesn't get a gamemode skin rather than explicitly outlined here.

@morrowwolf morrowwolf marked this pull request as draft September 12, 2023 16:19
@Drulikar Drulikar marked this pull request as ready for review September 12, 2023 21:31
@Drulikar
Copy link
Contributor Author

Fluff armors/helmets were also incorrectly losing FPRINT and CONDUCT flags when they set NO_NAME_OVERRIDE.
image
image
image
image

Also NO_SNOW_TYPE is a poor flag name since its more than snow, but it is also not universally used either. Many item types do not test it before calling select_gamemode_skin. Only 5/26 references test it:
image

@morrowwolf
Copy link
Member

Also NO_SNOW_TYPE is a poor flag name since its more than snow

Agreed

Many item types do not test it before calling select_gamemode_skin()

Frustrating SMH

@Drulikar
Copy link
Contributor Author

Drulikar commented Sep 12, 2023

Obvious solution to that would be to test the flag within the implementations of select_gamemode_skin instead of using it to prevent calling that proc, but I don't know if there was a particular reason why it was done this way in the first place (if I were to guess, to only actually replace sprites for things that should have the sprites).

@harryob harryob added this pull request to the merge queue Sep 14, 2023
Merged via the queue into cmss13-devs:master with commit e727e22 Sep 14, 2023
26 checks passed
cm13-github added a commit that referenced this pull request Sep 14, 2023
@Drulikar Drulikar deleted the Fix_Fluff_Camo branch September 14, 2023 19:21
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Biolock-Donor Sprite
3 participants