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

Feature/target block update #6422

Merged
merged 21 commits into from
May 8, 2024
Merged

Feature/target block update #6422

merged 21 commits into from
May 8, 2024

Conversation

ShaneBeee
Copy link
Contributor

@ShaneBeee ShaneBeee commented Feb 12, 2024 •
edited by sovdeeth

Description

This PR aims to add a use for the "actual" part of the target block expression

Whilst in there, I did a little cleanup.

NOTE: I didn't know which branch to target, so I left it as master

Target Minecraft Versions: any
Requirements: none
Related Issues: #5612

@TheLimeGlass
Copy link
Collaborator

You target dev/feature for enhancements

@ShaneBeee @TheLimeGlass
Update src/main/java/ch/njol/skript/expressions/ExprTargetedBlock.java
1915a2d 
Co-authored-by: LimeGlass <16087552+TheLimeGlass@users.noreply.github.com>
@ShaneBeee
Copy link
Contributor Author

You target dev/feature for enhancements

I'll wait for one of the SkriptLang team members to decide which one for this.
While it may come off as an enchantment, its more meant as a "fix" for the pattern that does nothing.

@sovdeeth sovdeeth changed the base branch from master to dev/feature February 13, 2024 07:32
@sovdeeth sovdeeth added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Feb 14, 2024
@APickledWalrus APickledWalrus added the breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) label Mar 9, 2024
ShaneBeee and others added 3 commits March 9, 2024 13:58
@ShaneBeee @APickledWalrus
Update src/main/java/ch/njol/skript/expressions/ExprTargetedBlock.java
e1d7470 
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
@ShaneBeee @APickledWalrus
Update src/main/java/ch/njol/skript/expressions/ExprTargetedBlock.java
d4428c8 
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
@ShaneBeee
ExprTargetedBlock - use internal isAir check
8cfc5e5 
- Suggested by Pickle
AyhamAl-Ali
AyhamAl-Ali approved these changes Mar 29, 2024
View reviewed changes
Copy link
Member

@AyhamAl-Ali AyhamAl-Ali left a comment

Choose a reason for hiding this comment

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

⚡

src/main/java/ch/njol/skript/expressions/ExprTargetedBlock.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/expressions/ExprTargetedBlock.java Outdated Show resolved Hide resolved
@AyhamAl-Ali AyhamAl-Ali added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Mar 29, 2024
ShaneBeee and others added 2 commits March 29, 2024 10:18
@ShaneBeee @AyhamAl-Ali
Update src/main/java/ch/njol/skript/expressions/ExprTargetedBlock.java
70cba37 
Co-authored-by: Ayham Al Ali <20037329+AyhamAl-Ali@users.noreply.github.com>
@ShaneBeee
ExprTargetedBlock - cut down patterns
f62f58a
Fusezion
Fusezion suggested changes Mar 29, 2024
View reviewed changes
Copy link
Contributor

@Fusezion Fusezion left a comment

Choose a reason for hiding this comment

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

Just some concerned

src/main/java/ch/njol/skript/expressions/ExprTargetedBlock.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/expressions/ExprTargetedBlock.java Outdated Show resolved Hide resolved
@ShaneBeee @sovdeeth
Update src/main/java/ch/njol/skript/expressions/ExprTargetedBlock.java
bf748f2 
Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>
APickledWalrus
APickledWalrus approved these changes May 8, 2024
View reviewed changes
Copy link
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

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

just some formatting recommendations

src/main/java/ch/njol/skript/expressions/ExprTargetedBlock.java

static {
Skript.registerExpression(ExprTargetedBlock.class, Block.class, ExpressionType.COMBINED,
"[the] target[ed] block[s] [of %players%]", "%players%'[s] target[ed] block[s]",
"[the] actual[ly] target[ed] block[s] [of %players%]", "%players%'[s] actual[ly] target[ed] block[s]");
"[the] [actual:(actual[ly]|exact)] target[ed] block[s] [of %livingentities%]", "%livingentities%'[s] [actual:(actual[ly]|exact)] target[ed] block[s]");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"[the] [actual:(actual[ly]|exact)] target[ed] block[s] [of %livingentities%]", "%livingentities%'[s] [actual:(actual[ly]|exact)] target[ed] block[s]");
"[the] [actual:(actual[ly]|exact)] target[ed] block[s] [of %livingentities%]",
"%livingentities%'[s] [actual:(actual[ly]|exact)] target[ed] block[s]"
);

formatting

src/main/java/ch/njol/skript/expressions/ExprTargetedBlock.java
"set {_block} to actual target block of player",
"break actual target block of player"
})
@Since("1.0, INSERT VERSION (actual/exact)")
Copy link
Member

Choose a reason for hiding this comment

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

might be worth mentioning support for living entities rather than just players?

Suggested change
@Since("1.0, INSERT VERSION (actual/exact)")
@Since("1.0, INSERT VERSION (support for more entities, actual/exact target block)")

@Moderocky Moderocky merged commit 380d044 into SkriptLang:dev/feature May 8, 2024
5 checks passed
@ShaneBeee ShaneBeee deleted the feature/target-block-update branch May 8, 2024 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AyhamAl-Ali AyhamAl-Ali AyhamAl-Ali approved these changes

@sovdeeth sovdeeth sovdeeth approved these changes

@TheLimeGlass TheLimeGlass TheLimeGlass approved these changes

@APickledWalrus APickledWalrus APickledWalrus approved these changes

@Fusezion Fusezion Fusezion approved these changes

Assignees
No one assigned
Labels
breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@ShaneBeee @TheLimeGlass @sovdeeth @AyhamAl-Ali @APickledWalrus @Fusezion @Moderocky