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

Copy Effect #6101

Merged
merged 9 commits into from
Jan 1, 2024
Merged

Copy Effect #6101

merged 9 commits into from
Jan 1, 2024

Conversation

UnderscoreTud
Copy link
Member

Description

This PR adds a new effect for copying objects over into variables. With this newly added effect, you can now copy a list into another and preserving its indices, plus copying over its sub-lists.

Example:

set {_foo::bar} to 1
set {_foo::sublist::foobar} to "hey"
copy {_foo::*} to {_copy::*}
broadcast indices of {_copy::*} # "bar", "sublist"
broadcast {_copy::bar} # 1
broadcast {_copy::sublist::foobar} # "hey!"

Target Minecraft Versions: any
Requirements: none
Related Issues: #3127, #4947

@UnderscoreTud UnderscoreTud added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Oct 5, 2023
@TheLimeGlass TheLimeGlass added the hacktoberfest-accepted Marks a PR as accepted for Hacktoberfest label Oct 5, 2023
src/main/java/ch/njol/skript/effects/EffCopy.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffCopy.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffCopy.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffCopy.java Outdated Show resolved Hide resolved
@moraedu
Copy link

moraedu commented Oct 6, 2023

If it isn't too much to ask, maybe add the ability to copy up to certain length of a variable, like:

copy %objects% into %objects% [from %integer% to %integer%]
copy %objects% into %objects% [up to %integer%]

maybe with a better syntax but you get the gist of it

@Fusezion
Copy link
Contributor

Fusezion commented Oct 6, 2023

If it isn't too much to ask, maybe add the ability to copy up to certain length of a variable, like:

Is there any practical use?

@Moderocky
Copy link
Member

If it isn't too much to ask, maybe add the ability to copy up to certain length of a variable, like:

copy %objects% into %objects% [from %integer% to %integer%]
copy %objects% into %objects% [up to %integer%]

maybe with a better syntax but you get the gist of it

This isn't very practical because lists are actually string-keyed maps in practice, so even if the indices happen to be numerical there's no guarantee they will resemble a proper order (e.g. you could have a list of 5 items with the indices 1, 2, 14, 22, 23, having deleted the intermediate entries or having set the indices individually) which means this would probably be very unintuitive and impractical.

@sovdeeth sovdeeth added 2.8 Targeting a 2.8.X version release feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. labels Dec 30, 2023
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.

tests? :)

@sovdeeth sovdeeth removed the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Jan 1, 2024
@sovdeeth
Copy link
Member

sovdeeth commented Jan 1, 2024

Needs tests and name change (see ayham review)

@APickledWalrus APickledWalrus merged commit f14816e into dev/feature Jan 1, 2024
5 checks passed
@APickledWalrus APickledWalrus deleted the feature/eff-copy branch January 1, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.8 Targeting a 2.8.X version release enhancement Feature request, an issue about something that could be improved, or a PR improving something. hacktoberfest-accepted Marks a PR as accepted for Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants