Ability to tilt the isometric camera

[AdamPlenty]

No description provided.

[PieterVdc]

check how it overlaps with #1781 that one has been draft for long time

[Loobinex]

I get this in the keys menu (yes I build the translation files)

[Loobinex]

Also a thing to note, it corrupts the settings.dat and my minimap zoom breaks:

deleting the settings.dat file fixes it, but that would be a pretty annoying thing to have to tell all users to do,...

[Loobinex]

This is straight view:

This is isometric with max tilt:

Would it be possible to set the limits so far that the same would be visible there? Notice how the increase level specials can hardly be seen in isometric even while tilting.

[AdamPlenty]

I get this in the keys menu (yes I build the translation files)

Did you delete your settings.dat?

[Loobinex]

Did you delete your settings.dat?

That fixed the translation strings there.

[Loobinex]

Straight view seems to be around tilt -360 ~ -364 to me.

[AdamPlenty]

Straight view seems to be around tilt -360 ~ -364 to me.

I'll need to double-check, but I think I saw some weirdness with too much tilt.

[AdamPlenty]

Straight view seems to be around tilt -360 ~ -364 to me.

I'll need to double-check, but I think I saw some weirdness with too much tilt.

Yeah, if I tilt too far down, I get things like this:

[Loobinex]

Straight view seems to be around tilt -360 ~ -364 to me.

I'll need to double-check, but I think I saw some weirdness with too much tilt.

Yeah, if I tilt too far down, I get things like this:

What value was this?

[AdamPlenty]

Straight view seems to be around tilt -360 ~ -364 to me.

I'll need to double-check, but I think I saw some weirdness with too much tilt.

Yeah, if I tilt too far down, I get things like this:

What value was this?

I don't know, but less than -500, I believe.

[walt253]

deleting the settings.dat file fixes it, but that would be a pretty annoying thing to have to tell all users to do,...

Maybe is it time to attempt moving at least the keybinds from settings.dat? #3329

[eddebaby]

With regards to settings.dat - it seems to me the only options for this PR (and any other future changes to FX that add new key binds, etc) are:

1. add (or wait for) support for a more flexible settings file (basically what walt just raised: Move keybinds from settings.dat on a more user-friendly text file #3329)
2. accept that settings.dat will need to be deleted for this PR to work
3. allow support for old settings.dat files (by changes to FX or by creating an external conversion tool)

I'd consider option 3 to be a poor approach (too much time spent both now for the immediate solution, and in the future for maintenance/improvement). I could elaborate on some ideas, but I don't want to encourage this option!

So, it might be a nice time to convert to using "settings.cfg" (or similar "text-based" solution) instead of settings.dat, i.e. go with option 1 above - the key is to get the "field names" in to the data, so that the position of the user data is not the deciding factor of what a particular piece of user data actually is.

Once the field names are in the data, then the data can be parsed in a similar fashion to the way FX parses scripts/cfg/toml etc - this makes it a lot easier to handle "missing user settings" within FX. The user will also be able to edit their keybinds (etc) with a text editor.

Also a "one-time" conversion from the last supported "settings.dat" to "settings.newwhatever" would be a nice touch on the user end (otherwise the user will lose their old settings).

Otherwise, I think it has to be option 2...

[Loobinex]

I too think option 2 is plenty nice. Changing the settings is not that much work and rare enough.

[AdamPlenty]

Could settings.dat be regenerated if its size is not what's expected?

[Loobinex]

Could settings.dat be regenerated if its size is not what's expected?

That would be nice. It does regenerate already when it does not exist.

[AdamPlenty]

Could settings.dat be regenerated if its size is not what's expected?

That would be nice. It does regenerate already when it does not exist.

We'd need a way to copy the current settings over, though. This probably won't be easy.

[eddebaby]

Could settings.dat be regenerated if its size is not what's expected?

It is fairly easy to add "size detection" of settings.dat (a list of old sizes would even allow a pretty good guess at what "version" the .dat file is).

It would be easy to regenerate settings.dat with the default settings, but as suggested, keeping the user's existing settings is not as straightforward.

To keep the old settings, you have to read the old settings file: with the complication being that "what the data/setting is" is determined by "the offset in settings.dat where the data is expected to be located".

Any reading of "old" dat files is the same as Option 3 above.

Here's my explanation:

• Any new setting might move the offset/location of other settings (settings won't always have to be moved, but when they do have to be moved this will "break" compatibility).
• The offset/location of the each setting cannot be guaranteed to be in the same place as previous version of the dat file - if a new setting is added to the middle of the dat file, then all of the subsequent settings will "move down" in the dat file.
• If new settings were only added to the end of the dat file, and old settings were never moved, resized, or deleted, then you could insinuate a smaller dat file size would indicate that it is only missing the "data at the end of the dat file", i.e. it is missing the "newest" settings.
• However, due to the fact that the keybinds are in an array, there is essentially 2 lists within the dat file, you can't have both the list of keybinds AND the end of "the other settings" at the end of the dat file, so adding anything new to whichever list isn't at the end of the file is the same as adding something to the middle of the file.
• So there is no way to have the current format of settings.dat communicate how the data is arranged.
• If settings.dat doesn't inform how the data is arranged, then it must be explicit in the code (as it is currently in struct GameSettings, the dat file is a "memcopy" of struct GameSettings). So you can say that the "map" of the data is defined in the code.
• So to support older version of the dat, the data would have to be identified by size and then parsed depending on the detected version of the dat file.
• A list of known "old file sizes" would give you reasonable detection of specific older version of settings.dat.
• For parsing old dat files you would need to have a "map" of each supported legacy settings.dat format - this would be akin to multiple legacy versions of struct GameSettings and/or setup_default_settings with some branching code sending the user's dat file to the correct parser. Each parser will need to read the dat file and spit out a valid and current struct GameSettings (which could then be immediately saved to get the user up to date). So copy-and-paste will do a lot of work here, as the parser will have similarities to existing code, and any future parsers can copy-and-paste the original parser. When thinking of ongoing maintenance: this isn't hard work, but it is bad work - creating additional work and a messier codebase each time the dat file changes.

Reading old settings.dat requires at least 1 piece of work (0 pieces of work is Option 2).

This is why I suggest Option 1 - with a flat text file, we can read the field name, then read the data - so old settings files can be more easily parsed because the presence of the field will be checked first, and absence of a field can be handled (i.e. it would work similar to parsing a CFG file). This should reduce future work to: "Add a new setting, handle absence/invalid value of said new setting, old settings files will be automatically supported and updated/resaved on first load".

Note: for option 2 to support a "one-time" conversion of old dat format to new format would need a "parser" as I described above for Option 3, but it is a one-time job rather than an on-going job, and the "old code" can just be mangled in to the "new parser" with no need to maintain many similar functions.

[eddebaby]

Here's a hastily thrown together "parser" function for old dat files, treat it as psuedo-code, as I've not checked it or anything, but I think it illustrates the concept:

TbBool load_old_settings(void)
{
    setup_default_settings(); // This ensure all of the missing fields are filled with default.
    struct OldGameSettings { // Old KFX settings
    unsigned char field_0;
    unsigned char video_shadows;
    unsigned char view_distance;
    unsigned char video_rotate_mode;
    unsigned char video_textures;
    unsigned char video_cluedo_mode;
    unsigned char sound_volume;
    unsigned char redbook_volume;
    unsigned char roomflags_on;
    unsigned short gamma_correction;
    int switching_vidmodes_index;
    struct GameKey kbkeys[40];
    TbBool tooltips_on;
    unsigned char first_person_move_invert;
    unsigned char first_person_move_sensitivity;
    unsigned int minimap_zoom;
    unsigned long isometric_view_zoom_level;
    unsigned long frontview_zoom_level;
    long mentor_volume;
    };
    struct OldGameSettings oldSettings;
    char* fname = prepare_file_path(FGrp_Save, "settings.dat");
    long len = LbFileLengthRnc(fname);
    if (len == sizeof(struct OldGameSettings))
    {
      if (LbFileLoadAt(fname, &oldSettings) == sizeof(struct OldGameSettings))
      {
          // copy valid fields from old settings file to current settings in RAM.
          &settings.field_0 = oldSettings.field_0;
          &settings.video_shadows = oldSettings.video_shadows;
          &settings.view_distance = oldSettings.view_distance;
          &settings.video_rotate_mode = oldSettings.video_rotate_mode;
          &settings.video_textures = oldSettings.video_textures;
          &settings.video_cluedo_mode = oldSettings.video_cluedo_mode;
          &settings.sound_volume = oldSettings.sound_volume;
          &settings.redbook_volume = oldSettings.redbook_volume;
          &settings.roomflags_on = oldSettings.roomflags_on;
          &settings.gamma_correction = oldSettings.gamma_correction;
          &settings.switching_vidmodes_index = oldSettings.switching_vidmodes_index;
          &settings.tooltips_on = oldSettings.tooltips_on;
          &settings.first_person_move_invert = oldSettings.first_person_move_invert;
          &settings.first_person_move_sensitivity = oldSettings.first_person_move_sensitivity;
          &settings.minimap_zoom = oldSettings.minimap_zoom;
          &settings.isometric_view_zoom_level = oldSettings.isometric_view_zoom_level;
          &settings.frontview_zoom_level = oldSettings.frontview_zoom_level;
          &settings.mentor_volume = oldSettings.mentor_volume;
          for (int i = 0, i < 40, i++)
          {
              &settings.kbkeys[i] = oldSettings.kbkeys[i];
          }
          return true;
      }
    }
    return false; // old settings file is wrong size, so will not be read
}

if the function returns true, then a up-to-date settings file can be saved consisting of the old settings along with the new settings with default values (i.e. what is held in RAM).

[Loobinex]

I don't mind having the users lose their old settings. This is not a frequent thing we do.

[eddebaby]

Here are 2 settings.dat files: settings.zip

Reproduced this way:

1. Deleted my settings.dat
2. Started a level on the master to generate settings.dat
3. Went to divine keys and changed 'WASD' to 'QASD'.
4. Copied and renamed copy to master_settings.dat
5. Build this PR and launched a map. Renamed settings.dat to pr_settings.dat

• Minimap was fine this time, the divine keys menu did not show the new entries in the list

Just to clarify - are you saying you got the same results on the key bind menu as the image you posted last week? (#3315 (comment))

I have compared the dat files you sent over and they indicate that the load_settings() function is working correctly - i.e. the old dat file is wiped and a new dat file (with the default settings) is created. The only differences between the files are:

• QASD in master_settings.dat, WASD in pr_settings.dat
• isometric_view_zoom_level is different in both files, and different to the default (presumably you scrolled the mouse wheel in game)
• The Gkey_TiltUp, Gkey_TiltDown, Gkey_TiltReset, and isometric_tilt settings are found only in pr_settings.dat (and are set to the default values)

Which all seems correct to me.

So some little nuisance in the code-base seems likely to be the cause:
Using this PR, try logging settings.kbkeys[40] in both load_settings() and whatever function is equivalent to "create key bind screen" or "show key bind screen" (make sure to log the function name, so that you can track the source of the log, and the order of events). Note: settings.kbkeys[40] is the "tilt up" key.

[Loobinex]

@eddebaby no, it did not show any text at all. I thought nothing of it because I remembered this as the corrupted list, but seeing the screenshot that was not it. I did not take the new language files that was why it stayed empty.
When I try now, I do not get any corruption.

@AdamPlenty Can you reproduce the corruption I experienced with settings.dat earlier? Ed, do you?

[eddebaby]

@eddebaby no, it did not show any text at all. I thought nothing of it because I remembered this as the corrupted list, but seeing the screenshot that was not it. I did not take the new language files that was why it stayed empty. When I try now, I do not get any corruption.

OK, maybe a bit of luck here and the "corrupt settings bug" is some "user error" to do with wrong files or some other developer headache!

@AdamPlenty Can you reproduce the corruption I experienced with settings.dat earlier? Ed, do you?

I'm not set up to test it, so I haven't seen the bug with this PR. As I said, I can remember something like this happening before, probably when adding the one-click keys to the menu, but I can't recall the cause unfortunately. But could easily have been due to user error.

I can only suggest trying to force a error on the first time launch of this PR with an old settings .dat file:

• load straight in to map
• go straight to key bind menu
• load to menu, then load map, then quit to menu, then key bind menu
• load save
• load old save
• load directly to save (if that is possible)
• could (loaded) packets have any impact?

Or if you can recall anything you might have been doing differently on that day!

[Loobinex]

Without vscode you can still download the prototypes directly from github. But I will test some more later too.

[AdamPlenty]

@AdamPlenty Can you reproduce the corruption I experienced with settings.dat earlier? Ed, do you?

When I first tested this, I had a corrupt key list until I deleted settings.dat, if that's what you mean?

[Loobinex]

@AdamPlenty Can you reproduce the corruption I experienced with settings.dat earlier? Ed, do you?

When I first tested this, I had a corrupt key list until I deleted settings.dat, if that's what you mean?

That is a corruption yes. Can you reproduce this?

[AdamPlenty]

@AdamPlenty Can you reproduce the corruption I experienced with settings.dat earlier? Ed, do you?

When I first tested this, I had a corrupt key list until I deleted settings.dat, if that's what you mean?

That is a corruption yes. Can you reproduce this?

No, I can't, surprisingly.

Edit: meant to quote your reply, but accidentally edited it instead 😒.

[Loobinex]

@AdamPlenty I plan to try myself too, but you are free to do so as well. I would like to investigate about the corrupt settings.dat this PR may or may not cause before I merge.

[AdamPlenty]

@AdamPlenty I plan to try myself too, but you are free to do so as well. I would like to investigate about the corrupt settings.dat this PR may or may not cause before I merge.

I haven't been able to reproduce it since, despite replacing the new settings.dat with the old one.

[Loobinex]

@AdamPlenty I plan to try myself too, but you are free to do so as well. I would like to investigate about the corrupt settings.dat this PR may or may not cause before I merge.

I haven't been able to reproduce it since, despite replacing the new settings.dat with the old one.

I guess we trigger a prototype and get some people to try it and see if they have issues.