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

New Expression: Attached Block of Arrow #5589

Merged

Conversation

NotSoDelayed
Copy link
Contributor

Description

This PR adds a new expression which gets the attached block of arrow(s) to Skript!


Target Minecraft Versions: any
Requirements: -
Related Issues: -

@sovdeeth
Copy link
Member

sovdeeth commented Apr 8, 2023

Is there a reason this isn't a SimplePropertyExpression?
And I'd suggest adding hit block as well as attached block

@NotSoDelayed
Copy link
Contributor Author

Is there a reason this isn't a SimplePropertyExpression?

Then I wouldn't be able to allow usage of at keyword here attached block *at* last shot arrow

@sovdeeth
Copy link
Member

sovdeeth commented Apr 8, 2023

Is there a reason this isn't a SimplePropertyExpression?

Then I wouldn't be able to allow usage of at keyword here attached block *at* last shot arrow

I don't think that syntax is worth it. Personally, it doesn't even make much grammatical sense to me.
If it's the attached block at the arrow, what is it attached to? "attached" becomes a property of the block, rather than a relation between the block and the arrow.
I'd suggest just using the SimplePropertyExpression syntaxes, but I'm not sure if that's the majority opinion.

Either way, you can still use SimplePropertyExpression but with custom patterns.

@AyhamAl-Ali AyhamAl-Ali added the feature Pull request adding a new feature. label Apr 8, 2023
@NotSoDelayed NotSoDelayed changed the title New Expression: Attached Block at Arrow New Expression: Attached Block of Arrow Apr 8, 2023
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.

Reviewing from the phone isn't easy but nice PR ⚡️

@NotSoDelayed
Copy link
Contributor Author

NotSoDelayed commented Apr 8, 2023

@AyhamAl-Ali Discard your requested changes? The expression's base class has been converted to SimplePropertyExpression. Although this kills multi-arrow projectile support in one line.

@sovdeeth
Copy link
Member

sovdeeth commented Apr 8, 2023

@AyhamAl-Ali Discard your requested changes? The expression's base class has been converted to SimplePropertyExpression. Although this kills multi-arrow projectile support in one line.

Just make the type plural.

@TheLimeGlass TheLimeGlass merged commit 0c4a190 into SkriptLang:master Aug 2, 2023
4 checks passed
Moderocky pushed a commit to Moderocky/Skript that referenced this pull request Sep 16, 2023
NotSoDelayed added a commit to NotSoDelayed/Skript that referenced this pull request Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Pull request adding a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants