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

Reworks Interdictor Mob Protection #14881

Merged
merged 14 commits into from Aug 5, 2023

Conversation

Wisemonster
Copy link
Contributor

@Wisemonster Wisemonster commented Jul 8, 2023

[BALANCE] [STATUS EFFECTS] [INPUT WANTED]

About the PR

This pr refactors the interdictor's mob protection effects for wormholes/radstorms/magnetic biofields to a status effect. This also makes it so the interdictor's power drain will be affected by the amount of players in it's AoE, and not be drained whenever someone within the AoE negates a portal/magnetic biofield.

Why's this needed?

Currently, not many people know what interdictors do, and not many people realize that being within an interdictor during a radstorm protects you. This pr aims to make it so that people are more aware that the interdictor protects them from wormholes/radstorms/magnetic biofields, and make it so that the amount of people protected will influence power drain to a degree.

Changelog

(u)Wisemonster
(*)Interdictors will now drain 6u of power per player in their influence
(*)Removed power drain on interdictors from negating worm holes and magnetic biofields
(+)Changed the interdictor protection to be tied to a status effect
(+)Added a particle effect for the interdictor status effect

@keywordlabeler keywordlabeler bot added A-Status-Effects Deals with status effects of any kind C-Balance Balance changes, buffs and nerfs E-Input-Wanted Input and feedback are wanted. Also posts a discussion thread on the forums. labels Jul 8, 2023
@github-actions github-actions bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 8, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2023

Created discussion thread: https://forum.ss13.co/showthread.php?tid=21501

@Wisemonster Wisemonster changed the title Converts interdictor protection to a status effect Reworks Interdictor Mob Protection Jul 8, 2023
Copy link
Contributor

@Kubius Kubius left a comment

Choose a reason for hiding this comment

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

I think this is a very solid idea overall for intuitive awareness of interdictors' protection functionality - exact power draw values might need tuning, but that's generally the case no matter what you change.

It's been mentioned that the buff could have a small visual effect, and if that's something you're inclined to do I think it might be interesting to have that as a faint golden "aura" underlaid around protected individuals, somewhat sporadic and sparkling in appearance.

interdictor_influence = 1
break
if(!interdictor_influence)
if (owner.hasStatus("spatial_protection"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing the radstorm_interdict call here means the baseline cost of interdicting a radstorm (independent of number of protected individuals) doesn't apply. My recommended change to preserve the baseline cost would probably be some functionality inside the main process() to apply that baseline cost if there's a currently active mass radiation event and the interdictor applied at least one protection buff on the last tick; to compensate for the per-user cost, it could probably be reduced slightly as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By the main_process(), do you mean for the radstorm, or interdictor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, so I simply added the radstorm_interdict proc to the end of the interdict proc, and it seems to work. It seems that something is already checking whether or not a radiation storm is going on for the proc. Although as a side effect, it will start draining the increased power when the event is announced, rather than when the radiation hits the station. If this is a problem, I'll try looking for another solution.

break
if(!interdictor_influence)
if (owner.hasStatus("spatial_protection"))
return
Copy link
Contributor

Choose a reason for hiding this comment

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

Returning here prevents the checks further below from happening. Could invert the hasStatus check with ! instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, done

src.changeStatus("spatial_protection", 3 SECONDS)
if (!iscarbon(src)) //Prevents non-carbons from getting the Zephyr stam boost, but still protects other mobs
break
if (IX.expend_interdict(4,src,TRUE,ITDR_ZEPHYR)) // Zephyr-class interdictor: carbon mobs in range gain a buff to stamina recovery, which can accumulate to linger briefly
Copy link
Contributor

Choose a reason for hiding this comment

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

As there's now an ongoing cost from having people in range at all, the Zephyr cost could probably be reduced, or skipped entirely (making it simply check for mob's carbon status and interdictor's board type inside of the main expend_interdict). Not vital, but worth considering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could do that, although I'm unsure how costly the interdictors are atm, and having one status effect from interdictors drain battery, and not the other could be a bit confusing

@Wisemonster
Copy link
Contributor Author

It's been mentioned that the buff could have a small visual effect, and if that's something you're inclined to do I think it might be interesting to have that as a faint golden "aura" underlaid around protected individuals, somewhat sporadic and sparkling in appearance.

I don't really have enough knowledge with making visual effects to make something more advanced than simply the magnetized aura, but yellow.

@Kubius
Copy link
Contributor

Kubius commented Jul 8, 2023

That'd probably be good enough, yeah.

@ThatFiveGuys
Copy link
Contributor

Good pr, in the long run the effects from the special mainframes should also be status effects but probably outside scope of this pr

Copy link
Contributor

@Kubius Kubius left a comment

Choose a reason for hiding this comment

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

LGTM. Haven't locally compiled it on my end but appears structurally correct; passive protection draw might need tuning, but that can be done easily down the line, and these values are a solid starting point.

@Wisemonster Wisemonster marked this pull request as ready for review July 10, 2023 01:37
@github-actions
Copy link
Contributor

This PR has been inactive for two weeks, and has been automatically marked as stale. This means it is at risk of being auto closed in another week. Please address any outstanding review items and ensure your PR is finished. If you are auto-staled anyway, ask developers if your PR will be merged. Once you have done any of the previous actions then you should request a developer remove the stale label on your PR, to reset the stale timer. If you feel no developer will respond in that time, you may wish to close this PR youself, while you seek developer comment, as you will then be able to reopen the PR yourself.

@github-actions github-actions bot added the S-Stale An inactive PR that has had no updates in the past two weeks label Jul 25, 2023
Copy link
Contributor

@TobleroneSwordfish TobleroneSwordfish left a comment

Choose a reason for hiding this comment

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

making interdictors less opaque is good 👍

code/mob/living/life/Life.dm Outdated Show resolved Hide resolved
code/mob/living/life/Life.dm Outdated Show resolved Hide resolved
effect_quality = STATUS_QUALITY_POSITIVE

onAdd(optional=null)
owner.add_filter("protection", 1, outline_filter(color="#e6ec21"))
Copy link
Contributor

Choose a reason for hiding this comment

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

how does this interact with the outline filter for biomagnetic fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NCS Atlas 7_28_2023 6_38_04 AM

The biofield outline appears to be closer to the mob than the interdictor outline, but both are visible

Wisemonster and others added 2 commits July 28, 2023 06:23
Co-authored-by: TobleroneSwordfish <20713227+TobleroneSwordfish@users.noreply.github.com>
Co-authored-by: TobleroneSwordfish <20713227+TobleroneSwordfish@users.noreply.github.com>
@github-actions github-actions bot removed the S-Stale An inactive PR that has had no updates in the past two weeks label Jul 29, 2023
@TobleroneSwordfish TobleroneSwordfish merged commit 1e592e8 into goonstation:master Aug 5, 2023
21 checks passed
github-actions bot pushed a commit that referenced this pull request Aug 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Status-Effects Deals with status effects of any kind C-Balance Balance changes, buffs and nerfs E-Input-Wanted Input and feedback are wanted. Also posts a discussion thread on the forums. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants