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

Overhaul how sechuds' and prodocs' icon adding/removing works. #5359

Merged

Conversation

Gerhazo
Copy link
Contributor

@Gerhazo Gerhazo commented Jul 2, 2021

About the PR

Uses client image groups added in #4789 to overhaul how assigning arrest/health icons to mobs wearing things like sechuds or prodocs works.

Currently, to deal with things like clients logging out/logging in, mobs swapping clients and whatnot processing loops are used to sanity-check constantly for whether the icons need to be added/removed. Invisibility changes are tracked in a similar fashion. This ends up with a lot of lines of repeating code for processing loops for every item that requires that functionality.

This PR instead makes use of signals to handle these events as they happen. It also moves a lot of the responsibility onto the forementioned image groups, drastically cutting down the amount of code required to make use of the icons.

There should be no noticeable difference from a player standpoint and all original functionality has been preserved.

Why's this needed?

Cleaner handling of adding arrest and health icons, with less repetitive code and no necessity of processing loops.

Changelog

(u)Gerhazo
(+)Back-end overhaul to how sechuds and prodocs work. This shouldn't be noticeable from a player standpoint.

@Gerhazo Gerhazo added the C-Code-Quality Cleans up code, refactors things to be more readable or intuitive label Jul 2, 2021
@github-actions github-actions bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 2, 2021
Copy link
Member

@ZeWaka ZeWaka left a comment

Choose a reason for hiding this comment

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

very good - don't think it needs a changelog though?

code/obj/item/organs/eye.dm Outdated Show resolved Hide resolved
code/obj/item/organs/eye.dm Outdated Show resolved Hide resolved
code/obj/item/organs/eye.dm Outdated Show resolved Hide resolved
code/obj/item/organs/eye.dm Outdated Show resolved Hide resolved
code/obj/item/clothing/helmets.dm Outdated Show resolved Hide resolved
code/obj/item/clothing/glasses.dm Outdated Show resolved Hide resolved
code/obj/item/clothing/glasses.dm Outdated Show resolved Hide resolved
code/obj/item/clothing/glasses.dm Outdated Show resolved Hide resolved
code/modules/robotics/robot/upgrade/sechud_scanner.dm Outdated Show resolved Hide resolved
code/modules/robotics/robot/upgrade/prodoc_scanner.dm Outdated Show resolved Hide resolved
Co-authored-by: ZeWaka <zewakagamer@gmail.com>
@Gerhazo
Copy link
Contributor Author

Gerhazo commented Jul 4, 2021

very good - don't think it needs a changelog though?

The idea behind the changelog entry was an immediate point of reference to a certain thing changing in case any issues happened that didn't appear in local testing as well as to bring to light that it's a thing now with the old method of adding images on a processing loop from a global list being deprecated.

That being said if it still seems unnecessary I don't mind it being removed so if that's the case feel free to (or if it's a thing I must do give me a heads up and I shall).

@Tarmunora
Copy link
Member

yolo

@Tarmunora Tarmunora merged commit 1acfd34 into goonstation:master Jul 10, 2021
github-actions bot pushed a commit that referenced this pull request Jul 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Code-Quality Cleans up code, refactors things to be more readable or intuitive 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.

Sechud icons are lost on reconnecting or leaving mob and re-entering.
3 participants