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

Mines Boom: The PR #3615

Closed
wants to merge 34 commits into from
Closed

Conversation

LynxSolstice
Copy link
Contributor

@LynxSolstice LynxSolstice commented Jun 13, 2023

About the pull request

This PR is a finalization of initial code from Totalepicness5. This PR adds antitank mines as an alternative/additive to claymores that target larger xenos.

Explain why it's good for the game

It offers a defensive/ambush utility to engineers that can be countered by xenos (dug up and acid'd) but that deters T3s.
(gonna write some more later I think, but it's sat in my tabs for like 8 hours already)

Testing Photographs and Procedure

The code hasn't changed but the placeholder sprite is kil

video

Changelog

🆑
add: Adds anti tank mines and the framework for creating more bury-able mine types.
balance: Adds M19 Antitank mines to the engineer vendor, the req vendor with a scale of (round * 2), adds the M19 antitank mine.
spellcheck: ((title suggested by Silencer_pl)) Added descriptions for the Antitank Mine.
code: Original code by Totalepicness5, maintenance of the code by LynxSolstice.
refactor: Mine code flesh out more. (Refactor by Totalepicness5)
imageadd: Adds a sprite for an unburied and a buried Antitank Mine (Sprites by Thwomper)
/:cl:

@github-actions
Copy link
Contributor

You currently have a negative Fix/Feature pull request delta of -11. Maintainers may close this PR at will. Fixing issues or improving the codebase will improve this score.

@github-actions github-actions bot added Sprites Remove the soul from the game. Feature Feature coder badge Balance You need to be a professional veteran game maintainer to comprehend what is being done here. Grammar and Formatting Fixes the codebase's tpyos and grammatical's errors Code Improvement Make the code longer Refactor Make the code harder to read labels Jun 13, 2023
code/game/objects/items/explosives/mine.dm Show resolved Hide resolved
code/game/objects/items/explosives/mine.dm Show resolved Hide resolved
code/game/objects/items/explosives/mine.dm Outdated Show resolved Hide resolved
if(heavy_trigger && buried)
playsound(loc, 'sound/weapons/flipblade.ogg', 35, 1)
if(isxeno(unfortunate_soul))
var/mob/living/carbon/xenomorph/xeno = unfortunate_soul
Copy link
Member

Choose a reason for hiding this comment

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

You don't need to cast to xeno if all you're doing is checking the mob_size

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But it triggers if someone is heavy enough, the check is so that T1s/Small xenos don't trigger it.

Copy link
Member

Choose a reason for hiding this comment

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

You can check mob_size of any mob, you don't have to make it a /mob/living/carbon/xenomorph variable

Comment on lines +283 to 285
unfortunate_soul.visible_message(SPAN_DANGER("[icon2html(src, viewers(src))] [name] clicks as [unfortunate_soul] moves on top of it."), \
SPAN_DANGER("[icon2html(src, unfortunate_soul)] [name] clicks as you move on top of it."), \
SPAN_DANGER("You hear a click."))
Copy link
Member

Choose a reason for hiding this comment

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

Looks like a leftover? This seems like it should already be done by heavy_trigger_notify

code/game/objects/items/explosives/mine.dm Show resolved Hide resolved
code/game/objects/items/explosives/mine.dm Show resolved Hide resolved
code/game/objects/items/explosives/mine.dm Outdated Show resolved Hide resolved
code/game/objects/items/explosives/mine.dm Outdated Show resolved Hide resolved
user.visible_message(SPAN_WARNING("[user] stops burying [src]."), \
SPAN_WARNING("You stop burying [src]."))
return
user.visible_message(SPAN_NOTICE("[user] finished burying [src]."), \
Copy link
Member

Choose a reason for hiding this comment

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

check again for anchored/active/buried/needs_digging (or whichever are appropriate) post do_after here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What?

Copy link
Member

Choose a reason for hiding this comment

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

You do checks to verify the mine is in a valid state to be burried.

But if for example several persons do it at same time, the state of the mine could be incorrect after the period waited in do_after.

So you need to re-check the mine is in a proper state after the wait to make sure the action is still valid

Copy link
Member

Choose a reason for hiding this comment

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

Similar to what's done just below for disarming with if(!active)//someone beat us to it

code/game/objects/items/explosives/mine.dm Outdated Show resolved Hide resolved
user.visible_message(SPAN_WARNING("[user] stops burying [src]."), \
SPAN_WARNING("You stop burying [src]."))
return
user.visible_message(SPAN_NOTICE("[user] finished burying [src]."), \
Copy link
Member

Choose a reason for hiding this comment

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

You do checks to verify the mine is in a valid state to be burried.

But if for example several persons do it at same time, the state of the mine could be incorrect after the period waited in do_after.

So you need to re-check the mine is in a proper state after the wait to make sure the action is still valid

user.visible_message(SPAN_WARNING("[user] stops burying [src]."), \
SPAN_WARNING("You stop burying [src]."))
return
user.visible_message(SPAN_NOTICE("[user] finished burying [src]."), \
Copy link
Member

Choose a reason for hiding this comment

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

Similar to what's done just below for disarming with if(!active)//someone beat us to it

if(heavy_trigger && buried)
playsound(loc, 'sound/weapons/flipblade.ogg', 35, 1)
if(isxeno(unfortunate_soul))
var/mob/living/carbon/xenomorph/xeno = unfortunate_soul
Copy link
Member

Choose a reason for hiding this comment

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

You can check mob_size of any mob, you don't have to make it a /mob/living/carbon/xenomorph variable

arming_time = 3 SECONDS
use_dir = FALSE
unacidable = TRUE
var/datum/effect_system/spark_spread/sparks = new
Copy link
Member

Choose a reason for hiding this comment

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

you can 't do = new here statically, new it where appropriate to use

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2023

This PR has been inactive for long enough to be automatically marked as stale. This means it is at risk of being auto closed in ~ 7 days, please address any outstanding review items and ensure your PR is finished, if these are all true and you are auto-staled anyway, you need to actively ask maintainers if your PR will be merged. Once you have done any of the previous actions then you should request a maintainer remove the stale label on your PR, to reset the stale timer. If you feel no maintainer will respond in that time, you may wish to close this PR youself, while you seek maintainer comment, as you will then be able to reopen the PR yourself

@github-actions github-actions bot added the Stale beg a maintainer to review your PR label Jul 1, 2023
@github-actions github-actions bot closed this Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Balance You need to be a professional veteran game maintainer to comprehend what is being done here. Code Improvement Make the code longer Feature Feature coder badge Grammar and Formatting Fixes the codebase's tpyos and grammatical's errors Refactor Make the code harder to read Sprites Remove the soul from the game. Stale beg a maintainer to review your PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants