bmwinstead / enigma-project

Other
1 stars 1 forks source link

Bug: GUI: "Default Configuration" does not reset rotor 4 if it is in use #35

Closed ikleyjl closed 10 years ago

ikleyjl commented 10 years ago

To reproduce:

  1. Load a new GUI
  2. Set rotor 4 to Beta or Gamma
  3. Press "Default Configuration"
  4. Observe lack of change in fourth rotor.

Expected result: Should change to the blank option.

Actual result: No change.

bmwinstead commented 10 years ago

I fixed this issue... however there is another one. Now the fourth ring setting doesn't go away. It's because of the rotorCheck construct that was put in.

case "fourthRotorRingSetting":
                if (rotorCheck && (fourthRotorChoice.getSelectedIndex() == 0)) { *<-- this*
                    // no fourth rotor
                    fourthRotorRingSetting.setSelectedIndex(0);
                    ringSettings[0] = '!';
                } else if (temp.getSelectedIndex() == 0) {
                    fourthRotorRingSetting.setSelectedIndex(1);
                    ringSettings[0] = temp.getSelectedItem().toString()
                            .charAt(0);
                } else {
                    ringSettings[0] = temp.getSelectedItem().toString()
                            .charAt(0);
                }
                break;

Because rotorCheck is false (set before this method call in update()), it goes to next if, and gets set back to '1'. I don't know if the rotorCheck provides other functionality and don't want to break anything, so I'll leave it to you to check out.

ikleyjl commented 10 years ago

When I was first coding the reset button, if the rotors are in a configuration like, say, "IV, VI, I", what would happen is that the first rotor would get set to I, which would then trigger the, "Can't have duplicate rotors" error, which would then reset the first rotor back to IV and leave the rotors not-reset. Because of this, I added the ability to temporarily disable that errorchecking. Now, this wouldn't normally affect fourthRotor (because the fourth rotor options are all unique, anyway), and could probably be removed from that without harming the rest of the code - the only reason I included the rotor checks in the fourth rotor was over some vague idea that during machine version dropdown, we could add a "Go nuts!" option or something similar that would allow people to build impossible machines such as duplicate rotors, use a non-thin reflector with the fourth rotor, or something similar.

However, probably better to remove the rotorCheck from the fourth rotor and worry about the "Go Nuts!" bridge (1) if we implement machine-version drop down and (2) if we include a "Go Nuts!" option to begin with.

ikleyjl commented 10 years ago

In retrospect, the rotorCheck shouldn't apply to that spot, anyway. So... removed and corrected. Bug closed.