8000 Fixed function argument failing tests in 1.18 due to NMS nonsense · CommandAPI/CommandAPI@21365c7 · GitHub
[go: up one dir, main page]
Skip to content

Commit 21365c7

Browse files
JorelAliJorelAli
committed
Fixed function argument failing tests in 1.18 due to NMS nonsense
1 parent 41cda5b commit 21365c7

File tree

8 files changed

+184
-29
lines changed

8 files changed

+184
-29
lines changed

‎README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ This is the current roadmap for the CommandAPI (as of 2nd November 2022):
404404
<li>Fixed <code>AdventureChatArgument</code> not working on Minecraft 1.17</li>
405405
<li>Fixed commands with no executors not being caught by the CommandAPI</li>
406406
<li>Fixed <code>ParticleArgument</code> producing "Invalid particle data type" warnings on Minecraft 1.16.5 and below</li>
407+
<li>Fixed <code>FunctionArgument</code> not working on Minecraft 1.17.x and 1.18.x</li>
407408
</ul>
408409
<li>Integrated the CommandAPI repository with SonarCloud to identify bugs and improve the internal code</li>
409410
<b>Bugs found (and fixed) as a result of using SonarCloud:</b>

‎commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.17-common/src/main/java/dev/jorel/commandapi/nms/NMS_1_17_Common.java

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,28 @@ public FunctionWrapper[] getFunction(CommandContext<CommandSourceStack> cmdCtx,
395395
return result.toArray(new FunctionWrapper[0]);
396396
}
397397

398+
@Override
399+
public final SimpleFunctionWrapper getFunction(NamespacedKey key) {
400+
final ResourceLocation resourceLocation = new ResourceLocation(key.getNamespace(), key.getKey());
401+
Optional<CommandFunction> commandFunctionOptional = this.<MinecraftServer>getMinecraftServer().getFunctions().get(resourceLocation);
402+
if(commandFunctionOptional.isPresent()) {
403+
return convertFunction(commandFunctionOptional.get());
404+
} else {
405+
throw new IllegalStateException("Failed to get defined function " + key
406+
+ "! This should never happen - please report this to the CommandAPI"
407+
+ "developers, we'd love to know how you got this error message!");
408+
}
409+
}
410+
411+
@Override
412+
public final Set<NamespacedKey> getFunctions() {
413+
Set<NamespacedKey> result = new HashSet<>();
414+
for (ResourceLocation resourceLocation : this.<MinecraftServer>getMinecraftServer().getFunctions().getFunctionNames()) {
415+
result.add(fromResourceLocation(resourceLocation));
416+
}
417+
return result;
418+
}
419+
398420
@Override
399421
public org.bukkit.inventory.ItemStack getItemStack(CommandContext<CommandSourceStack> cmdCtx, String key) throws CommandSyntaxException {
400422
ItemInput input = ItemArgument.getItem(cmdCtx, key);
@@ -640,26 +662,4 @@ public <T> T getMinecraftServer() {
640662
}
641663
}
642664

643-
@Override
644-
public final SimpleFunctionWrapper getFunction(NamespacedKey key) {
645-
final ResourceLocation resourceLocation = new ResourceLocation(key.getNamespace(), key.getKey());
646-
Optional<CommandFunction> commandFunctionOptional = this.<MinecraftServer>getMinecraftServer().getFunctions().get(resourceLocation);
647-
if(commandFunctionOptional.isPresent()) {
648-
return convertFunction(commandFunctionOptional.get());
649-
} else {
650-
throw new IllegalStateException("Failed to get defined function " + key
651-
+ "! This should never happen - please report this to the CommandAPI"
652-
+ "developers, we'd love to know how you got this error message!");
653-
}
654-
}
655-
656-
@Override
657-
public final Set<NamespacedKey> getFunctions() {
658-
Set<NamespacedKey> result = new HashSet<>();
659-
for (ResourceLocation resourceLocation : this.<MinecraftServer>getMinecraftServer().getFunctions().getFunctionNames()) {
660-
result.add(fromResourceLocation(resourceLocation));
661-
}
662-
return result;
663-
}
664-
665665
}

‎commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.18.2/src/main/java/dev/jorel/commandapi/nms/NMS_1_18_R2.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,12 @@
2727
import java.nio.charset.StandardCharsets;
2828
import java.util.ArrayList;
2929
import java.util.Collection;
30+
import java.util.HashSet;
3031
import java.util.Iterator;
3132
import java.util.List;
3233
import java.util.Map;
3334
import java.util.Optional;
35+
import java.util.Set;
3436
import java.util.concurrent.CompletableFuture;
3537
import java.util.function.Predicate;
3638
import java.util.function.ToIntFunction;
@@ -456,6 +458,28 @@ public FunctionWrapper[] getFunction(CommandContext<CommandSourceStack> cmdCtx,
456458
return result.toArray(new FunctionWrapper[0]);
457459
}
458460

461+
@Override
462+
public SimpleFunctionWrapper getFunction(NamespacedKey key) {
463+
final ResourceLocation resourceLocation = new ResourceLocation(key.getNamespace(), key.getKey());
464+
Optional<CommandFunction> commandFunctionOptional = this.<MinecraftServer>getMinecraftServer().getFunctions().get(resourceLocation);
465+
if(commandFunctionOptional.isPresent()) {
466+
return convertFunction(commandFunctionOptional.get());
467+
} else {
468+
throw new IllegalStateException("Failed to get defined function " + key
469+
+ "! This should never happen - please report this to the CommandAPI"
470+
+ "developers, we'd love to know how you got this error message!");
471+
}
472+
}
473+
474+
@Override
475+
public Set<NamespacedKey> getFunctions() {
476+
Set<NamespacedKey> result = new HashSet<>();
477+
for (ResourceLocation resourceLocation : this.<MinecraftServer>getMinecraftServer().getFunctions().getFunctionNames()) {
478+
result.add(fromResourceLocation(resourceLocation));
479+
}
480+
return result;
481+
}
482+
459483
@Override
460484
public org.bukkit.inventory.ItemStack getItemStack(CommandContext<CommandSourceStack> cmdCtx, String key) throws CommandSyntaxException {
461485
ItemInput input = ItemArgument.getItem(cmdCtx, key);

‎commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.18/src/main/java/dev/jorel/commandapi/nms/NMS_1_18_R1.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@
2626
import java.io.StringWriter;
2727
import java.nio.charset.StandardCharsets;
2828
import java.util.ArrayList;
29+
import java.util.HashSet;
2930
import java.util.Iterator;
3031
import java.util.List;
3132
import java.util.Map;
3233
import java.util.Optional;
34+
import java.util.Set;
3335
import java.util.concurrent.CompletableFuture;
3436
import java.util.function.Predicate;
3537
import java.util.function.ToIntFunction;
@@ -399,6 +401,28 @@ public FunctionWrapper[] getFunction(CommandContext<CommandSourceStack> cmdCtx,
399401
}
400402
return result.toArray(new FunctionWrapper[0]);
401403
}
404+
405+
@Override
406+
public SimpleFunctionWrapper getFunction(NamespacedKey key) {
407+
final ResourceLocation resourceLocation = new ResourceLocation(key.getNamespace(), key.getKey());
408+
Optional<CommandFunction> commandFunctionOptional = this.<MinecraftServer>getMinecraftServer().getFunctions().get(resourceLocation);
409+
if(commandFunctionOptional.isPresent()) {
410+
return convertFunction(commandFunctionOptional.get());
411+
} else {
412+
throw new IllegalStateException("Failed to get defined function " + key
413+
+ "! This should never happen - please report this to the CommandAPI"
414+
+ "developers, we'd love to know how you got this error message!");
415+
}
416+
}
417+
418+
@Override
419+
public Set<NamespacedKey> getFunctions() {
420+
Set<NamespacedKey> result = new HashSet<>();
421+
for (ResourceLocation resourceLocation : this.<MinecraftServer>getMinecraftServer().getFunctions().getFunctionNames()) {
422+
result.add(fromResourceLocation(resourceLocation));
423+
}
424+
return result;
425+
}
402426

403427
@Override
404428
public org.bukkit.inventory.ItemStack getItemStack(CommandContext<CommandSourceStack> cmdCtx, String key) throws CommandSyntaxException {

‎commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-nms-common/src/main/java/dev/jorel/commandapi/nms/NMS_Common.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ public final FloatRange getFloatRange(CommandContext<CommandSourceStack> cmdCtx,
464464
public abstract FunctionWrapper[] getFunction(CommandContext<CommandSourceStack> cmdCtx, String key) throws CommandSyntaxException;
465465

466466
@Override
467-
// TODO: This has its own implementation for 1.17
467+
// TODO: This has its own implementation for 1.17, 1.18 and 1.18.2
468468
public SimpleFunctionWrapper getFunction(NamespacedKey key) {
469469
final ResourceLocation resourceLocation = new ResourceLocation(key.getNamespace(), key.getKey());
470470
Optional<CommandFunction> commandFunctionOptional = this.<MinecraftServer>getMinecraftServer().getFunctions().get(resourceLocation);
@@ -478,7 +478,7 @@ public SimpleFunctionWrapper getFunction(NamespacedKey key) {
478478
}
479479

480480
@Override
481-
// TODO: This has its own implementation for 1.17
481+
// TODO: This has its own implementation for 1.17, 1.18 and 1.18.2
482482
public Set<NamespacedKey> getFunctions() {
483483
Set<NamespacedKey> result = new HashSet<>();
484484
for (ResourceLocation resourceLocation : this.<MinecraftServer>getMinecraftServer().getFunctions().getFunctionNames()) {

‎commandapi-platforms/commandapi-bukkit/commandapi-bukkit-test/commandapi-bukkit-test-impl-1.18/src/main/java/dev/jorel/commandapi/test/MockNMS.java

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import java.io.File;
88
import java.io.IOException;
9+
import java.security.CodeSource;
910
import java.util.ArrayList;
1011
import java.util.Arrays;
1112
import java.util.Collection;
@@ -51,13 +52,15 @@
5152
import be.seeseemelk.mockbukkit.WorldMock;
5253
import be.seeseemelk.mockbukkit.enchantments.EnchantmentMock;
5354
import be.seeseemelk.mockbukkit.potion.MockPotionEffectType;
55+
import dev.jorel.commandapi.Brigadier;
5456
import dev.jorel.commandapi.CommandAPIBukkit;
5557
import dev.jorel.commandapi.commandsenders.AbstractCommandSender;
5658
import dev.jorel.commandapi.commandsenders.BukkitCommandSender;
59+
import dev.jorel.commandapi.commandsenders.BukkitPlayer;
5760
import io.papermc.paper.advancement.AdvancementDisplay;
58-
import net.kyori.adventure.text.Component;
5961
import net.minecraft.SharedConstants;
6062
import net.minecraft.advancements.Advancement;
63+
import net.minecraft.commands.CommandFunction;
6164
import net.minecraft.commands.CommandSourceStack;
6265
import net.minecraft.commands.arguments.EntityAnchorArgument.Anchor;
6366
import net.minecraft.core.BlockPos;
@@ -68,13 +71,19 @@
6871
import net.minecraft.server.Bootstrap;
6972
import net.minecraft.server.MinecraftServer;
7073
import net.minecraft.server.ServerAdvancementManager;
74+
import net.minecraft.server.ServerFunctionLibrary;
75+
import net.minecraft.server.ServerFunctionManager;
7176
import net.minecraft.server.ServerScoreboard;
7277
import net.minecraft.server.level.ServerLevel;
7378
import net.minecraft.server.level.ServerPlayer;
7479
import net.minecraft.server.players.GameProfileCache;
7580
import net.minecraft.server.players.PlayerList;
81+
import net.minecraft.tags.Tag;
82+
import net.minecraft.tags.TagCollection;
83+
import net.minecraft.util.profiling.metrics.profiling.InactiveMetricsRecorder;
7684
import net.minecraft.world.item.crafting.Recipe;
7785
import net.minecraft.world.item.crafting.RecipeManager;
86+
import net.minecraft.world.level.GameRules;
7887
import net.minecraft.world.level.Level;
7988
import net.minecraft.world.level.storage.loot.BuiltInLootTables;
8089
import net.minecraft.world.level.storage.loot.LootTables;
@@ -84,12 +93,25 @@
8493

8594
public class MockNMS extends Enums {
8695

96+
static {
97+
CodeSource src = PotionEffectType.class.getProtectionDomain().getCodeSource();
98+
if (src != null) {
99+
System.err.println("Loading PotionEffectType sources from " + src.getLocation());
100+
}
101+
src = MinecraftServer.class.getProtectionDomain().getCodeSource();
102+
if (src != null) {
103+
System.err.println("Loading MinecraftServer sources from " + src.getLocation());
104+
}
105+
}
106+
87107
static ServerAdvancementManager advancementDataWorld = new ServerAdvancementManager(null);
88108

89109
MinecraftServer minecraftServerMock = null;
90110
List<ServerPlayer> players = new ArrayList<>();
91111
PlayerList playerListMock;
92112
final RecipeManager recipeManager;
113+
Map<ResourceLocation, CommandFunction> functions = new HashMap<>();
114+
Map<ResourceLocation, Collection<CommandFunction>> tags = new HashMap<>();
93115

94116
public MockNMS(CommandAPIBukkit<?> baseNMS) {
95117
super(baseNMS);
@@ -119,6 +141,7 @@ public MockNMS(CommandAPIBukkit<?> baseNMS) {
119141
registerDefaultEnchantments();
120142

121143
this.recipeManager = new RecipeManager();
144+
this.functions = new HashMap<>();
122145
registerDefaultRecipes();
123146
}
124147

@@ -311,6 +334,11 @@ public CommandSourceStack getBrigadierSourceFromCommandSender(AbstractCommandSen
311334
// ChatArgument, AdventureChatArgument
312335
Mockito.when(css.hasPermission(anyInt())).thenAnswer(invocation -> sender.isOp());
313336
Mockito.when(css.hasPermission(anyInt(), anyString())).thenAnswer(invocation -> sender.isOp());
337+
338+
// FunctionArgument
339+
// We don't really need to do anything funky here, we'll just return the same CSS
340+
Mockito.when(css.withSuppressedOutput()).thenReturn(css);
341+
Mockito.when(css.withMaximumPermission(anyInt())).thenReturn(css);
314342
}
315343
return css;
316344
}
@@ -434,9 +462,79 @@ public <T> T getMinecraftServer() {
434462
// RecipeArgument
435463
Mockito.when(minecraftServerMock.getRecipeManager()).thenAnswer(i -> this.recipeManager);
436464

465+
// FunctionArgument
466+
// We're using 2 as the function compilation level.
467+
Mockito.when(minecraftServerMock.getFunctionCompilationLevel()).thenReturn(2);
468+
Mockito.when(minecraftServerMock.getFunctions()).thenAnswer(i -> {
469+
ServerFunctionLibrary serverFunctionLibrary = Mockito.mock(ServerFunctionLibrary.class);
470+
471+
// Functions
472+
Mockito.when(serverFunctionLibrary.getFunction(any())).thenAnswer(invocation -> Optional.ofNullable(functions.get(invocation.getArgument(0))));
473+
Mockito.when(serverFunctionLibrary.getFunctions()).thenAnswer(invocation -> functions);
474+
475+
// Tags
476+
Mockito.when(serverFunctionLibrary.getTag(any())).thenAnswer(invocation -> {
477+
Collection<CommandFunction> tagsFromResourceLocation = tags.getOrDefault(invocation.getArgument(0), List.of());
478+
return Tag.fromSet(Set.copyOf(tagsFromResourceLocation));
479+
});
480+
Mockito.when(serverFunctionLibrary.getTags()).thenAnswer(invocation -> {
481+
Map<ResourceLocation, Tag<?>> tagMap = new HashMap<>();
482+
for(Map.Entry<ResourceLocation, Collection<CommandFunction>> entry : tags.entrySet()) {
483+
tagMap.put(entry.getKey(), Tag.fromSet(Set.copyOf(entry.getValue())));
484+
}
485+
return TagCollection.of((Map) tagMap);
486+
});
487+
488+
return new ServerFunctionManager(minecraftServerMock, serverFunctionLibrary) {
489+
490+
// Make sure we don't use ServerFunctionManager#getDispatcher!
491+
// That method accesses MinecraftServer.vanillaCommandDispatcher
492+
// directly (boo) and that causes all sorts of nonsense.
493+
@Override
494+
public CommandDispatcher<CommandSourceStack> getDispatcher() {
495+
return Brigadier.getCommandDispatcher();
496+
}
497+
};
498+
});
499+
500+
Mockito.when(minecraftServerMock.getGameRules()).thenAnswer(i -> new GameRules());
501+
Mockito.when(minecraftServerMock.getProfiler()).thenAnswer(i -> InactiveMetricsRecorder.INSTANCE.getProfiler());
502+
437503
return (T) minecraftServerMock;
438504
}
439505

506+
@SuppressWarnings("unchecked")
507+
@Override
508+
public void addFunction(NamespacedKey key, List<String> commands) {
509+
if(Bukkit.getOnlinePlayers().isEmpty()) {
510+
throw new IllegalStateException("You need to have at least one player on the server to add a function");
511+
}
512+
513+
ResourceLocation resourceLocation = new ResourceLocation(key.toString());
514+
CommandSourceStack css = getBrigadierSourceFromCommandSender(new BukkitPlayer(Bukkit.getOnlinePlayers().iterator().next()));
515+
516+
// So for very interesting reasons, Brigadier.getCommandDispatcher()
517+
// gives a different result in this method than using getBrigadierDispatcher()
518+
this.functions.put(resourceLocation, CommandFunction.fromLines(resourceLocation, Brigadier.getCommandDispatcher(), css, commands));
519+
}
520+
521+
@SuppressWarnings("unchecked")
522+
@Override
523+
public void addTag(NamespacedKey key, List<List<String>> commands) {
524+
if(Bukkit.getOnlinePlayers().isEmpty()) {
525+
throw new IllegalStateException("You need to have at least one player on the server to add a function");
526+
}
527+
528+
ResourceLocation resourceLocation = new ResourceLocation(key.toString());
529+
CommandSourceStack css = getBrigadierSourceFromCommandSender(new BukkitPlayer(Bukkit.getOnlinePlayers().iterator().next()));
530+
531+
List<CommandFunction> tagFunctions = new ArrayList<>();
532+
for(List<String> functionCommands : commands) {
533+
tagFunctions.add(CommandFunction.fromLines(resourceLocation, Brigadier.getCommandDispatcher(), css, functionCommands));
534+
}
535+
this.tags.put(resourceLocation, tagFunctions);
536+
}
537+
440538
@Override
441539
public org.bukkit.advancement.Advancement addAdvancement(NamespacedKey key) {
442540
advancementDataWorld.advancements.advancements.put(new ResourceLocation(key.toString()),

‎commandapi-platforms/commandapi-bukkit/commandapi-bukkit-test/commandapi-bukkit-test-tests/src/test/java/dev/jorel/commandapi/MCVersion.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,12 @@ public boolean greaterThanOrEqualTo(MCVersion version) {
6262
public boolean lessThanOrEqualTo(MCVersion version) {
6363
return this.version.lessThanOrEqualTo(version.version);
6464
}
65+
66+
public boolean greaterThan(MCVersion version) {
67+
return this.version.greaterThan(version.version);
68+
}
69+
70+
public boolean lessThan(MCVersion version) {
71+
return this.version.lessThan(version.version);
72+
} E303
6573
}

‎commandapi-platforms/commandapi-bukkit/commandapi-bukkit-test/commandapi-bukkit-test-tests/src/test/java/dev/jorel/commandapi/test/arguments/ArgumentChatComponentTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,9 @@ void executionTestWithSpigotChatComponentArgument() {
8989

9090
// /test []
9191
// Fails due to invalid JSON for a chat component
92-
if(version.equals(MCVersion.V1_18)) {
92+
if(version.lessThan(MCVersion.V1_18)) {
9393
assertCommandFailsWith(player, "test []", "Invalid chat component: empty at position 8: test []<--[HERE]");
94-
} else if(version.greaterThanOrEqualTo(MCVersion.V1_19)) {
94+
} else {
9595
assertCommandFailsWith(player, "test []", "Invalid chat component: Invalid chat component: empty at position 8: test []<--[HERE] at position 8: test []<--[HERE]");
9696
}
9797

@@ -122,9 +122,9 @@ void executionTestWithAdventureChatComponentArgument() {
122122

123123
// /test []
124124
// Fails due to invalid JSON for a chat component
125-
if(version.equals(MCVersion.V1_18)) {
125+
if(version.lessThan(MCVersion.V1_18)) {
126126
assertCommandFailsWith(player, "test []", "Invalid chat component: empty at position 8: test []<--[HERE]");
127-
} else if(version.greaterThanOrEqualTo(MCVersion.V1_19)) {
127+
} else {
128128
assertCommandFailsWith(player, "test []", "Invalid chat component: Invalid chat component: empty at position 8: test []<--[HERE] at position 8: test []<--[HERE]");
129129
}
130130

0 commit comments

Comments
 (0)
0