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

Fixes hammer upgrade #8155

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Humonitarian
Copy link
Contributor

About The Pull Request

No idea who added the hammer upgrade, but not only did they forget to test the code, noone actually noticed this bug by what I saw so far.

Why It's Good For The Game

Fixes flat top.

Testing

Not tested. Do not merge until someone tests this.

Changelog

🆑
fix: fixed flat top not giving qualities when installed
/:cl:

@Mycah142
Copy link
Contributor

Mycah142 commented May 6, 2023

No one noticed because no one with half a braincell uses this tool mod seriously. It's functionally a waste as wrench, shovel, and crowbar all have hammering aspects already.

Attempts were made to give it another benefit but were shot down.

@DiaFRAME444
Copy link
Contributor

#7822

Actually, you all are wrong. I had this scenario before - power shovel modded to hell with 900% speed and 30 precision. I added the flat surface, didn't really impact the stats. Suddenly I had a tool that was effectively a maxed out hammer, alongside the usual shovel qualities.

Hmmm...

Comment on lines +289 to +295
if(tool_upgrades[UPGRADE_QUALITY])
if(T.toggleable)
T.switched_on_qualities |= UPGRADE_QUALITY
if(T.switched_on)
T.tool_qualities |= UPGRADE_QUALITY
else
T.tool_qualities |= UPGRADE_QUALITY
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we also need to reset switched_on_qualities and tool_qualities to their initial values in /obj/item/tool/refresh_upgrades(). Otherwise it will never be removed even if the mod goes away.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants