Skip to content

Fix double evaluation when using VariableStrings with EffBroadcast. #7775

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

Open
wants to merge 2 commits into
base: dev/feature
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 25 additions & 18 deletions src/main/java/ch/njol/skript/effects/EffBroadcast.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
package ch.njol.skript.effects;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.regex.Pattern;

import ch.njol.skript.Skript;
import ch.njol.skript.doc.Description;
import ch.njol.skript.doc.Examples;
Expand All @@ -24,6 +17,7 @@
import ch.njol.skript.util.Utils;
import ch.njol.skript.util.chat.BungeeConverter;
import ch.njol.skript.util.chat.ChatMessages;
import ch.njol.skript.util.chat.MessageComponent;
import ch.njol.util.Kleenean;
import ch.njol.util.StringUtils;
import ch.njol.util.coll.CollectionUtils;
Expand All @@ -35,6 +29,9 @@
import org.bukkit.event.server.BroadcastMessageEvent;
import org.jetbrains.annotations.Nullable;

import java.util.*;
import java.util.regex.Pattern;

@Name("Broadcast")
@Description("Broadcasts a message to the server.")
@Examples({
Expand Down Expand Up @@ -83,12 +80,15 @@ public void execute(Event event) {
}

for (Expression<?> message : getMessages()) {
if (message instanceof VariableString) {
if (!dispatchEvent(getRawString(event, (VariableString) message), receivers))
if (message instanceof VariableString variableString) {
// get both unformatted and components with single evaluation: https://github.com/SkriptLang/Skript/issues/7718
StringBuilder unformattedString = new StringBuilder();
List<MessageComponent> messageComponents = variableString.getMessageComponents(event, unformattedString);
if (!dispatchEvent(unformattedString.toString(), receivers))
continue;
BaseComponent[] components = BungeeConverter.convert(((VariableString) message).getMessageComponents(event));
BaseComponent[] components = BungeeConverter.convert(messageComponents);
receivers.forEach(receiver -> receiver.spigot().sendMessage(components));
} else if (message instanceof ExprColoured && ((ExprColoured) message).isUnsafeFormat()) { // Manually marked as trusted
} else if (message instanceof ExprColoured coloured && coloured.isUnsafeFormat()) { // Manually marked as trusted
for (Object realMessage : message.getArray(event)) {
if (!dispatchEvent(Utils.replaceChatStyles((String) realMessage), receivers))
continue;
Expand All @@ -97,7 +97,7 @@ public void execute(Event event) {
}
} else {
for (Object messageObject : message.getArray(event)) {
String realMessage = messageObject instanceof String ? (String) messageObject : Classes.toString(messageObject);
String realMessage = messageObject instanceof String string ? string : Classes.toString(messageObject);
if (!dispatchEvent(Utils.replaceChatStyles(realMessage), receivers))
continue;
receivers.forEach(receiver -> receiver.sendMessage(realMessage));
Expand All @@ -123,9 +123,9 @@ private Expression<?>[] getMessages() {
* @param message the message
* @return true if the dispatched event does not get cancelled
*/
@SuppressWarnings("deprecation")
@SuppressWarnings("removal")
private static boolean dispatchEvent(String message, List<CommandSender> receivers) {
Set<CommandSender> recipients = Collections.unmodifiableSet(new HashSet<>(receivers));
Set<CommandSender> recipients = Set.copyOf(receivers);
BroadcastMessageEvent broadcastEvent;
if (Skript.isRunningMinecraft(1, 14)) {
broadcastEvent = new BroadcastMessageEvent(!Bukkit.isPrimaryThread(), message, recipients);
Expand All @@ -136,11 +136,18 @@ private static boolean dispatchEvent(String message, List<CommandSender> receive
return !broadcastEvent.isCancelled();
}

@Nullable
private static String getRawString(Event event, Expression<? extends String> string) {
if (string instanceof VariableString)
return ((VariableString) string).toUnformattedString(event);
/**
* Gets the raw string from the expression, replacing colour codes.
* @param event the event
* @param string the expression
* @return the raw string
*/
private static @Nullable String getRawString(Event event, Expression<? extends String> string) {
if (string instanceof VariableString variableString)
return variableString.toUnformattedString(event);
String rawString = string.getSingle(event);
if (rawString == null)
return null;
rawString = SkriptColor.replaceColorChar(rawString);
if (rawString.toLowerCase().contains("&x")) {
rawString = StringUtils.replaceAll(rawString, HEX_PATTERN, matchResult ->
Expand Down
30 changes: 22 additions & 8 deletions src/main/java/ch/njol/skript/lang/VariableString.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
import com.google.common.collect.Lists;
import org.bukkit.ChatColor;
import org.bukkit.event.Event;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.skriptlang.skript.lang.script.Script;

import java.util.ArrayList;
Expand Down Expand Up @@ -385,6 +385,19 @@ public String toUnformattedString(Event event) {
* @return Message components.
*/
public List<MessageComponent> getMessageComponents(Event event) {
return getMessageComponents(event, null);
}

/**
* Gets message components from this string. Formatting is parsed only
* in simple parts for security reasons. Providing a StringBuilder allows an unformatted output
* identical to {@link #toUnformattedString(Event)} while only evaluating any expressions once.
*
* @param event Currently running event.
* @param unformattedBuilder Unformatted string to append to.
* @return Message components.
*/
public List<MessageComponent> getMessageComponents(Event event, @Nullable StringBuilder unformattedBuilder) {
if (isSimple) { // Trusted, constant string in a script
assert simpleUnformatted != null;
return ChatMessages.parse(simpleUnformatted);
Expand All @@ -408,14 +421,15 @@ public List<MessageComponent> getMessageComponents(Event event) {

// Convert it to plain text
String text = null;
if (string instanceof ExprColoured && ((ExprColoured) string).isUnsafeFormat()) { // Special case: user wants to process formatting
String unformatted = Classes.toString(((ExprColoured) string).getArray(event), true, mode);
if (unformatted != null) {
message.addAll(ChatMessages.parse(unformatted));
if (string instanceof Expression<?> expression) {
text = Classes.toString(expression.getArray(event), true, mode);
if (unformattedBuilder != null)
unformattedBuilder.append(text);
// Special case: user wants to process formatting
if (string instanceof ExprColoured exprColoured && exprColoured.isUnsafeFormat()) {
message.addAll(ChatMessages.parse(text));
continue;
}
continue;
} else if (string instanceof Expression<?>) {
text = Classes.toString(((Expression<?>) string).getArray(event), true, mode);
}

assert text != null;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
test "broadcast evaluates vstrings twice":
broadcast "%func()%"
assert {7718::calls} is 1 with "Called func() more than once"
delete {7718::calls}

function func() returns string:
add 1 to {7718::calls}
return "7718 broadcast regression test"