[BUG] Classic Mode checkboxes enabled when re-entering the Config menu [FIXED]

[WillLem]

Steps to reproduce:

1) Go to Config menu and hit "Activate Classic Mode" (relevant checkboxes will be disabled)
2) Click Apply, OK
3) Exit the Config menu
4) Re-enter the Config menu

The checkboxes are enabled again.

[namida]

If I remember correctly, you implemented this so that it disables the checkboxes when Classic Mode is turned on. If the config menu is opened with Classic Mode already turned on, this code would likely not trigger (though it depends exactly how the code is written).

The best way to handle this might be to create a procedure "SetCheckboxesEnabledByClassicMode" which sets the enabled / checked values of checkboxes as applicable based on the Classic Mode setting (or on a boolean value passed to it, either way works), and call this in any situation where the checkboxes' states need to be set - so, when the form is first shown, and when the Classic Mode setting is changed.

[WillLem]

Got it, thanks for the tip @namida.

Solved the problem using this:

Code Select Expand

Line 83
procedure SetClassicModeCheckboxes;

Line 425
procedure TFormNXConfig.FormCreate(Sender: TObject);
begin
  SetClassicModeCheckboxes;
end;

Line 474
procedure TFormNXConfig.SetClassicModeCheckboxes;
  begin
    if GameParams.ClassicMode then
      begin
         cbHideShadows.Enabled := false;
         cbHideClearPhysics.Enabled := false;
         cbHideAdvancedSelect.Enabled := false;
         cbHideFrameskipping.Enabled := false;
         cbHideHelpers.Enabled := false;
         cbHideSkillQ.Enabled := false;
      end;
  end;

Commit d9aaaa699 applies the fix.

[namida]

I would suggest checking the state of the Classic Mode checkbox rather than of the actual setting. This is because (at least if it was implemented the same way as the other options) the setting isn't actually changed until the options menu is closed. This way, you could call the same function when the checkbox is clicked, rather than duplicating the code.

[WillLem]

I would suggest checking the state of the Classic Mode checkbox rather than of the actual setting.

I tried that tbh, it didn't work. I tried a number of different arrangements to prevent code duplication*, but none of them wanted to fly (either the checkboxes were always disabled, or always enabled, or they changed when re-entering the config menu but didn't change when clicking the button, things like that).

*specifically, I had SetClassicModeCheckboxes do all of the Enabling/Disabling, then asked the Activate/Deactivate buttons to call SetClassicModeCheckboxes. This works for neither checking for the mode nor the cb state. It's also buggy if I don't ask them to call the function.

Unfortunately, the above fix seems to be the only one that works as desired. Once I've got the other features up and running and have a stable version out, I'll take a look at this again.

[WillLem]

There is a variable called fIsSetting which (I think) checks all settings on the way into the config menu and sets the checkboxes accordingly. I'm pretty sure that this is what's needed to condense everything down to a single block.

(Note to self: see Commit 7dd8fdd75 for an example of how this is used to stop the Reset Window checkboxes from being checked iff Show Minimap is pre-checked when entering the Config menu)



Yeah, after a bit of exploration, the SetCheckboxes method is the best way around this.