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

Repeat Expression #5098

Merged
merged 43 commits into from Jan 1, 2024
Merged

Repeat Expression #5098

merged 43 commits into from Jan 1, 2024

Conversation

Fusezion
Copy link
Contributor

@Fusezion Fusezion commented Sep 12, 2022

Description

This PR adds a new repeat expression into skript, pattern usage is %string% repeated %integer% time[s]


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

@AyhamAl-Ali AyhamAl-Ali added the feature Pull request adding a new feature. label Sep 13, 2022
@TheLimeGlass
Copy link
Collaborator

TheLimeGlass commented Sep 13, 2022

Skript has it's own implementation of multiply in StringUtils

public static String multiply(final @Nullable String s, final int amount) {
assert amount >= 0 : amount;
if (s == null)
return "";
if (amount <= 0)
return "";
if (amount == 1)
return s;
final char[] input = s.toCharArray();
final char[] multiplied = new char[input.length * amount];
for (int i = 0; i < amount; i++)
System.arraycopy(input, 0, multiplied, i * input.length, input.length);
return new String(multiplied);
}
public static String multiply(final char c, final int amount) {
if (amount == 0)
return "";
final char[] multiplied = new char[amount];
for (int i = 0; i < amount; i++)
multiplied[i] = c;
return new String(multiplied);
}

I haven't checked if it would be more efficient to use Skript's or not. The collection util in Java probably didn't exist back then. We need to ensure this is currently the most efficient operation count. The old method might even be better to replace with new, or vice versa.

@Fusezion
Copy link
Contributor Author

Skript has it's own implementation of multiply in StringUtils

public static String multiply(final @Nullable String s, final int amount) {
assert amount >= 0 : amount;
if (s == null)
return "";
if (amount <= 0)
return "";
if (amount == 1)
return s;
final char[] input = s.toCharArray();
final char[] multiplied = new char[input.length * amount];
for (int i = 0; i < amount; i++)
System.arraycopy(input, 0, multiplied, i * input.length, input.length);
return new String(multiplied);
}
public static String multiply(final char c, final int amount) {
if (amount == 0)
return "";
final char[] multiplied = new char[amount];
for (int i = 0; i < amount; i++)
multiplied[i] = c;
return new String(multiplied);
}

I haven't checked if it would be more efficient to use Skript's or not. The collection util in Java probably didn't exist back then. We need to ensure this is currently the most efficient operation count. The old method might even be better to replace with new, or vice versa.

It's entirely possible that skript's method would be better with the fact it's built for string while nCopy seems to support
use of non string types and if I compare multiply with Java11 repeat the only difference is some limitations and checks most of everything else is the same. If you have any clue on how I could possibly test which one is more suited then I'd love to hear.

@Fusezion
Copy link
Contributor Author

Moved it to multiply just to keep it with current standards of skript if we ever decide that multiply is outdated we can update it then

@Pikachu920
Copy link
Member

Moved it to multiply just to keep it with current standards of skript if we ever decide that multiply is outdated we can update it then

I think lime was just suggesting you use the function in your code. as a name, I think multiply is a lot less clear than repeat. Also, what do you think about making this an expression? I think "string" repeated 5 times is probably clearer than repeat("string", 5)

@Fusezion
Copy link
Contributor Author

I think lime was just suggesting you use the function in your code. as a name, I think multiply is a lot less clear than repeat. Also, what do you think about making this an expression? I think "string" repeated 5 times is probably clearer than repeat("string", 5)

I think I could do that tho would it be best to keep repeat function and add it as a side option like how round is
or just removing the function entirely and adding a %string% repeated %number% times

Removed function repeat
Added repeat expression
@Fusezion Fusezion changed the title new repeat() default function repeat expression Oct 4, 2022
@Fusezion Fusezion changed the title repeat expression Repeat Expression Oct 4, 2022
@Fusezion
Copy link
Contributor Author

Changes should of been made now, sorry for the delay on those

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.

Looks good to me! Nice work :)

Co-Authored-By: LimeGlass <16087552+TheLimeGlass@users.noreply.github.com>
@Moderocky Moderocky changed the base branch from master to dev/feature September 18, 2023 09:39
@sovdeeth sovdeeth added the 2.8 Targeting a 2.8.X version release label Dec 30, 2023
@sovdeeth sovdeeth added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Dec 31, 2023
@sovdeeth sovdeeth merged commit a9e9a78 into SkriptLang:dev/feature Jan 1, 2024
4 checks passed
@Fusezion Fusezion deleted the funcRepeat branch January 1, 2024 07:00
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 feature Pull request adding a new feature. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants