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

feat: Added SendingCommand event to Player #163

Merged
merged 35 commits into from
Jan 16, 2025
Merged

feat: Added SendingCommand event to Player #163

merged 35 commits into from
Jan 16, 2025

Conversation

Mar1ki
Copy link

@Mar1ki Mar1ki commented Oct 31, 2024 •
edited by VladTheCow
Loading

Description

Describe the changes
Added SendingCommadnEvent, that is invoked, when player sends proper command.

What is the current behavior?
Added event, so it will be easier to contrrol commands.


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

@Mar1ki Mar1ki changed the title Sendingcommandevent Added SendingCommand event to Player Oct 31, 2024
@Mar1ki
Copy link
Author

Mar1ki commented Oct 31, 2024

Lol, i forgot to remove my test eventarg in first commit, thats why it was not working.

@VALERA771 VALERA771 added the Requires Testing Things need to be verified by an Exiled Developer/Contributor label Oct 31, 2024
@VALERA771
Copy link

I think event name should indicate that this is RA command

@Mar1ki
Copy link
Author

Mar1ki commented Oct 31, 2024

Like, change it to SendingRACommand?

@Mar1ki
Copy link
Author

Mar1ki commented Oct 31, 2024

Or SendingValidRACommand

@VALERA771
Copy link

Idk. It's up to you but it must properly indicate when this event fires

@Mar1ki
Copy link
Author

Mar1ki commented Oct 31, 2024

Oh, ok

VladTheCow
VladTheCow requested changes Oct 31, 2024
View reviewed changes
Copy link
Member

@VladTheCow VladTheCow left a comment

Choose a reason for hiding this comment

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

PRs should target dev branch, not master

@Mar1ki Mar1ki changed the base branch from master to dev October 31, 2024 22:00
@Mar1ki Mar1ki requested a review from VladTheCow November 1, 2024 06:57
@louis1706
Copy link

i am not sure about the name SendingValidRACommandEventArgs because it's not only valid command ?
it's just any RA command right ?

@Mar1ki
Copy link
Author

Mar1ki commented Nov 9, 2024

Nope, only if command exist.

@Mar1ki
Copy link
Author

Mar1ki commented Nov 9, 2024

Oh, my English sucks

VladTheCow
VladTheCow previously requested changes Nov 9, 2024
View reviewed changes
Copy link
Member

@VladTheCow VladTheCow left a comment

Choose a reason for hiding this comment

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

Can you make sure everything works as expected?

EXILED/Exiled.Events/Handlers/Player.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Handlers/Player.cs Outdated Show resolved Hide resolved
@Mar1ki
Copy link
Author

Mar1ki commented Nov 9, 2024

Yeah, i checked this on my own. Also your upper request is a little bit incorrect, the event is invoked before command execution.

@VladTheCow
Copy link
Member

Yeah, i checked this on my own. Also your upper request is a little bit incorrect, the event is invoked before command execution.

No? Everything is fine, look at other summaries

Misfiy
Misfiy requested changes Jan 7, 2025
View reviewed changes
EXILED/Exiled.Events/Exiled.Events.csproj Outdated Show resolved Hide resolved
@Mar1ki Mar1ki requested a review from Misfiy January 7, 2025 19:27
louis1706
louis1706 approved these changes Jan 13, 2025
View reviewed changes
Copy link

@louis1706 louis1706 left a comment

Choose a reason for hiding this comment

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

i did tested it and work fine

a only thing could be done more now it's doing SendedValidCommand that return the response of the plugin and success or not of it

LumiFae
LumiFae suggested changes Jan 14, 2025
View reviewed changes
Copy link

@LumiFae LumiFae left a comment

Choose a reason for hiding this comment

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

some more spelling mistakes to fix

EXILED/Exiled.Events/Handlers/Player.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Patches/Events/Player/SendingValidGameConsoleCommand.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Patches/Events/Player/SendingValidGameConsoleCommand.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Patches/Events/Player/SendingValidRACommand.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Patches/Events/Player/SendingValidRACommand.cs Outdated Show resolved Hide resolved
LumiFae
LumiFae suggested changes Jan 14, 2025
View reviewed changes
Copy link

@LumiFae LumiFae left a comment

Choose a reason for hiding this comment

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

Fix some comments

EXILED/Exiled.Events/Patches/Events/Player/SendingValidGameConsoleCommand.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Patches/Events/Player/SendingValidGameConsoleCommand.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Patches/Events/Player/SendingValidRACommand.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Patches/Events/Player/SendingValidRACommand.cs Outdated Show resolved Hide resolved
@Mar1ki
Copy link
Author

Mar1ki commented Jan 14, 2025

Editing comment from moble phone, couse i want to sleep for now.

LumiFae
LumiFae suggested changes Jan 14, 2025
View reviewed changes
Copy link

@LumiFae LumiFae left a comment

Choose a reason for hiding this comment

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

Last few issues I noticed

EXILED/Exiled.Events/Patches/Events/Player/SendingValidGameConsoleCommand.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Patches/Events/Player/SendingValidGameConsoleCommand.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Patches/Events/Player/SendingValidRACommand.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Patches/Events/Player/SendingValidRACommand.cs Outdated Show resolved Hide resolved
@Mar1ki
Copy link
Author

Mar1ki commented Jan 15, 2025

I hope thats all on my English spelling

LumiFae
LumiFae suggested changes Jan 15, 2025
View reviewed changes
Copy link

@LumiFae LumiFae left a comment

Choose a reason for hiding this comment

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

I don't see any issues other than here now

EXILED/Exiled.Events/EventArgs/Player/SendingValidCommandEventArgs.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Handlers/Player.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Handlers/Player.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Handlers/Player.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Handlers/Player.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Patches/Events/Player/SendingValidGameConsoleCommand.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Events/Patches/Events/Player/SendingValidRACommand.cs Outdated Show resolved Hide resolved
@Mar1ki
Copy link
Author

Mar1ki commented Jan 15, 2025

I think it is finally done.

LumiFae
LumiFae suggested changes Jan 15, 2025
View reviewed changes
Copy link

@LumiFae LumiFae left a comment

Choose a reason for hiding this comment

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

Only thing I can notice

EXILED/Exiled.Events/EventArgs/Player/SendingValidCommandEventArgs.cs Outdated Show resolved Hide resolved
@louis1706 louis1706 merged commit 137f9c4 into ExMod-Team:dev Jan 16, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Misaka-ZeroTwo Misaka-ZeroTwo Misaka-ZeroTwo left review comments

@Misfiy Misfiy Misfiy approved these changes

@VALERA771 VALERA771 VALERA771 approved these changes

@louis1706 louis1706 louis1706 approved these changes

@LumiFae LumiFae LumiFae approved these changes

@VladTheCow VladTheCow Awaiting requested review from VladTheCow

@Jesus-QC Jesus-QC Awaiting requested review from Jesus-QC

@IRacle1 IRacle1 Awaiting requested review from IRacle1

Assignees

@Mar1ki Mar1ki

Labels
Requires Testing Things need to be verified by an Exiled Developer/Contributor Transpiler
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Mar1ki @VALERA771 @louis1706 @VladTheCow @Misaka-ZeroTwo @LumiFae @Misfiy