Fixed errors in making the keybinding replaceable.

- Changed KeyFactory to not be a singleton
- Simplified keyBindings initializer
- Modified and simplified keyListener
- Added method to copy existing keyState
- Changed button events
- Changed to newer lambda functions
- Modified updateKey method
- Left option for multiple keys bound to one command
- Updated and wrote new JavaDoc

#story[1197]

racevisionGame/src/main/java/visualiser/Controllers/KeyBindingsController.java

@@ -1,9 +1,6 @@
 package visualiser.Controllers;
 
 import javafx.application.Platform;
-import javafx.collections.ObservableList;
-import javafx.event.ActionEvent;
-import javafx.event.EventHandler;
 import javafx.fxml.FXML;
 import javafx.scene.control.Button;
 import javafx.scene.control.ListView;
@@ -11,25 +8,40 @@ import javafx.scene.input.KeyEvent;
 import javafx.scene.layout.AnchorPane;
 import visualiser.gameController.Keys.ControlKey;
 import visualiser.gameController.Keys.KeyFactory;
+
+import java.util.HashMap;
 import java.util.Map;
 
+import static visualiser.app.App.keyFactory;
+
 /**
  * Controller for the scene used to display and update current key bindings.
  */
 public class KeyBindingsController {
-    @FXML Button btnSave;
+    private @FXML Button btnSave;
-    @FXML Button btnCancel;
+    private @FXML Button btnCancel;
-    @FXML Button btnReset;
+    private @FXML Button btnReset;
-    @FXML ListView lstControl;
+    private @FXML ListView lstControl;
-    @FXML ListView lstKey;
+    private @FXML ListView lstKey;
-    @FXML ListView lstDescription;
+    private @FXML ListView lstDescription;
-    @FXML AnchorPane anchor;
+    private @FXML AnchorPane anchor;
-    String currentButton = null;
+    private Button currentButton = null;
-    KeyFactory keyFactory = KeyFactory.getFactory();
+    private KeyFactory newKeyFactory;
 
     public void initialize(){
+        // create new key factory to modify, keeping the existing one safe
+        newKeyFactory = copyExistingFactory();
+        initializeTable();
+        populateTable();
+        setKeyListener();
+    }
 
-        // headings for each column
+    /**
+     * Sets up table before populating it.
+     * Set up includes headings, CSS styling and modifying default properties.
+     */
+    public void initializeTable(){
+        // set the headings for each column
         lstKey.getItems().add("Key");
         lstControl.getItems().add("Command");
         lstDescription.getItems().add("Description");
@@ -37,47 +49,6 @@ public class KeyBindingsController {
         lstControl.getSelectionModel().select(0);
         lstDescription.getSelectionModel().select(0);
 
-        // populate columns with current key bindings and buttons to update
-        for (Map.Entry<String, ControlKey> entry : keyFactory.getKeyState().entrySet()) {
-            Button button = new Button(entry.getKey());
-            button.setMinWidth(120);
-            button.setOnAction(new EventHandler<ActionEvent>() {
-                @Override public void handle(ActionEvent e) {
-                    currentButton = button.getText();
-                    System.out.println("Button clicked");
-                }
-            });
-            lstKey.getItems().add(button);
-            lstControl.getItems().add(entry.getValue());
-            lstDescription.getItems().add(entry.getValue().getProtocolCode());
-        }
-
-        // stop the columns from being selectable, so only the buttons are
-        lstKey.getSelectionModel().selectedItemProperty()
-                .addListener((observable, oldvalue, newValue) -> {
-                    Platform.runLater(new Runnable() {
-                        public void run() {
-                            lstKey.getSelectionModel().select(0);
-                        }
-                    });
-                });
-        lstDescription.getSelectionModel().selectedItemProperty()
-                .addListener((observable, oldvalue, newValue) -> {
-                    Platform.runLater(new Runnable() {
-                        public void run() {
-                            lstDescription.getSelectionModel().select(0);
-                        }
-                    });
-                });
-        lstControl.getSelectionModel().selectedItemProperty()
-                .addListener((observable, oldvalue, newValue) -> {
-                    Platform.runLater(new Runnable() {
-                        public void run() {
-                            lstControl.getSelectionModel().select(0);
-                        }
-                    });
-                });
-
         // add CSS stylesheet once the scene has been created
         lstKey.sceneProperty().addListener((obs, oldScene, newScene) -> {
             if (newScene != null) {
@@ -85,7 +56,54 @@ public class KeyBindingsController {
             }
         });
 
-        setKeyListener();
+        // stop the columns from being selectable, so only the buttons are
+        lstKey.getSelectionModel().selectedItemProperty()
+                .addListener((observable, oldvalue, newValue) ->
+                        Platform.runLater(() ->
+                        lstKey.getSelectionModel().select(0)));
+        lstDescription.getSelectionModel().selectedItemProperty()
+                .addListener((observable, oldvalue, newValue) ->
+                        Platform.runLater(() ->
+                        lstDescription.getSelectionModel().select(0)));
+        lstControl.getSelectionModel().selectedItemProperty()
+                .addListener((observable, oldvalue, newValue) ->
+                        Platform.runLater(() ->
+                        lstControl.getSelectionModel().select(0)));
+    }
+
+    /**
+     * Populates the table with commands and their key binding details.
+     */
+    public void populateTable(){
+        // add each command to the table
+        for (Map.Entry<String, ControlKey> entry : newKeyFactory.getKeyState().entrySet()) {
+            // create button for command
+            Button button = new Button(entry.getKey());
+            button.setMinWidth(120);
+            button.setId(entry.getValue().toString());
+            button.setOnAction(e -> currentButton = button);
+            // display details for command in table
+            lstControl.getItems().add(entry.getValue());
+            lstKey.getItems().add(button);
+            lstDescription.getItems().add(entry.getValue().getProtocolCode());
+        }
+    }
+
+    /**
+     * Makes a copy of the keyfactory that does not modify the original.
+     * @return new keyfactory to be modified
+     */
+    public KeyFactory copyExistingFactory(){
+        newKeyFactory = new KeyFactory();
+        Map<String, ControlKey> newKeyState = newKeyFactory.getKeyState();
+        Map<String, ControlKey> oldKeyState = keyFactory.getKeyState();
+        newKeyState = new HashMap<>();  // clear default keys
+
+        // copy over commands and their keys
+        for (Map.Entry<String, ControlKey> entry : oldKeyState.entrySet()){
+            newKeyState.put(entry.getKey(), entry.getValue());
+        }
+        return newKeyFactory;
     }
 
     /**
@@ -96,17 +114,12 @@ public class KeyBindingsController {
         anchor.addEventFilter(KeyEvent.KEY_PRESSED, event -> {
             // if a button was clicked
             if (currentButton != null) {
+                System.out.println("button is clicked");
                 // update text on the button
-                ObservableList buttons = lstKey.getItems();
+                currentButton.setText(event.getCode().toString());
-                for (int i = 1; i < buttons.size(); i++) {
-                    if (currentButton == ((Button)buttons.get(i)).getText()) {
-                        ((Button)buttons.get(i)).setText(event.getCode().toString
-                                ());
-                        break;
-                    }
-                }
                 // update the control key
-                keyFactory.updateKey(currentButton, event.getCode().toString());
+                newKeyFactory.updateKey(event.getCode().toString(),
+                        currentButton.getId());
                 // remove current button selection
                 currentButton = null;
             }

racevisionGame/src/main/java/visualiser/Controllers/RaceController.java

@@ -10,7 +10,6 @@ import javafx.collections.transformation.SortedList;
 import javafx.fxml.FXML;
 import javafx.scene.chart.LineChart;
 import javafx.scene.control.*;
-import javafx.scene.control.Label;
 import javafx.scene.input.KeyCode;
 import javafx.scene.input.KeyEvent;
 import javafx.scene.layout.AnchorPane;
@@ -23,7 +22,6 @@ import shared.model.Leg;
 import visualiser.app.App;
 import visualiser.gameController.ControllerClient;
 import visualiser.gameController.Keys.ControlKey;
-import visualiser.gameController.Keys.KeyFactory;
 import visualiser.model.*;
 
 import java.io.IOException;
@@ -33,6 +31,7 @@ import java.util.ResourceBundle;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import static visualiser.app.App.keyFactory;
 
 
 /**
@@ -113,7 +112,7 @@ public class RaceController extends Controller {
 
     @Override
     public void initialize(URL location, ResourceBundle resources) {
-        KeyFactory keyFactory = KeyFactory.getFactory();
+//        KeyFactory keyFactory = KeyFactory.getFactory();
         infoTableShow = true;
 
         // Initialise keyboard handler

racevisionGame/src/main/java/visualiser/app/App.java

@@ -27,6 +27,7 @@ import javafx.stage.StageStyle;
 import javafx.stage.WindowEvent;
 import javafx.util.Duration;
 import visualiser.Controllers.MainController;
+import visualiser.gameController.Keys.KeyFactory;
 
 public class App extends Application {
 
@@ -37,6 +38,7 @@ public class App extends Application {
     private Label progressText;
     private static final int SPLASH_WIDTH = 676;
     private static final int SPLASH_HEIGHT = 227;
+    public static KeyFactory keyFactory = new KeyFactory();
 
     public static App app;
 

racevisionGame/src/main/java/visualiser/gameController/InputChecker.java

@@ -3,10 +3,11 @@ package visualiser.gameController;
 import javafx.animation.AnimationTimer;
 import javafx.scene.Scene;
 import visualiser.gameController.Keys.ControlKey;
-import visualiser.gameController.Keys.KeyFactory;
 
 import java.util.HashMap;
 
+import static visualiser.app.App.keyFactory;
+
 /**
  * Class for checking what keys are currently being used
  */
@@ -18,7 +19,7 @@ public class InputChecker {
      * @param scene Scene the controller is to run in parallel with.
      */
     public void runWithScene(Scene scene){
-        KeyFactory keyFactory = KeyFactory.getFactory();
+//        KeyFactory keyFactory = KeyFactory.getFactory();
 
         scene.setOnKeyPressed(event -> {
             String codeString = event.getCode().toString();

racevisionGame/src/main/java/visualiser/gameController/Keys/DownWindKey.java

@@ -9,11 +9,9 @@ public class DownWindKey extends ControlKey {
 
     /**
      * Constructor for Control
-     *  @param name    name of the key
-     *
      */
-    public DownWindKey(String name) {
+    public DownWindKey() {
-        super(name, BoatActionEnum.DOWNWIND);
+        super("Downwind", BoatActionEnum.DOWNWIND);
     }
 
     @Override

racevisionGame/src/main/java/visualiser/gameController/Keys/KeyFactory.java

@@ -13,30 +13,17 @@ public class KeyFactory {
     private Map<String, ControlKey> keyState;
 
     /**
-     * Singleton instance to enforce consistent key state
+     * Constructor for key state, set up initial state of each action.
      */
-    private static KeyFactory theFactory = new KeyFactory();
+    public KeyFactory() {
-
-    /**
-     * Singleton constructor for key state, set up initial state of each action.
-     */
-    private KeyFactory() {
         this.keyState = new HashMap<>();
-        keyState.put("Z", new ZoomInKey("Zoom In"));
+        keyState.put("Z", new ZoomInKey());
-        keyState.put("X", new ZoomOutKey("Zoom Out"));
+        keyState.put("X", new ZoomOutKey());
-        keyState.put("SPACE", new VMGKey("VMG"));
+        keyState.put("SPACE", new VMGKey());
-        keyState.put("SHIFT", new SailsToggleKey("Toggle Sails"));
+        keyState.put("SHIFT", new SailsToggleKey());
-        keyState.put("ENTER", new TackGybeKey("Tack/Gybe"));
+        keyState.put("ENTER", new TackGybeKey());
-        keyState.put("UP", new UpWindKey("Upwind"));
+        keyState.put("UP", new UpWindKey());
-        keyState.put("DOWN", new DownWindKey("Downwind"));
+        keyState.put("DOWN", new DownWindKey());
-    }
-
-    /**
-     * Get singleton instance of KeyFactory to interact with key state
-     * @return automatically constructed KeyFactory
-     */
-    public static KeyFactory getFactory() {
-        return theFactory;
     }
 
     /**
@@ -54,12 +41,21 @@ public class KeyFactory {
 
     /**
      * Update the key bound to a particular command in the keystate.
-     * @param oldKey the existing key to updated
+     * @param newKey the new key value for the command
-     * @param newKey the new key value to replace the old
+     * @param command the command to be updated
      */
-    public void updateKey(String oldKey, String newKey){
+    public void updateKey(String newKey, String command){
-        ControlKey controlKey = keyState.get(oldKey);
+        ControlKey controlKey = null;
+        String oldKey = null;
+        for (Map.Entry<String, ControlKey> entry : keyState.entrySet()) {
+            // if this is the correct command
+            if (entry.getValue().toString()==command){
+                controlKey = entry.getValue();
+                oldKey = entry.getKey();
+            }
+        }
         keyState.remove(oldKey, controlKey);
         keyState.put(newKey, controlKey);
     }
+
 }

racevisionGame/src/main/java/visualiser/gameController/Keys/SailsToggleKey.java

@@ -10,11 +10,9 @@ public class SailsToggleKey extends ControlKey {
 
     /**
      * Constructor for Control
-     *  @param name    name of the key
-     *
      */
-    public SailsToggleKey(String name) {
+    public SailsToggleKey() {
-        super(name, BoatActionEnum.NOT_A_STATUS);
+        super("Toggle Sails", BoatActionEnum.NOT_A_STATUS);
     }
 
     /**

racevisionGame/src/main/java/visualiser/gameController/Keys/TackGybeKey.java

@@ -9,11 +9,9 @@ public class TackGybeKey extends ControlKey {
 
     /**
      * Constructor for Control
-     *  @param name    name of the key
-     *
      */
-    public TackGybeKey(String name) {
+    public TackGybeKey() {
-        super(name, BoatActionEnum.TACK_GYBE);
+        super("Tack/Gybe", BoatActionEnum.TACK_GYBE);
     }
 
     @Override

racevisionGame/src/main/java/visualiser/gameController/Keys/UpWindKey.java

@@ -9,11 +9,9 @@ public class UpWindKey extends ControlKey {
 
     /**
      * Constructor for Control
-     *  @param name    name of the key
-     *
      */
-    public UpWindKey(String name) {
+    public UpWindKey() {
-        super(name, BoatActionEnum.UPWIND);
+        super("Upwind", BoatActionEnum.UPWIND);
     }
 
     @Override

racevisionGame/src/main/java/visualiser/gameController/Keys/VMGKey.java

@@ -1,6 +1,5 @@
 package visualiser.gameController.Keys;
 
-import javafx.scene.input.KeyCode;
 import network.Messages.Enums.BoatActionEnum;
 
 /**
@@ -10,11 +9,9 @@ public class VMGKey extends ControlKey{
 
     /**
      * Constructor for Control
-     *
-     * @param name    name of the key
      */
-    public VMGKey(String name) {
+    public VMGKey() {
-        super(name, BoatActionEnum.AUTO_PILOT);
+        super("VMG", BoatActionEnum.AUTO_PILOT);
     }
 
     @Override

racevisionGame/src/main/java/visualiser/gameController/Keys/ZoomInKey.java

@@ -5,8 +5,8 @@ package visualiser.gameController.Keys;
  */
 public class ZoomInKey extends ControlKey {
 
-    public ZoomInKey(String name) {
+    public ZoomInKey() {
-        super(name);
+        super("Zoom In");
     }
 
     @Override

racevisionGame/src/main/java/visualiser/gameController/Keys/ZoomOutKey.java

@@ -7,11 +7,9 @@ public class ZoomOutKey extends ControlKey{
 
     /**
      * Constructor for Control
-     *  @param name    name of the key
-     *
      */
-    public ZoomOutKey(String name) {
+    public ZoomOutKey() {
-        super(name);
+        super("Zoom Out");
     }
 
     @Override