-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Move TriggerSystem trigger methods to shared #27970
base: master
Are you sure you want to change the base?
Move TriggerSystem trigger methods to shared #27970
Conversation
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 these details and you have a personal self merge token
{ | ||
// Check if entity is bomb/mod. grenade/etc | ||
if (_container.TryGetContainer(uid, "payload", out var container) && |
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.
container id to const string
chemicalPayloadComponent.BeakerSlotB.Item is not { } beakerB || | ||
!TryComp(beakerA, out SolutionContainerManagerComponent? containerA) || | ||
!TryComp(beakerB, out SolutionContainerManagerComponent? containerB) || | ||
!TryComp(beakerA, out FitsInDispenserComponent? fitsA) || | ||
!TryComp(beakerB, out FitsInDispenserComponent? fitsB) || | ||
!_solutions.TryGetSolution((beakerA, containerA), fitsA.Solution, out _, out var solutionA) || | ||
!_solutions.TryGetSolution((beakerB, containerB), fitsB.Solution, out _, out var solutionB)) | ||
return; |
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.
wtf is this (not a review since you didnt make... this, just a visceral reaction)
User = user; | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Raised when timer trigger becomes active. | ||
/// </summary> | ||
[ByRefEvent] | ||
public readonly record struct ActiveTimerTriggerEvent(EntityUid Triggered, EntityUid? User); |
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.
shouldnt this be removed now?
[Dependency] private readonly ISharedAdminLogManager _adminLogger = default!; | ||
[Dependency] private readonly SharedAppearanceSystem _appearance = default!; | ||
[Dependency] private readonly SharedContainerSystem _container = default!; |
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.
_container can be protected
/// <summary> | ||
/// Raised when timer trigger becomes active. | ||
/// </summary> | ||
[ByRefEvent] | ||
public readonly record struct ActiveTimerTriggerEvent(EntityUid Triggered, EntityUid? User); | ||
|
||
[UsedImplicitly] | ||
public sealed partial class TriggerSystem : EntitySystem | ||
public sealed partial class TriggerSystem : SharedTriggerSystem | ||
{ | ||
[Dependency] private readonly ExplosionSystem _explosions = default!; | ||
[Dependency] private readonly FixtureSystem _fixtures = default!; |
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.
something tells me some of the deps below are unused now
About the PR
Media
Breaking changes
TriggerEvent has been moved from Content.Server.Explosion.EntitySystems to Content.Shared.Explosion