wrye-bash / wrye-bash

A swiss army knife for modding Bethesda games.
https://wrye-bash.github.io
GNU General Public License v3.0
455 stars 79 forks source link

Improve the usability of the INI Edits tab #652

Closed Infernio closed 1 year ago

Infernio commented 1 year ago

Started with me wanting to make the Jump to Installer link/shortcut from the Mods tab work on the INI Edits tab too, turned into a mini-refactoring that made a bunch of functionality from the Installers and Screenshots tab work on the INI Edits tab too.

Utumno commented 1 year ago

Good job - I am not sure about making IniInfo(s) -> FileInfo(s). For a related attempt see https://github.com/Utumno/wrye-bash/commit/358c239da489c509b8bd46041d25bff6180e6c81 - geared towards a more accurate hierarchy (masters/headers and a lot of other FileInfo machinery is very special to mods/saves/bsas and makes no sense for inis or screenshost - the latter was made into a FileInfo(s) to get rid of the refresh override but still inherited much from FileInfo(s) that does not belong there. Note also I got rid of the very ugly Abstract overrides of the DefaultIni class in linked commit). Do you think ca0516689fc309d51ca51e5985710d4e47d28687 could work without making IniInfo(s) to FileInfo(s)? I wanted to merge these _Afile/ListInfo Info changes (I think we better move towards those classes rather than towards FileInfo) right after 543 but we had other priorities. Speaking of which I am very near a working record grups implementation that is not ad hoc as the previous one - and should more easily generalize to other games.

Infernio commented 1 year ago

I tried without FileInfos, it failed due to copy_info only being in FileInfo (and unique_key, but that one is in ListInfo, so not a problem).

If we could work around the copy_info stuff, we should be able to do without FileInfos (I think).

Utumno commented 1 year ago

I will give it a whirl tomorrowish if you wish - and if you won't get there first - may be able to, as I just passed verification for Oblivion.esm WRLD 🍾 :P

Infernio commented 1 year ago

Okay, File_Duplicate appears to heavily need FileInfo(s) right now:

  1. It calls ask_resources_ok, which is super specific to plugins. We can (and should) fix that by making an override of the link itself for plugins so that that method only gets called for plugins - that one has always been an eyesore to me because it's so 100% plugin-specific. Edit: see 49c735c090d7fee68b9c57c2eda8326d2434e767.
  2. It calls unique_key, which is on ListInfo so that's fine, but unique_key itself requires fn_key to work, which is only on FileInfo.
  3. It calls validate_name, which is only on FileInfo.
  4. It calls copy_info, which is only on FileInfos.

I could just write a new INI_Duplicate that doesn't rely on these, but I'd rather avoid that.

Utumno commented 1 year ago

Let the eye move from id_cellBlocks :P

EDIT: I am (1h) near

EDIT2: validate_name remains (2 was already handled by ListInfo in bolt (minus the conflicts) while copy_info moved to TabledFileInfos which is a base of IniInfos)

Utumno commented 1 year ago

Done - guess needs some more testing and a bit of squashing :)

Now you are at it Installer_Duplicate could maybe be absorbed in there too