Fix KeyModifiers not being properly taken into account. Closes #9806

src/main/java/net/minecraftforge/client/extensions/IForgeKeyMapping.java

@@ -14,11 +14,9 @@ import org.jetbrains.annotations.NotNull;
 /**
  * Extension interface for {@link KeyMapping}.
  */
-public interface IForgeKeyMapping
-{
-    private KeyMapping self()
-    {
-        return (KeyMapping) this;
+public interface IForgeKeyMapping {
+    private KeyMapping self() {
+        return (KeyMapping)this;
     }
 
     @NotNull InputConstants.Key getKey();
@@ -26,13 +24,11 @@ public interface IForgeKeyMapping
     /**
      * {@return true if the key conflict context and modifier are active and the keyCode matches this binding, false otherwise}
      */
-    default boolean isActiveAndMatches(InputConstants.Key keyCode)
-    {
+    default boolean isActiveAndMatches(InputConstants.Key keyCode) {
         return keyCode != InputConstants.UNKNOWN && keyCode.equals(getKey()) && getKeyConflictContext().isActive() && getKeyModifier().isActive(getKeyConflictContext());
     }
 
-    default void setToDefault()
-    {
+    default void setToDefault() {
         setKeyModifierAndCode(getDefaultKeyModifier(), self().getDefaultKey());
     }
 
@@ -46,22 +42,17 @@ public interface IForgeKeyMapping
 
     void setKeyModifierAndCode(KeyModifier keyModifier, InputConstants.Key keyCode);
 
-    default boolean isConflictContextAndModifierActive()
-    {
+    default boolean isConflictContextAndModifierActive() {
         return getKeyConflictContext().isActive() && getKeyModifier().isActive(getKeyConflictContext());
     }
 
     /**
      * Returns true when one of the bindings' key codes conflicts with the other's modifier.
      */
-    default boolean hasKeyModifierConflict(KeyMapping other)
-    {
-        if (getKeyConflictContext().conflicts(other.getKeyConflictContext()) || other.getKeyConflictContext().conflicts(getKeyConflictContext()))
-        {
+    default boolean hasKeyModifierConflict(KeyMapping other) {
+        if (getKeyConflictContext().conflicts(other.getKeyConflictContext()) || other.getKeyConflictContext().conflicts(getKeyConflictContext())) {
             if (getKeyModifier().matches(other.getKey()) || other.getKeyModifier().matches(getKey()))
-            {
                 return true;
-            }
         }
         return false;
     }

src/main/java/net/minecraftforge/client/settings/KeyMappingLookup.java

@@ -10,53 +10,40 @@ import net.minecraft.client.KeyMapping;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.ArrayList;
-import java.util.Collection;
+import java.util.Collections;
 import java.util.EnumMap;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-public class KeyMappingLookup
-{
-    private static final EnumMap<KeyModifier, Map<InputConstants.Key, Collection<KeyMapping>>> map = new EnumMap<>(KeyModifier.class);
-    static
-    {
+public class KeyMappingLookup {
+    private static final EnumMap<KeyModifier, Map<InputConstants.Key, List<KeyMapping>>> map = new EnumMap<>(KeyModifier.class);
+    static {
         for (KeyModifier modifier : KeyModifier.values())
-        {
             map.put(modifier, new HashMap<>());
-        }
     }
 
     /** Replaced by {@link #getAll(InputConstants.Key) getAll} */
     @Deprecated(forRemoval = true, since = "1.20.1")
     @Nullable
-    public KeyMapping get(InputConstants.Key keyCode)
-    {
-        KeyModifier activeModifier = KeyModifier.getActiveModifier();
-        if (!activeModifier.matches(keyCode))
-        {
-            KeyMapping binding = get(keyCode, activeModifier);
+    public KeyMapping get(InputConstants.Key keyCode) {
+        var activeModifier = KeyModifier.getActiveModifier();
+        if (!activeModifier.matches(keyCode)) {
+            var binding = get(keyCode, activeModifier);
             if (binding != null)
-            {
                 return binding;
-            }
         }
         return get(keyCode, KeyModifier.NONE);
     }
 
     @Nullable
     @Deprecated(forRemoval = true, since = "1.20.1")
-    private KeyMapping get(InputConstants.Key keyCode, KeyModifier keyModifier)
-    {
-        Collection<KeyMapping> bindings = map.get(keyModifier).get(keyCode);
-        if (bindings != null)
-        {
-            for (KeyMapping binding : bindings)
-            {
+    private KeyMapping get(InputConstants.Key keyCode, KeyModifier keyModifier) {
+        var bindings = map.get(keyModifier).get(keyCode);
+        if (bindings != null) {
+            for (var binding : bindings) {
                 if (binding.isActiveAndMatches(keyCode))
-                {
                     return binding;
-                }
             }
         }
         return null;
@@ -69,54 +56,50 @@ public class KeyMappingLookup
      * @param keyCode the key being pressed
      * @return the list of key mappings
      */
-    public List<KeyMapping> getAll(InputConstants.Key keyCode)
-    {
-        List<KeyMapping> matchingBindings = new ArrayList<KeyMapping>();
-        for (Map<InputConstants.Key, Collection<KeyMapping>> bindingsMap : map.values())
-        {
-            Collection<KeyMapping> bindings = bindingsMap.get(keyCode);
-            if (bindings != null)
-            {
-                matchingBindings.addAll(bindings);
+    public List<KeyMapping> getAll(InputConstants.Key keyCode) {
+        var ret = new ArrayList<KeyMapping>();
+        var modifier = KeyModifier.getActiveModifier();
+        if (!modifier.matches(keyCode)) {
+            for (var binding : get(modifier, keyCode)) {
+                if (binding.isActiveAndMatches(keyCode))
+                    ret.add(binding);
             }
         }
-        return matchingBindings;
+
+        if (!ret.isEmpty() || modifier == KeyModifier.NONE)
+            return ret;
+
+        for (var binding : get(KeyModifier.NONE, keyCode)) {
+            if (binding.isActiveAndMatches(keyCode))
+                ret.add(binding);
+        }
+
+        return ret;
     }
 
-    public void put(InputConstants.Key keyCode, KeyMapping keyBinding)
-    {
-        KeyModifier keyModifier = keyBinding.getKeyModifier();
-        Map<InputConstants.Key, Collection<KeyMapping>> bindingsMap = map.get(keyModifier);
-        Collection<KeyMapping> bindingsForKey = bindingsMap.get(keyCode);
-        if (bindingsForKey == null)
-        {
-            bindingsForKey = new ArrayList<KeyMapping>();
-            bindingsMap.put(keyCode, bindingsForKey);
-        }
+    private List<KeyMapping> get(KeyModifier modifier, InputConstants.Key keyCode) {
+        var bindings = map.get(modifier).get(keyCode);
+        return bindings == null ? Collections.emptyList() : bindings;
+    }
+
+    public void put(InputConstants.Key keyCode, KeyMapping keyBinding) {
+        var bindingsMap = map.get(keyBinding.getKeyModifier());
+        var bindingsForKey = bindingsMap.computeIfAbsent(keyCode, k -> new ArrayList<KeyMapping>());
         bindingsForKey.add(keyBinding);
     }
 
-    public void remove(KeyMapping keyBinding)
-    {
-        KeyModifier keyModifier = keyBinding.getKeyModifier();
-        InputConstants.Key keyCode = keyBinding.getKey();
-        Map<InputConstants.Key, Collection<KeyMapping>> bindingsMap = map.get(keyModifier);
-        Collection<KeyMapping> bindingsForKey = bindingsMap.get(keyCode);
-        if (bindingsForKey != null)
-        {
+    public void remove(KeyMapping keyBinding) {
+        var keyCode = keyBinding.getKey();
+        var bindingsMap = map.get(keyBinding.getKeyModifier());
+        var bindingsForKey = bindingsMap.get(keyCode);
+        if (bindingsForKey != null) {
             bindingsForKey.remove(keyBinding);
             if (bindingsForKey.isEmpty())
-            {
                 bindingsMap.remove(keyCode);
-            }
         }
     }
 
-    public void clear()
-    {
-        for (Map<InputConstants.Key, Collection<KeyMapping>> bindings : map.values())
-        {
-            bindings.clear();
-        }
+    public void clear() {
+        map.values().forEach(Map::clear);
     }
 }

src/main/java/net/minecraftforge/client/settings/KeyModifier.java

@@ -18,134 +18,102 @@ import org.lwjgl.glfw.GLFW;
 public enum KeyModifier {
     CONTROL {
         @Override
-        public boolean matches(InputConstants.Key key)
-        {
+        public boolean matches(InputConstants.Key key) {
             int keyCode = key.getValue();
             if (Minecraft.ON_OSX)
-            {
                 return keyCode == GLFW.GLFW_KEY_LEFT_SUPER || keyCode == GLFW.GLFW_KEY_RIGHT_SUPER;
-            }
             else
-            {
                 return keyCode == GLFW.GLFW_KEY_LEFT_CONTROL || keyCode == GLFW.GLFW_KEY_RIGHT_CONTROL;
-            }
         }
 
         @Override
-        public boolean isActive(@Nullable IKeyConflictContext conflictContext)
-        {
+        public boolean isActive(@Nullable IKeyConflictContext conflictContext) {
             return Screen.hasControlDown();
         }
 
         @Override
-        public Component getCombinedName(InputConstants.Key key, Supplier<Component> defaultLogic)
-        {
+        public Component getCombinedName(InputConstants.Key key, Supplier<Component> defaultLogic) {
             String localizationFormatKey = Minecraft.ON_OSX ? "forge.controlsgui.control.mac" : "forge.controlsgui.control";
             return Component.translatable(localizationFormatKey, defaultLogic.get());
         }
     },
     SHIFT {
         @Override
-        public boolean matches(InputConstants.Key key)
-        {
+        public boolean matches(InputConstants.Key key) {
             return key.getValue() == GLFW.GLFW_KEY_LEFT_SHIFT || key.getValue() == GLFW.GLFW_KEY_RIGHT_SHIFT;
         }
 
         @Override
-        public boolean isActive(@Nullable IKeyConflictContext conflictContext)
-        {
+        public boolean isActive(@Nullable IKeyConflictContext conflictContext) {
             return Screen.hasShiftDown();
         }
 
         @Override
-        public Component getCombinedName(InputConstants.Key key, Supplier<Component> defaultLogic)
-        {
+        public Component getCombinedName(InputConstants.Key key, Supplier<Component> defaultLogic) {
             return Component.translatable("forge.controlsgui.shift", defaultLogic.get());
         }
     },
     ALT {
         @Override
-        public boolean matches(InputConstants.Key key)
-        {
+        public boolean matches(InputConstants.Key key) {
             return key.getValue() == GLFW.GLFW_KEY_LEFT_ALT || key.getValue() == GLFW.GLFW_KEY_RIGHT_ALT;
         }
 
         @Override
-        public boolean isActive(@Nullable IKeyConflictContext conflictContext)
-        {
+        public boolean isActive(@Nullable IKeyConflictContext conflictContext) {
             return Screen.hasAltDown();
         }
 
         @Override
-        public Component getCombinedName(InputConstants.Key keyCode, Supplier<Component> defaultLogic)
-        {
+        public Component getCombinedName(InputConstants.Key keyCode, Supplier<Component> defaultLogic) {
             return Component.translatable("forge.controlsgui.alt", defaultLogic.get());
         }
     },
     NONE {
         @Override
-        public boolean matches(InputConstants.Key key)
-        {
+        public boolean matches(InputConstants.Key key) {
             return false;
         }
 
         @Override
-        public boolean isActive(@Nullable IKeyConflictContext conflictContext)
-        {
-            if (conflictContext != null && !conflictContext.conflicts(KeyConflictContext.IN_GAME))
-            {
-                for (KeyModifier keyModifier : MODIFIER_VALUES)
-                {
+        public boolean isActive(@Nullable IKeyConflictContext conflictContext) {
+            if (conflictContext != null && !conflictContext.conflicts(KeyConflictContext.IN_GAME)) {
+                for (KeyModifier keyModifier : MODIFIER_VALUES) {
                     if (keyModifier.isActive(conflictContext))
-                    {
                         return false;
-                    }
                 }
             }
             return true;
         }
 
         @Override
-        public Component getCombinedName(InputConstants.Key key, Supplier<Component> defaultLogic)
-        {
+        public Component getCombinedName(InputConstants.Key key, Supplier<Component> defaultLogic) {
             return defaultLogic.get();
         }
     };
 
     public static final KeyModifier[] MODIFIER_VALUES = {SHIFT, CONTROL, ALT};
 
-    public static KeyModifier getActiveModifier()
-    {
-        for (KeyModifier keyModifier : MODIFIER_VALUES)
-        {
+    public static KeyModifier getActiveModifier() {
+        for (KeyModifier keyModifier : MODIFIER_VALUES) {
             if (keyModifier.isActive(null))
-            {
                 return keyModifier;
-            }
         }
         return NONE;
     }
 
-    public static boolean isKeyCodeModifier(InputConstants.Key key)
-    {
-        for (KeyModifier keyModifier : MODIFIER_VALUES)
-        {
+    public static boolean isKeyCodeModifier(InputConstants.Key key) {
+        for (KeyModifier keyModifier : MODIFIER_VALUES) {
             if (keyModifier.matches(key))
-            {
                 return true;
-            }
         }
         return false;
     }
 
-    public static KeyModifier valueFromString(String stringValue)
-    {
-        try
-        {
+    public static KeyModifier valueFromString(String stringValue) {
+        try {
             return valueOf(stringValue);
-        }
-        catch (NullPointerException | IllegalArgumentException ignored)
-        {
+        } catch (NullPointerException | IllegalArgumentException ignored) {
             return NONE;
         }
     }

src/main/java/net/minecraftforge/network/ChannelListManager.java

@@ -119,7 +119,7 @@ public class ChannelListManager {
 
         var changed = new HashSet<ResourceLocation>();
         for (var channel : channels) {
-            // It also says nothing about the format of channels of channels so ignore bad channels.
+            // It also says nothing about the format of channels so ignore bad channels.
             if (channel.isEmpty())
                 continue;