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

Temporarily make singularity a bit harder to loose as non-antag #33358

Conversation

SaphireLattice
Copy link
Contributor

@SaphireLattice SaphireLattice commented Nov 16, 2024

About the PR

The singularity/tesla generator, when hit by PA, now checks in cardinal directions for a containment field, and enters a 30 second failsafe lockout if it's not contained.

Why / Balance

Prevents instant loose by non-antag players.

Technical details

Kinda copy-pasted from ContainmentFieldGeneratorSystem.cs

Also breaks the emag feature freeze so can ditch that.

Media

image
(ignore the missing s, it's 4AM)

Requirements

Breaking changes

Changelog
🆑

  • tweak: The Singularity/Tesla generator now requires being surrounded by containment fields to activate. This can be disabled with an Emag.

@github-actions github-actions bot added S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. size/L Denotes a PR that changes 1000-4999 lines. labels Nov 16, 2024
@ScarKy0 ScarKy0 added P1: High Priority: Higher priority than other items, but isn't an emergency. D2: Medium Difficulty: A good amount of codebase knowledge required. S: Needs Review Status: Requires additional reviews before being fully accepted A: Engineering Area: Engineering department, including Atmospherics. T: Of Admin Interest Type: Affects administration work a lot, and might require admins to weigh in on and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Nov 17, 2024
@Spessmann
Copy link
Contributor

Spessmann commented Nov 18, 2024

your changelog doesn't look right
I think it needs 🆑 to work properly

maint doesn't even know how to cl smh my head /j

Copy link
Member

@SlamBamActionman SlamBamActionman left a comment

Choose a reason for hiding this comment

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

Tiny time change suggestion, otherwise I think it looks good

/// For how long the failsafe will cause the generator to stop working and not issue a failsafe warning
/// </summary>
[DataField]
public TimeSpan FailsafeCooldown = TimeSpan.FromSeconds(30);
Copy link
Member

Choose a reason for hiding this comment

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

Lower this to something like 10 seconds. It can feel kinda weird if you miss the text and then it takes half a minute to get any idea why it failed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! Set it to 10 seconds exactly

@SlamBamActionman SlamBamActionman added S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. and removed S: Needs Review Status: Requires additional reviews before being fully accepted labels Nov 18, 2024
@github-actions github-actions bot added the S: Needs Review Status: Requires additional reviews before being fully accepted label Nov 19, 2024
@SaphireLattice SaphireLattice force-pushed the feature/singularity-failsafe branch from 9ab2e28 to 9c66645 Compare November 20, 2024 01:53
@metalgearsloth metalgearsloth merged commit a818c2a into space-wizards:master Nov 20, 2024
12 checks passed
@Olexiy-Lapin

This comment was marked as off-topic.

@ArtisticRoomba
Copy link
Contributor

Brilliant idea, let's do the same with the AME (sarcasm)

Do you have anything constructive to add other than a witty remark

@Olexiy-Lapin

This comment was marked as off-topic.

@Anzarot121
Copy link
Contributor

RIP Killdozer

@SaphireLattice
Copy link
Contributor Author

RIP Killdozer

o7

Though it's still possible to make one either through managing containment field stuff just right, or having an emag handy. So not all is lost.

@ArchPigeon
Copy link
Contributor

Honestly I think that accidents should be possible, but unlikely. I dislike the idea of making it impossible to screw up the power at all and think that there should instead be more measures to abort an early singuloose/teslaloose like providing the anti singuloose gun roundstart to the CE etc to mitigate disasters like this. Maybe make the failsafe only work for 1-2 hits before failing with big red text would be a better idea generally if people REALLY think engi should get a failsafe at all for singulo/tesla

@SlamBamActionman
Copy link
Member

Honestly I think that accidents should be possible, but unlikely. I dislike the idea of making it impossible to screw up the power at all and think that there should instead be more measures to abort an early singuloose/teslaloose like providing the anti singuloose gun roundstart to the CE etc to mitigate disasters like this. Maybe make the failsafe only work for 1-2 hits before failing with big red text would be a better idea generally if people REALLY think engi should get a failsafe at all for singulo/tesla

As evident from a round I played just the other day, Engineering is more than capable of loosing a Singularity that is already contained through incompetence. It can happen.

The issue this PR primarily aims to solve is that of bad actors (non-antags) who intentionally bring the generator in front of the PA to loose as soon as they spawn in. While yes, it does also prevent singuloosing due to incompetence or miscommunication roundstart, I feel keeping the round from ending in the first 10 minutes is an alright compromise in this case.

@TurboTrackerss14
Copy link
Contributor

#8524
EMAG INTERACTIONS - we have too many and need to cut down. Needs a design doc.

I sure hope there was a merged design document for this Emag Interaction :trollface:

@SlamBamActionman
Copy link
Member

#8524 EMAG INTERACTIONS - we have too many and need to cut down. Needs a design doc.

I sure hope there was a merged design document for this Emag Interaction :trollface:

We discussed this among maintainers and weighed the other options available to us, and decided that despite the linked issue the EMAG was a necessary compromise in this instance. The choice was made with the EMAG functionality bloat considered.

There are a lot more existing EMAG interactions I would cut before this one, and I think that will be reflected in whatever PR ends up tackling that issue.

@SaphireLattice
Copy link
Contributor Author

Honestly the EMAG functionality might be removed as well. Something to consider. There was some push-back against the change as is, with a concern about antagonist freedom, so I made this interaction as a compromise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: Engineering Area: Engineering department, including Atmospherics. D2: Medium Difficulty: A good amount of codebase knowledge required. P1: High Priority: Higher priority than other items, but isn't an emergency. S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. S: Needs Review Status: Requires additional reviews before being fully accepted size/L Denotes a PR that changes 1000-4999 lines. T: Of Admin Interest Type: Affects administration work a lot, and might require admins to weigh in on
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

10 participants