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 carlarc's OP chocolate #6003

Merged
merged 19 commits into from
Apr 2, 2024
Merged

Fixes carlarc's OP chocolate #6003

merged 19 commits into from
Apr 2, 2024

Conversation

vero5123
Copy link
Contributor

@vero5123 vero5123 commented Mar 24, 2024

About the pull request

Fixes #4695, The issue isn't inherently the attack speed, although it is quite fast, many small items also have a relatively fast attack speed. The problem is that when you combine the attack speed with a food item that somehow has a force of 35 you get a very effective weapon capable of destroying many different types of structures fairly easily, same issue with the scalpel.

My solution here was to just decrease the amount of force the items have rather than lowering the attack speed. You shouldn't be able to use a scalpel or a box of chocolate to more effectively destroy weeds rather than a machete. I doubt these are the only two items that have this problem, I guess in the future when someone comes across a similar issue just link this pr so they can easily identify the exploit.

Changelog

🆑
fix: lowers the scalpel and chunk box's ability to destroy various structures.
add: chunk box is now a melee weapon
/:cl:

@github-actions github-actions bot added the Fix Fix one bug, make ten more label Mar 24, 2024
@harryob harryob changed the title Fixes carlac's OP chocolate Fixes carlarc's OP chocolate Mar 24, 2024
@Blundir
Copy link
Contributor

Blundir commented Mar 25, 2024

The whole gimmick of chunk box is it having fun amount of damage, which is a reference to its historical real life counterpart, here you just kill the whole reference joke (without even editing the item description) despite having ability to just set low attack speed, or make it so the chunk box breaks (deletes) after being used a weapon a few times, or you just could've been more smart about it and ported this instead, which'd solve any possible similar problems in the future forever, also your pr doesn't even fix the referenced issue to begin with.

@vero5123
Copy link
Contributor Author

vero5123 commented Mar 25, 2024

The whole gimmick of chunk box is it having fun amount of damage, which is a reference to its historical real life counterpart, here you just kill the whole reference joke (without even editing the item description) despite having ability to just set low attack speed, or make it so the chunk box breaks (deletes) after being used a weapon a few times, or you just could've been more smart about it and ported this instead, which'd solve any possible similar problems in the future forever, also your pr doesn't even fix the referenced issue to begin with.

Obviously I'll wait for maintainer input on the pr, but a few things
-The person who made the issue believed that the problem was the click delay. However, I don't believe the issue was necessarily the attack speed. The real problem imo was that you can use certain items to essentially destroy structures in a way they were not intended to do. This pr fixed that for the items listed, by modifying the force and not the attack speed, which made destroying structures take a lot longer than before. Hence now you would use a machete to destroy weeds and not a scalpel. Although other items most definitely have this problem.
-This is my personal opinion but while the linked pr you provided does solve the problem, and modify the damage done to structures, I don't think adding complexity is necessarily the best approach (feel free to disagree). Currently as it stands any future similar issues that may arise would be from a wonky combination of force and attack_speed. This in my opinion can be mitigated by simply having a few defines for the type of attack speed we want, that way we can make everything consistent and remove the variability of the attack speeds from people just throwing in random numbers. People would then quickly identify the type of attack speed they need for their item based off the existing defined values. Force is a lot more variable in general so we can leave that as is. Obviously this is less robust than the damage modifier of the pr you linked but in general I think it should be ok and it's best to keep it simple, if the maintainers disagree then I'll port the tg pr. As long as the attack speed isn't too fast for a given item it shouldn't really be a big deal altogether.

@vero5123
Copy link
Contributor Author

vero5123 commented Mar 26, 2024

Ok, took a closer look into the codebase and the attack speed is too variable to make it consistent between a few defines. It would essentially be a balance change which is not ideal for a bug fix. I'll go ahead and port the tg implementation.

@vero5123
Copy link
Contributor Author

vero5123 commented Mar 26, 2024

So for the maintainers to catch up, there was a discussion on discord. Turns out the whole meme of the chunk box being a weapon is in-itself a meme because you can't even hit mobs with it; the chunk box now has a demolition modifier making it pretty much useless against structures but an effective melee weapon and the attack speed was lowered significantly.

code/game/machinery/bots/bots.dm Outdated Show resolved Hide resolved
code/game/machinery/bots/bots.dm Outdated Show resolved Hide resolved
code/game/objects/structures/lamarr_cage.dm Outdated Show resolved Hide resolved
code/game/machinery/deployable.dm Outdated Show resolved Hide resolved
code/game/machinery/deployable.dm Outdated Show resolved Hide resolved
code/game/objects/structures/lamarr_cage.dm Outdated Show resolved Hide resolved
code/game/objects/structures/mirror.dm Outdated Show resolved Hide resolved
code/modules/vehicles/vehicle.dm Outdated Show resolved Hide resolved
code/game/objects/items/reagent_containers/food/snacks.dm Outdated Show resolved Hide resolved
code/game/objects/items/reagent_containers/food/snacks.dm Outdated Show resolved Hide resolved
code/game/objects/items/reagent_containers/food/snacks.dm Outdated Show resolved Hide resolved
code/game/objects/items/tools/surgery_tools.dm Outdated Show resolved Hide resolved
code/game/objects/structures/fence.dm Outdated Show resolved Hide resolved
code/game/objects/structures/mirror.dm Outdated Show resolved Hide resolved
Copy link
Member

@SabreML SabreML left a comment

Choose a reason for hiding this comment

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

Couple more things

code/game/objects/items/reagent_containers/food/snacks.dm Outdated Show resolved Hide resolved
code/game/objects/structures/barricade/misc.dm Outdated Show resolved Hide resolved
code/game/objects/structures/barricade/misc.dm Outdated Show resolved Hide resolved
code/game/machinery/bots/bots.dm Show resolved Hide resolved
@SabreML SabreML added this pull request to the merge queue Apr 2, 2024
Merged via the queue into cmss13-devs:master with commit 35d80bb Apr 2, 2024
26 checks passed
cm13-github added a commit that referenced this pull request Apr 2, 2024
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.

CHUNK box breaks click delay
4 participants