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.

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

Implementation of CommandFilters, replacing cooldowns and costs #3886

Open
wants to merge 15 commits into
base: 2.x
Choose a base branch
from

Conversation

FrankHeijden
Copy link
Contributor

Basically implements #3200, with just a small difference in yaml format: made the name required, as a key, just like kits.
Automatic converter is included, and will create the new yaml format from existing nodes.
Formats for command will be the same as with the command-costs, formats for pattern will be the same as in command-cooldowns.

Closes #3200, #3847

@Bobcat00
Copy link
Contributor

How does this affect PR #3744 ?

@FrankHeijden
Copy link
Contributor Author

FrankHeijden commented Dec 30, 2020

How does this affect PR #3744 ?

They should go hand in hand with each other, defining a pattern filter in this version will behave the same as a regular command-cooldown in that PR

@Bobcat00
Copy link
Contributor

Bobcat00 commented Dec 30, 2020

They should go hand in hand with each other, defining a pattern filter in this version will behave the same as a regular command-cooldown in that PR

So this PR needs #3744 ? Just asking because 3744 hasn't been merged yet.

@FrankHeijden
Copy link
Contributor Author

They should go hand in hand with each other, defining a pattern filter in this version will behave the same as a regular command-cooldown in that PR

So this PR needs #3744 ? Just asking because 3744 hasn't been merged yet.

I mean they are independent features -- #3744 fixes a bug where commands.yml defined commands aren't recognised by EssX, this one refactors the cooldowns/costs into a separate yml file as described per #3200 (and by doing that fixing a bug where periods couldn't be used in the regexes, because yml prohibits periods in keys).

Make CommandFilter more extendable + fix AsyncTeleport not applying command cooldown
@FrankHeijden FrankHeijden force-pushed the extract-command-filters branch from eefb99e to dfda202 Compare December 31, 2020 18:57
@FrankHeijden
Copy link
Contributor Author

So I just came across a thing which prevented AsyncTeleport's from applying command cooldowns - this is due to the fact AsyncTeleport creates a new Trade without the command defined in it. I added a new cooldownCommand parameter for the Trade#charge() command, so this method can know what command we should apply cooldowns on.

@JRoy JRoy requested a review from mdcfe January 3, 2021 23:34
@triagonal triagonal added type: enhancement Features and feature requests. bug: confirmed Confirmed bugs in EssentialsX. type: bugfix PRs that fix bugs in EssentialsX. and removed bug: confirmed Confirmed bugs in EssentialsX. labels Jan 9, 2021
# Conflicts:
#	Essentials/src/main/java/com/earth2me/essentials/EssentialsPlayerListener.java
@JRoy
Copy link
Member

JRoy commented Jun 11, 2021

Conflicts need to be addressed, please reopen when done so.

@JRoy JRoy closed this Jun 11, 2021
@FrankHeijden
Copy link
Contributor Author

Cant reopen this one, opening #4224 instead

@triagonal triagonal reopened this Jun 15, 2021
# Conflicts:
#	Essentials/src/main/java/com/earth2me/essentials/Essentials.java
#	Essentials/src/main/java/com/earth2me/essentials/Settings.java
@JRoy JRoy added the module: main Issues or PRs for the main Essentials module label Jun 17, 2021
@FrankHeijden
Copy link
Contributor Author

What’s the status on this PR? 😅

@JRoy
Copy link
Member

JRoy commented Jul 27, 2021

What’s the status on this PR? 😅

This likely won't make it in time for 2.19 but 100% 2.20/2.19.x

@JRoy JRoy added this to the 2.19.1 milestone Aug 2, 2021
@JRoy JRoy modified the milestones: 2.19.1, 2.20.0 Oct 24, 2021
Copy link
Member

@JRoy JRoy left a comment

Choose a reason for hiding this comment

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

Needs to be updated to configurate (use EssentialsConfiguration over EssentialsConf)

@FrankHeijden
Copy link
Contributor Author

Needs to be updated to configurate (use EssentialsConfiguration over EssentialsConf)

Done

@mdcfe
Copy link
Member

mdcfe commented Dec 29, 2021

Just a quick heads-up: I have a pending review for this PR, but I likely won't have time to properly review this for a few weeks.

@JRoy
Copy link
Member

JRoy commented Oct 14, 2022

Remaining errors (EssentialsConf and logger stuff) need to be fixed. It's almost been 2 years since this PR was made, and I appreciate your persistence 😅. This pull request will be merged for 2.20 which is HOPEFULLY before the new year.

@FrankHeijden
Copy link
Contributor Author

@JRoy Some build dependencies are timing out because they redirect to jcenter, which closed down. Do they need to be updated or is there a repo missing for these dependencies?

WkyRXerS

@JRoy JRoy modified the milestones: 2.20.0, 2.21.0 Apr 28, 2023
JRoy added 2 commits November 11, 2023 17:35
# Conflicts:
#	Essentials/src/main/java/com/earth2me/essentials/EssentialsPlayerListener.java
@mdcfe mdcfe modified the milestones: 2.21.0, 2.22.0 Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: main Issues or PRs for the main Essentials module type: bugfix PRs that fix bugs in EssentialsX. type: enhancement Features and feature requests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace command-costs and command-cooldowns with command filters system
5 participants