-
-
Notifications
You must be signed in to change notification settings - Fork 6
ExprRegions #18
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
base: main
Are you sure you want to change the base?
ExprRegions #18
Conversation
src/main/java/org/skriptlang/skriptworldguard/elements/expressions/ExprRegions.java
Outdated
Show resolved
Hide resolved
…ions/ExprRegions.java Co-authored-by: Shane Bee <shanebolenback@me.com>
src/main/java/org/skriptlang/skriptworldguard/elements/expressions/ExprRegions.java
Outdated
Show resolved
Hide resolved
…ions/ExprRegions.java Co-authored-by: Shane Bee <shanebolenback@me.com>
src/main/java/org/skriptlang/skriptworldguard/worldguard/RegionUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skriptworldguard/worldguard/RegionUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skriptworldguard/elements/expressions/ExprRegions.java
Outdated
Show resolved
Hide resolved
managers.put(world1, container.get(BukkitAdapter.adapt(world1))); | ||
} else { | ||
managers.put(world, container.get(BukkitAdapter.adapt(world))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why aren't these using getRegionManager()? I'd think the loss of benefits of caching the container would be outweighed by the improvements to readability.
public class ExprRegions extends SimpleExpression<WorldGuardRegion> { | ||
|
||
static { | ||
Skript.registerExpression(ExprRegions.class, WorldGuardRegion.class, ExpressionType.SIMPLE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skript.registerExpression(ExprRegions.class, WorldGuardRegion.class, ExpressionType.SIMPLE, | |
Skript.registerExpression(ExprRegions.class, WorldGuardRegion.class, ExpressionType.COMBINED, |
due to presence of expression (even if optional)
WorldGuardRegion[] worldRegions = RegionUtils.getRegions(world); | ||
if (worldRegions == null || worldRegions.length == 0) | ||
continue; | ||
regions.addAll(Arrays.stream(worldRegions).collect(Collectors.toList())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
regions.addAll(Arrays.stream(worldRegions).collect(Collectors.toList())); | |
regions.addAll(Arrays.asList(worldRegions)); |
Any reason for using a stream?
|
||
@Override | ||
public String toString(@Nullable Event event, boolean debug) { | ||
return "all of the regions" + (worlds == null ? "" : " " + worlds.toString(event, debug)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return "all of the regions" + (worlds == null ? "" : " " + worlds.toString(event, debug)); | |
return "all of the regions" + (worlds == null ? "" : " in " + worlds.toString(event, debug)); |
Might be worth specifying this
* @param world The {@link World} to get regions from, or {@code null} to get regions from all worlds | ||
* @return All regions from the world, or all worlds if {@code world} is null. | ||
*/ | ||
public static WorldGuardRegion @Nullable [] getRegions(@Nullable World world) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these methods nullable? They don't appear to return null (nor should they - just return an empty array)
This PR aims to allow users to get all regions from all worlds or specific worlds.
javaw_QIuU5pB6VJ.mp4