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

Helmet HUDs skip over HUDs that are already in effect on the player #5600

Merged
merged 8 commits into from
Feb 3, 2024

Conversation

Staykeu
Copy link
Contributor

@Staykeu Staykeu commented Jan 31, 2024

About the pull request

Makes it so that if you have an active HUD, a helmet visor that does the same thing will not be considered when switching through visors in your helmet. Credit to harryob for his help with the code.

Explain why it's good for the game

Visors are usually a pain to switch through when their functionality isn't needed, take for example, the squad optic, which does the same job as the headset (HUD-wise). This change makes it so you don't need to switch through unneeded HUDs.

Testing Photographs and Procedure

Screenshots & Videos

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

Changelog

🆑 Stakeyng
qol: Removes the need to cycle through helmet visors that contain HUD effects that are already active on your character.
/:cl:

@github-actions github-actions bot added the Quality of Life Make the game harder to play label Jan 31, 2024
Copy link
Member

@harryob harryob left a comment

Choose a reason for hiding this comment

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

for sanity

code/modules/clothing/head/helmet.dm Show resolved Hide resolved
@Staykeu Staykeu requested a review from harryob February 1, 2024 16:05
code/modules/clothing/head/helmet.dm Outdated Show resolved Hide resolved
Copy link
Member

@harryob harryob left a comment

Choose a reason for hiding this comment

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

im fine with this but given i my hand in creating the pr we should get someone else to look it over

Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

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

This is close, but it doesn't fully handle the situation where the next visor is not valid - it skips disabling the current visor. But this isn't your fault; this is actually fairly complex and probably needs to be refactored because its the logic in toggle_visor that's complicating everything. But I believe my suggestions below address the issue I discovered.

For example, equip a helmet w/ medical visor and squad visor. Remove your headset. Toggle visor to squad. Equip your medical glasses. Try to toggle the visor: Icon will change to no visor, but it fails to remove the squad visor and now thinks it cannot toggle to any visors.

However, this requires modification of lines outside of suggestable scope so best I can streamline this is to provide a patch:
patch.diff.txt

To apply it copy this file to the root of your fork, then open terminal in VSC and enter git apply patch.diff.txt

The changes should result in this:
image

Feel free to ask any questions why I did what I did and to test it.

@Drulikar Drulikar marked this pull request as draft February 1, 2024 20:32
@Staykeu Staykeu marked this pull request as ready for review February 2, 2024 04:28
@Drulikar Drulikar added this pull request to the merge queue Feb 3, 2024
Merged via the queue into cmss13-devs:master with commit 56c7062 Feb 3, 2024
27 checks passed
cm13-github added a commit that referenced this pull request Feb 3, 2024
Birdtalon added a commit to Birdtalon/cmss13 that referenced this pull request Feb 3, 2024
@Staykeu Staykeu deleted the helmet-optics branch September 13, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Quality of Life Make the game harder to play
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants