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.

Sign up for GitHub

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

Jump to bottom

Firelock Doors Now Respond Correctly When Trying To Open With No Power #26472

Closed
wants to merge 6 commits into from
Closed

Firelock Doors Now Respond Correctly When Trying To Open With No Power #26472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7fca496
firelock doors are now client predictive when missing power
Huxellberger Mar 27, 2024
e9d6742
made format improvements suggested in review
Huxellberger Mar 29, 2024
b31cb75
firelock component return removed missed in first commit
Huxellberger Mar 29, 2024
87fd458
added newline to file...vs code just makes it look like you added one…
Huxellberger Mar 29, 2024
7193c5f
Merge branch 'master' into firelock-prediction
Huxellberger Mar 29, 2024
230be89
Remove unnecessary curly brackets
ShadowCommander Mar 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Content.Client/Doors/FirelockSystem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Content.Shared.Doors;
using Content.Shared.Doors.Components;
using Robust.Client.GameObjects;

Expand All @@ -10,9 +11,17 @@ public sealed class FirelockSystem : EntitySystem
public override void Initialize()
{
base.Initialize();

SubscribeLocalEvent<FirelockComponent, BeforeDoorOpenedEvent>(OnBeforeDoorOpened);
SubscribeLocalEvent<FirelockComponent, AppearanceChangeEvent>(OnAppearanceChange);
}

private void OnBeforeDoorOpened(EntityUid uid, FirelockComponent component, BeforeDoorOpenedEvent args)
{
if (!component.Powered)
args.Cancel();
}

private void OnAppearanceChange(EntityUid uid, FirelockComponent comp, ref AppearanceChangeEvent args)
{
if (args.Sprite == null)
Expand Down
21 changes: 19 additions & 2 deletions Content.Server/Doors/Systems/FirelockSystem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ public override void Initialize()

private void PowerChanged(EntityUid uid, FirelockComponent component, ref PowerChangedEvent args)
{
if (component.Powered != args.Powered)
{
component.Powered = args.Powered;
Dirty(uid, component);
}

// TODO this should REALLLLY not be door specific appearance thing.
_appearance.SetData(uid, DoorVisuals.Powered, args.Powered);
}
Expand Down Expand Up @@ -133,10 +139,21 @@ public bool EmergencyPressureStop(EntityUid uid, FirelockComponent? firelock = n

private void OnBeforeDoorOpened(EntityUid uid, FirelockComponent component, BeforeDoorOpenedEvent args)
{
if (!this.IsPowered(uid, EntityManager))
{
if (args.User != null)
{
_popupSystem.PopupEntity(Loc.GetString("firelock-component-is-unpowered-message"),
uid, args.User.Value, PopupType.Medium);
}

args.Cancel();
return;
}

// Give the Door remote the ability to force a firelock open even if it is holding back dangerous gas
var overrideAccess = (args.User != null) && _accessReaderSystem.IsAllowed(args.User.Value, uid);

if (!this.IsPowered(uid, EntityManager) || (!overrideAccess && IsHoldingPressureOrFire(uid, component)))
if (!overrideAccess && IsHoldingPressureOrFire(uid, component))
args.Cancel();
}

Expand Down
9 changes: 8 additions & 1 deletion Content.Shared/Doors/Components/FirelockComponent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Robust.Shared.GameStates;
using Content.Shared.Doors.Components;

namespace Content.Shared.Doors.Components
Expand All @@ -7,9 +8,15 @@ namespace Content.Shared.Doors.Components
/// auto-closing on depressurization, air/fire alarm interactions, and preventing normal door functions when
/// retaining pressure..
/// </summary>
[RegisterComponent]
[RegisterComponent, NetworkedComponent, AutoGenerateComponentState]
public sealed partial class FirelockComponent : Component
{
/// <summary>
/// Networked powered status for firelock to allow for proper client prediction
/// </summary>
[DataField, AutoNetworkedField]
public bool Powered;

/// <summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really like adding even more Powered fields to components as these will just need ripping out later, it should really be on some separate shared component so multiple systems can take advantage of it.

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'm not sure how I feel about this. Are we really making things simpler by doing this? We're taking a boolean flag to be replicated and changing it into a full fat component. All we're doing right now is listening for an event and setting a flag. Now we'd be listening for an event and adding a component. That seems like we're doing more work to achieve the same thing. Maybe it allows us to merge the APIs...but I'm skeptical to that. I suppose it would be some sort of powered system that listens for a Power change event and adds a powered component to an object? Possibly...I would be interested to know if that would cover all existing cases, and if it's saving that much of a headache.

Regardless, I'm still new to this codebase so if it's believed to be the best course of action I'm happy to make the change of course! Just wary of over engineering something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although, thinking about the idea for debug that powering something could be as easy as adding a component...that actually sounds pretty useful. So now I'm talking myself into it a bit more 😆

/// Pry time modifier to be used when the firelock is currently closed due to fire or pressure.
/// </summary>
Expand Down
4 changes: 3 additions & 1 deletion Resources/Locale/en-US/atmos/firelock-component.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
firelock-component-is-holding-pressure-message = A gush of air blows in your face... Maybe you should reconsider.
firelock-component-is-holding-fire-message = A gush of warm air blows in your face... Maybe you should reconsider.
firelock-component-is-holding-fire-message = A gush of warm air blows in your face... Maybe you should reconsider.
firelock-component-is-unpowered-message = The door is unpowered and can't be opened by hand!

Loading