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

Add spread effect #6973

Open
wants to merge 7 commits into
base: dev/feature
Choose a base branch
from
Open

Add spread effect #6973

wants to merge 7 commits into from

Conversation

Pikachu920
Copy link
Member

Description

Adds a spread effect which can be used to spread a list of objects across multiple settable expressions.

giphy-20733828


Target Minecraft Versions: any
Requirements: N/A
Related Issues: N/A

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.

Some minor feedback as I didn't have time to look through the trigger method

Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

ValueError: too many values to unpack (expected 2) moment

looks very useful, nice work pikachu!

src/main/java/ch/njol/skript/classes/Changer.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffSpread.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffSpread.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffSpread.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffSpread.java Outdated Show resolved Hide resolved
src/test/skript/tests/syntaxes/effects/EffSpread.sk Outdated Show resolved Hide resolved
@sovdeeth sovdeeth added the feature Pull request adding a new feature. label Aug 15, 2024
assert {_2} is 2 with "Spread to {_2} failed"

test "EffSpread - spreading 9":
spawn a cat
Copy link
Member Author

Choose a reason for hiding this comment

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

image

Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

as mentioned on discord, this warning needs to be more specific about the issue at hand.
image

@TheBentoBox
Copy link
Member

Just wanna call out that, as a professional TypeScript/JS dev and huge Skript user, this is one of the MRs I'm most excited for. Reminds me of the destructuring syntax in JS. Really cool!

Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

spread 1, "a", and pig across (vector length of {_a}), (name of {_b}), and (spawner type of {_c})
let me know if you don't want to support it

Copy link
Member

@Moderocky Moderocky left a comment

Choose a reason for hiding this comment

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

I am still in favour of the changes I originally suggested.

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.

5 participants