feat: elevator arrived event

[LumiFae]

Description

Adds an event that is triggered when an elevator has stopped and the doors are opening, aka, arrived at its destination.

Also includes a change that stopped rider from screaming at me

What is the current behavior? (You can also link to an open issue here)
No event for this behavior, manual patching

What is the new behavior? (if this is a feature change)
Adds the event, no need to manually patch

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Not really

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentations

Submission checklist

• I have checked the project can be compiled
• I have tested my changes and it worked as expected

Patches (if there are any changes related to Harmony patches)

• I have checked no IL patching errors in the console

Other

• Still requires more testing

[scp252arc]

not sure if this is really needed, can be done with delay at triggering InteractingElevator. still can me added tho

[LumiFae]

not sure if this is really needed, can be done with delay at triggering InteractingElevator. still can me added tho

delaying code is not best practice, if there is another way, it should be done, like this.

[louis1706]

wouldn't be better to make an event based on ElevatorChamber.CurSequence that trigger for all it's sequence

[LumiFae]

wouldn't be better to make an event based on ElevatorChamber.CurSequence that trigger for all it's sequence

I could do that, yeah, but CurSequence gives back "Arriving" which could be misleading with how exiled works because you can deny events like that, whereas you wont be able to here as this is an animation currently going.

[LumiFae]

Here is the current list of sequences that can be triggered

[LumiFae]

I could add ElevatorReady and ElevatorMovingAway, but I think that's really all that is needed

[louis1706]

wouldn't be better to make an event based on ElevatorChamber.CurSequence that trigger for all it's sequence

I could do that, yeah, but CurSequence gives back "Arriving" which could be misleading with how exiled works because you can deny events like that, whereas you wont be able to here as this is an animation currently going.

yeah just put like a Name ElevatorSequencesUpdated and you give CurSequence as argument

[LumiFae]

Sure, I will keep the elevator arrived thing though because I think it might be easier to find this way

[LumiFae]

@louis1706 done

[louis1706]

Sure, I will keep the elevator arrived thing though because I think it might be easier to find this way

i feel than it's look useless now ?

[LumiFae]

I don't think it is really, many people may not know arrived is under ElevatorSequencesUpdated

[VALERA771]

Better to merge 2 patches that patch the same method

[louis1706]

I don't think it is really, many people may not know arrived is under ElevatorSequencesUpdated

yes but it's does the exact same things and do not give more utility

[LumiFae]

Better to merge 2 patches that patch the same method

Ok, which patch file should I use though?

[LumiFae]

yes but it's does the exact same things and do not give more utility

Well the arrived one gives back the level that it has arrived at, and also it's better for dev experience to do this IMO

[louis1706]

yes but it's does the exact same things and do not give more utility

Well the arrived one gives back the level that it has arrived at, and also it's better for dev experience to do this IMO

Wouldn't that possible for update sequence too ?

[VALERA771]

Better to merge 2 patches that patch the same method

Ok, which patch file should I use though?

Better to create a new file that will indicate that both patches are in this file

[LumiFae]

@VALERA771 done

[LumiFae]

Wouldn't that possible for update sequence too ?

Yes, but here it just provides it to you and you don't need to find it

[VALERA771]

Lgtm

[louis1706]

I still have the same opinion that both events is not needed

[LumiFae]

I still have the same opinion that both events is not needed

I feel like this is just pointless to state, doesn't matter that much, I think it does imo because it saves me having to check the sequence and then get the level, I think EXILED doing this for the user is a much nicer developer experience, and that's what exiled is for, no?

[LumiFae]

@VALERA771 this can now be merged

[louis1706]

I still have the same opinion that both events is not needed

I feel like this is just pointless to state, doesn't matter that much, I think it does imo because it saves me having to check the sequence and then get the level, I think EXILED doing this for the user is a much nicer developer experience, and that's what exiled is for, no?

i mean if we go like that we could make an different event for every Different Role Spawn so people don't have to check than OnChangingRole it's for Scp096 for example OnChangingScp096Role

[LumiFae]

i mean if we go like that we could make an different event for every Different Role Spawn so people don't have to check than OnChangingRole it's for Scp096 for example OnChangingScp096Role

Not really because with the OnChangingRole event it is inferred because Scp096 is a role that a user can have. For the changing sequences event here, elevator arrived is not inferred from it.

[Misfiy]

i kinda agree with the french for this

[LumiFae]

i kinda agree with the french for this

Do I have to do it for this PR to be merged?

[VALERA771]

Idk really. In ElevatorArrived there is also Level property (I don't remember if lift has something similar)

[LumiFae]

Personally, I would just merge, but yeah