-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Pipebombs now require ignition source #26400
Conversation
can u add changelog and media |
nice <3 |
Move out outside of the comment block |
also put: resolves [issue number] or fix [issue number] in front of the #26239 so it closes the issue if this PR is merged |
Alright, how are we looking now? Thanks for your help. |
|
||
namespace Content.Server.Explosion | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File-scoped namespaces please.
|
||
namespace Content.Shared.Explosion.Components | ||
{ | ||
[RegisterComponent, NetworkedComponent] | ||
public sealed partial class OnIgniteTimerTriggerComponent : Component | ||
{ | ||
[DataField] public float Delay = 1f; | ||
|
||
/// <summary> | ||
/// If not null, a user can use verbs to configure the delay to one of these options. | ||
/// </summary> | ||
[DataField] public List<float>? DelayOptions = null; | ||
|
||
/// <summary> | ||
/// If not null, this timer will periodically play this sound while active. | ||
/// </summary> | ||
[DataField] public SoundSpecifier? BeepSound; | ||
|
||
/// <summary> | ||
/// Time before beeping starts. Defaults to a single beep interval. If set to zero, will emit a beep immediately after use. | ||
/// </summary> | ||
[DataField] public float? InitialBeepDelay; | ||
|
||
[DataField] public float BeepInterval = 1; | ||
|
||
/// <summary> | ||
/// Whether the timer should instead be activated through a verb in the right-click menu | ||
/// </summary> | ||
[DataField] public bool UseVerbInstead = false; | ||
|
||
/// <summary> | ||
/// Should timer be started when it was stuck to another entity. | ||
/// Used for C4 charges and similar behaviour. | ||
/// </summary> | ||
[DataField] public bool StartOnStick; | ||
|
||
/// <summary> | ||
/// Allows changing the start-on-stick quality. | ||
/// </summary> | ||
[DataField("canToggleStartOnStick")] public bool AllowToggleStartOnStick; | ||
|
||
/// <summary> | ||
/// Whether you can examine the item to see its timer or not. | ||
/// </summary> | ||
[DataField] public bool Examinable = true; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks copy-pasted from OnuseTimerTrigger. Components should be made reuseable.
This should just be a generic "TriggerOnIgniteComponent" instead with all these copied options removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could probably remove some of these options, but why not leave some of them like beepinterval and beepsound? Wouldn't this allow for more items to use this component in the future? Same for below, IO could probably remove a bit, but why not leave some things the same if they work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@metalgearsloth Can I get your thoughts on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They should be on 1 component not spread across multiple, it just increases technical debt and makes it more of a pain to maintain the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just have AutomaticTriggerComponent or whatever i called it then call TriggerSystem's public api to start the timer once its igniter
|
||
public override void Initialize() | ||
{ | ||
SubscribeLocalEvent<OnIgniteTimerTriggerComponent, InteractUsingEvent>(OnInteracted); | ||
SubscribeLocalEvent<RandomTimerTriggerComponent, MapInitEvent>(OnRandomTimerTriggerMapInit2); | ||
//SubscribeLocalEvent<OnIgniteTimerTriggerComponent, UseInHandEvent>(OnUseInHand); | ||
} | ||
|
||
private void OnInteracted(EntityUid uid, OnIgniteTimerTriggerComponent component, InteractUsingEvent args) | ||
{ | ||
if (TryComp<IgnitionSourceComponent>(args.Used, out var comp)) { | ||
if(comp.Ignited == true) | ||
{ | ||
if (args.Handled) | ||
return; | ||
|
||
var active = AddComp<ActiveTimerTriggerComponent>(uid); | ||
active.TimeRemaining = component.Delay; | ||
active.User = args.User; | ||
active.BeepSound = component.BeepSound; | ||
active.BeepInterval = component.BeepInterval; | ||
active.TimeUntilBeep = component.InitialBeepDelay ?? 0f; | ||
active.TimeUntilBeep = component.InitialBeepDelay == null ? active.BeepInterval : component.InitialBeepDelay.Value; | ||
|
||
var ev = new ActiveTimerTriggerEvent(uid, args.User); | ||
RaiseLocalEvent(uid, ref ev); | ||
_popupSystem.PopupEntity(Loc.GetString("trigger-activated", ("device", uid)), args.User, args.User); | ||
Log.Debug("You used this with another item!"); | ||
var triggerEvent = new TriggerEvent(uid, args.User); | ||
|
||
if (TryComp<AppearanceComponent>(uid, out var appearance)) | ||
_appearance.SetData(uid, TriggerVisuals.VisualState, TriggerVisualState.Primed, appearance); | ||
|
||
} | ||
} | ||
} | ||
|
||
private void OnUseInHand(EntityUid uid, OnIgniteTimerTriggerComponent component, UseInHandEvent args) | ||
{ | ||
Log.Debug("You used this in your hand!"); | ||
} | ||
private void OnRandomTimerTriggerMapInit2(Entity<RandomTimerTriggerComponent> ent, ref MapInitEvent args) | ||
{ | ||
var (_, comp) = ent; | ||
|
||
if (!TryComp<OnIgniteTimerTriggerComponent>(ent, out var timerTriggerComp)) | ||
return; | ||
|
||
timerTriggerComp.Delay = _random.NextFloat(comp.Min, comp.Max); | ||
} | ||
|
||
|
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here on copy-paste.
Closed due to feature freeze May 10th-June 14th. Comment to have it reopen after this. |
About the PR
Pipebombs must now be ignited by things like lighters/welders.
Why / Balance
It makes more sense to have pipe bombs need ignition to match other systems in the game (like welder bombs).
Resolves #26239
#26239
Technical details
I changed the pipebomb component from an OnUse, to a new component that looks at if an item is being used on it, that item must have an IgnitionSourceComponent and be set to ignited.
Media
Breaking changes
Changelog
🆑