Merge remote-tracking branch 'origin/api-12' into api-13

src/main/java/org/spongepowered/common/command/brigadier/SpongeParameterTranslator.java

@@ -35,6 +35,7 @@
 import org.spongepowered.api.command.parameter.managed.Flag;
 import org.spongepowered.common.command.brigadier.argument.ArgumentParser;
 import org.spongepowered.common.command.brigadier.argument.CustomArgumentParser;
+import org.spongepowered.common.command.brigadier.tree.SpongeArgumentCommandNode;
 import org.spongepowered.common.command.brigadier.tree.SpongeArgumentCommandNodeBuilder;
 import org.spongepowered.common.command.brigadier.tree.SpongeCommandExecutorWrapper;
 import org.spongepowered.common.command.brigadier.tree.SpongeFlagLiteralCommandNode;
@@ -189,6 +190,7 @@ private Collection<? extends CommandNode<CommandSourceStack>> createAndAttachNod
             final boolean shouldTerminate,
             final boolean allowSubcommands) {
 
+        final Set<CommandNode<CommandSourceStack>> tailNodes = new HashSet<>(parents);
         final Set<CommandNode<CommandSourceStack>> nodesToAttachTo = new HashSet<>(parents);
         final ListIterator<Parameter> parameterIterator = children.listIterator();
         while (parameterIterator.hasNext()) {
@@ -291,30 +293,43 @@ private Collection<? extends CommandNode<CommandSourceStack>> createAndAttachNod
 
                     parametersToAttachTo.add(builtNode);
 
+                    // Filter out optional nodes that have a matching type, they are nearly impossible to "merge" correctly.
+                    if (isOptional) {
+                        final Set<CommandNode<CommandSourceStack>> conflictingNodes = nodesToAttachTo.stream()
+                            .filter(n -> n instanceof final SpongeArgumentCommandNode<?> argumentNodeToAttachTo
+                                && argumentNodeToAttachTo.isOptional() && argumentNodeToAttachTo.key().type().equals(valueParameter.key().type()))
+                            .collect(Collectors.toSet());
+                        if (!conflictingNodes.isEmpty()) {
+                            nodesToAttachTo.removeIf(n -> n.getChildren().stream().anyMatch(conflictingNodes::contains));
+                        }
+                    }
+
                     // Make sure the nodes we need to attach to have the nodes we need to
                     nodesToAttachTo.forEach(x -> x.addChild(builtNode));
                 }
 
                 // If this is not optional, then we clear the "toAttachTo" list because we do not want to skip the parameter.
                 if (!isOptional) {
+                    tailNodes.clear();
                     nodesToAttachTo.clear();
                 }
 
+                tailNodes.addAll(parametersToAttachTo);
                 nodesToAttachTo.addAll(parametersToAttachTo);
             }
         }
 
         // If we should make any terminal parameters actually terminal, we do that now.
         if (shouldTerminate) {
-            for (final CommandNode<CommandSourceStack> node : nodesToAttachTo) {
+            for (final CommandNode<CommandSourceStack> node : tailNodes) {
                 // These are therefore terminal.
                 if (node instanceof SpongeNode) { // they should be, but just in case
                     ((SpongeNode) node).forceExecutor(executorWrapper);
                 }
             }
         }
 
-        return nodesToAttachTo;
+        return tailNodes;
     }
 
     @SuppressWarnings({"unchecked", "rawtypes"})
@@ -334,7 +349,8 @@ private Collection<? extends CommandNode<CommandSourceStack>> createAndAttachNod
                 parameter.completer(),
                 parameter.modifier().orElse(null),
                 parameter.valueUsage().orElse(null),
-                suffix
+                suffix,
+                parameter.isOptional()
         );
         // CommandCause is mixed into CommandSource, so this is okay.
         argumentBuilder.requires((Predicate) parameter.requirement());

src/main/java/org/spongepowered/common/command/brigadier/tree/SpongeArgumentCommandNode.java

@@ -98,6 +98,7 @@ public final class SpongeArgumentCommandNode<T> extends ArgumentCommandNode<Comm
     private final @Nullable ValueParameterModifier<T> modifier;
     private final ValueUsage usage;
     private final boolean isComplexSuggestions;
+    private final boolean isOptional;
 
     // used so we can have insertion order.
     private final UnsortedNodeHolder nodeHolder = new UnsortedNodeHolder();
@@ -117,7 +118,8 @@ public SpongeArgumentCommandNode(
             final RedirectModifier<CommandSourceStack> modifier,
             final boolean forks,
             final String keyName,
-            final @Nullable ValueParameterModifier<T> parameterModifier) {
+            final @Nullable ValueParameterModifier<T> parameterModifier,
+            final boolean isOptional) {
         super(keyName,
                 (ArgumentType<T>) Constants.Command.STANDARD_STRING_ARGUMENT_TYPE, // we can abuse generics, we're not actually going to use this.
                 command,
@@ -131,6 +133,11 @@ public SpongeArgumentCommandNode(
         this.isComplexSuggestions = this.parser instanceof ComplexSuggestionNodeProvider;
         this.key = key;
         this.usage = usage;
+        this.isOptional = isOptional;
+    }
+
+    public Parameter.Key<? super T> key() {
+        return this.key;
     }
 
     public final boolean isComplex() {
@@ -300,6 +307,10 @@ private String getUsageTextForClient() {
         return this.getName();
     }
 
+    public boolean isOptional() {
+        return this.isOptional;
+    }
+
     @Override
     public final void parse(final StringReader reader, final CommandContextBuilder<CommandSourceStack> contextBuilder) throws CommandSyntaxException {
         final int start = reader.getCursor();

src/main/java/org/spongepowered/common/command/brigadier/tree/SpongeArgumentCommandNodeBuilder.java

@@ -43,6 +43,7 @@ public final class SpongeArgumentCommandNodeBuilder<T> extends ArgumentBuilder<C
     private final @Nullable String suffix;
     private final @Nullable ValueUsage usage;
     private final @Nullable ValueParameterModifier<T> modifier;
+    private final boolean isOptional;
 
     private static @Nullable ValueCompleter filterNativeCompleters(final ArgumentParser<?> parser, final ValueCompleter completer) {
         if (parser == completer && parser.hasClientNativeCompletions()) {
@@ -57,13 +58,15 @@ public SpongeArgumentCommandNodeBuilder(
             final ValueCompleter completer,
             final @Nullable ValueParameterModifier<T> modifier,
             final @Nullable ValueUsage usage,
-            final @Nullable String suffix) {
+            final @Nullable String suffix,
+            final boolean isOptional) {
         this.key = key;
         this.type = type;
         this.completer = SpongeArgumentCommandNodeBuilder.filterNativeCompleters(type, completer);
         this.modifier = modifier;
         this.usage = usage;
         this.suffix = suffix;
+        this.isOptional = isOptional;
     }
 
     @Override
@@ -84,7 +87,8 @@ public SpongeArgumentCommandNode<T> build() {
                 this.getRedirectModifier(),
                 this.isFork(),
                 this.suffix == null ? this.key.key() : this.key.key() + "_" + this.suffix,
-                this.modifier);
+                this.modifier,
+                this.isOptional);
         for (final CommandNode<CommandSourceStack> child : this.getArguments()) {
             node.addChild(child);
         }

src/main/java/org/spongepowered/common/event/tracking/PhaseContext.java

@@ -368,6 +368,10 @@ public void printTrace(final PrettyPrinter printer) {
         return null;
     }
 
+    public boolean isClientSide() {
+        return false;
+    }
+
     protected boolean isRunaway(final PhaseContext<?> phaseContext) {
         return phaseContext.getClass() == this.getClass();
     }

src/main/java/org/spongepowered/common/event/tracking/context/transaction/TransactionSink.java

@@ -364,8 +364,8 @@ default EffectTransactor logOpenInventory(final Player player) {
         return this.pushEffect(new ResultingTransactionBySideEffect(InventoryEffect.getInstance()));
     }
 
-    default EffectTransactor logCloseInventory(final Player player, final boolean clientSource) {
-        final CloseMenuTransaction transaction = new CloseMenuTransaction(player, clientSource);
+    default EffectTransactor logCloseInventory(final PhaseContext<@NonNull ?> current, final Player player) {
+        final CloseMenuTransaction transaction = new CloseMenuTransaction(player, current.isClientSide());
         this.logTransaction(transaction);
         return this.pushEffect(new ResultingTransactionBySideEffect(InventoryEffect.getInstance()));
     }

src/main/java/org/spongepowered/common/event/tracking/phase/packet/BasicPacketContext.java

@@ -35,10 +35,6 @@ public BasicPacketContext(final PacketState<BasicPacketContext> state, final Pha
     @SuppressWarnings("unchecked")
     @Override
     public boolean hasCaptures() {
-        if (this.state == PacketPhase.General.CLOSE_WINDOW) {
-            return true;
-        }
-
         return super.hasCaptures();
     }
 }

src/main/java/org/spongepowered/common/event/tracking/phase/packet/BasicPacketState.java

@@ -24,9 +24,6 @@
  */
 package org.spongepowered.common.event.tracking.phase.packet;
 
-import net.minecraft.server.level.ServerPlayer;
-import org.checkerframework.checker.nullness.qual.Nullable;
-import org.spongepowered.common.bridge.server.TickTaskBridge;
 import org.spongepowered.common.event.tracking.PhaseTracker;
 
 public class BasicPacketState extends PacketState<BasicPacketContext> {
@@ -35,14 +32,4 @@ public class BasicPacketState extends PacketState<BasicPacketContext> {
     public BasicPacketContext createNewContext(final PhaseTracker tracker) {
         return new BasicPacketContext(this, tracker);
     }
-
-    @Override
-    public void foldContextForThread(final BasicPacketContext context, final TickTaskBridge returnValue) {
-        final @Nullable ServerPlayer source = context.getPacketPlayer();
-        returnValue.bridge$contextShift((c, f) -> {
-            if (source != null) {
-                f.pushCause(source);
-            }
-        });
-    }
 }

src/main/java/org/spongepowered/common/event/tracking/phase/packet/PacketPhase.java

@@ -62,6 +62,7 @@
 import org.spongepowered.common.event.tracking.phase.packet.drag.PrimaryDragInventoryStopState;
 import org.spongepowered.common.event.tracking.phase.packet.drag.SecondaryDragInventoryStopState;
 import org.spongepowered.common.event.tracking.phase.packet.inventory.BasicInventoryPacketState;
+import org.spongepowered.common.event.tracking.phase.packet.inventory.CloseWindowContext;
 import org.spongepowered.common.event.tracking.phase.packet.inventory.CloseWindowState;
 import org.spongepowered.common.event.tracking.phase.packet.inventory.CreativeInventoryPacketState;
 import org.spongepowered.common.event.tracking.phase.packet.inventory.DoubleClickInventoryState;
@@ -127,7 +128,7 @@ public static final class General {
         static final IPhaseState<BasicPacketContext> STOP_SPRINTING = new BasicPacketState();
         static final IPhaseState<BasicPacketContext> STOP_SLEEPING = new StopSleepingPacketState();
         static final IPhaseState<BasicPacketContext> TAB_COMPLETE = new BasicPacketState();
-        public static final IPhaseState<BasicPacketContext> CLOSE_WINDOW = new CloseWindowState();
+        public static final IPhaseState<CloseWindowContext> CLOSE_WINDOW = new CloseWindowState();
         public static final IPhaseState<BasicPacketContext> UPDATE_SIGN = new BasicPacketState();
         static final IPhaseState<BasicPacketContext> STOP_RIDING_JUMP = new BasicPacketState();
         static final IPhaseState<BasicPacketContext> HANDLED_EXTERNALLY = new UnknownPacketState();

src/main/java/org/spongepowered/common/event/tracking/phase/packet/PacketState.java

@@ -28,13 +28,16 @@
 import net.minecraft.network.protocol.Packet;
 import net.minecraft.resources.ResourceLocation;
 import net.minecraft.server.level.ServerLevel;
+import net.minecraft.server.level.ServerPlayer;
 import net.minecraft.world.level.block.Block;
 import net.minecraft.world.level.chunk.LevelChunk;
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.spongepowered.api.ResourceKey;
 import org.spongepowered.api.entity.living.player.Player;
 import org.spongepowered.api.event.CauseStackManager;
 import org.spongepowered.api.event.cause.entity.SpawnType;
 import org.spongepowered.api.event.cause.entity.SpawnTypes;
+import org.spongepowered.common.bridge.server.TickTaskBridge;
 import org.spongepowered.common.bridge.world.level.chunk.LevelChunkBridge;
 import org.spongepowered.common.entity.PlayerTracker;
 import org.spongepowered.common.event.tracking.IPhaseState;
@@ -106,6 +109,16 @@ protected boolean alwaysUnwinds() {
         return false;
     }
 
+    @Override
+    public void foldContextForThread(final P context, final TickTaskBridge returnValue) {
+        final @Nullable ServerPlayer source = context.getPacketPlayer();
+        returnValue.bridge$contextShift((c, f) -> {
+            if (source != null) {
+                f.pushCause(source);
+            }
+        });
+    }
+
     private final String desc = TrackingUtil.phaseStateToString("Packet", this);
 
     @Override

src/main/java/org/spongepowered/common/event/tracking/phase/packet/inventory/CloseWindowContext.java

@@ -0,0 +1,59 @@
+/*
+ * This file is part of Sponge, licensed under the MIT License (MIT).
+ *
+ * Copyright (c) SpongePowered <https://www.spongepowered.org>
+ * Copyright (c) contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.spongepowered.common.event.tracking.phase.packet.inventory;
+
+import org.spongepowered.common.event.tracking.PhaseTracker;
+import org.spongepowered.common.event.tracking.phase.packet.PacketContext;
+import org.spongepowered.common.event.tracking.phase.packet.PacketState;
+
+public final class CloseWindowContext extends PacketContext<CloseWindowContext> {
+
+    private boolean clientSide = true;
+
+    public CloseWindowContext(final PacketState<CloseWindowContext> state, final PhaseTracker tracker) {
+        super(state, tracker);
+    }
+
+    public CloseWindowContext isClientSide(final boolean clientSide) {
+        this.clientSide = clientSide;
+        return this;
+    }
+
+    @Override
+    public boolean hasCaptures() {
+        return true;
+    }
+
+    @Override
+    public boolean isClientSide() {
+        return this.clientSide;
+    }
+
+    @Override
+    protected void reset() {
+        super.reset();
+        this.clientSide = true;
+    }
+}

src/main/java/org/spongepowered/common/event/tracking/phase/packet/inventory/CloseWindowState.java

@@ -34,23 +34,28 @@
 import org.spongepowered.api.event.cause.entity.SpawnTypes;
 import org.spongepowered.api.event.entity.SpawnEntityEvent;
 import org.spongepowered.api.util.Tuple;
+import org.spongepowered.common.event.tracking.PhaseTracker;
 import org.spongepowered.common.event.tracking.context.transaction.GameTransaction;
 import org.spongepowered.common.event.tracking.context.transaction.world.SpawnEntityTransaction;
-import org.spongepowered.common.event.tracking.phase.packet.BasicPacketContext;
-import org.spongepowered.common.event.tracking.phase.packet.BasicPacketState;
+import org.spongepowered.common.event.tracking.phase.packet.PacketState;
 
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
-public final class CloseWindowState extends BasicPacketState {
+public final class CloseWindowState extends PacketState<CloseWindowContext> {
 
     @Override
-    public Supplier<SpawnType> getSpawnTypeForTransaction(final BasicPacketContext context, final Entity entityToSpawn) {
+    protected CloseWindowContext createNewContext(final PhaseTracker tracker) {
+        return new CloseWindowContext(this, tracker);
+    }
+
+    @Override
+    public Supplier<SpawnType> getSpawnTypeForTransaction(final CloseWindowContext context, final Entity entityToSpawn) {
         return SpawnTypes.DROPPED_ITEM;
     }
 
     @Override
-    public SpawnEntityEvent createSpawnEvent(final BasicPacketContext context, final @Nullable GameTransaction<@NonNull ?> parent,
+    public SpawnEntityEvent createSpawnEvent(final CloseWindowContext context, final @Nullable GameTransaction<@NonNull ?> parent,
             final ImmutableList<Tuple<Entity, SpawnEntityTransaction.DummySnapshot>> collect, final Cause currentCause) {
         return SpongeEventFactory.createDropItemEventClose(currentCause,
                 collect.stream()

src/mixins/java/org/spongepowered/common/mixin/inventory/api/server/level/ServerPlayerMixin_Inventory_API.java

@@ -30,12 +30,12 @@
 import org.spongepowered.api.item.inventory.Container;
 import org.spongepowered.api.item.inventory.Inventory;
 import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Shadow;
 import org.spongepowered.common.bridge.world.inventory.container.ContainerBridge;
 import org.spongepowered.common.event.inventory.InventoryEventFactory;
 import org.spongepowered.common.event.tracking.PhaseContext;
 import org.spongepowered.common.event.tracking.PhaseTracker;
 import org.spongepowered.common.event.tracking.TrackingUtil;
-import org.spongepowered.common.event.tracking.context.transaction.EffectTransactor;
 import org.spongepowered.common.event.tracking.phase.packet.PacketPhase;
 import org.spongepowered.common.mixin.inventory.api.world.entity.player.PlayerMixin_Inventory_API;
 
@@ -44,6 +44,8 @@
 @Mixin(net.minecraft.server.level.ServerPlayer.class)
 public abstract class ServerPlayerMixin_Inventory_API extends PlayerMixin_Inventory_API implements ServerPlayer {
 
+    @Shadow public abstract void shadow$doCloseContainer();
+
     @Override
     public Optional<Container> openInventory() {
         return Optional.ofNullable((Container) this.containerMenu);
@@ -75,12 +77,10 @@ public boolean closeInventory() throws IllegalArgumentException {
         try (final PhaseContext<@NonNull ?> ctx = PacketPhase.General.CLOSE_WINDOW.createPhaseContext(PhaseTracker.getWorldInstance(player.serverLevel()))
                 .source(this)
                 .packetPlayer(player)
+                .isClientSide(false)
         ) {
             ctx.buildAndSwitch();
-            try (final EffectTransactor ignored = ctx.getTransactor().logCloseInventory(player, false)) {
-                this.containerMenu.removed(player); // Drop & capture cursor item
-                this.containerMenu.broadcastChanges();
-            }
+            this.shadow$doCloseContainer();
 
             if (!TrackingUtil.processBlockCaptures(ctx)) {
                 return false;