Skip to content

Patchers Refactoring #312

@Utumno

Description

@Utumno

Opening this to group patchers related refactorings

  • Config phase belongs to the gui side of patchers:
    • Move the methods/attributes that belong to the config phase in gui_patchers - done: 122784f
    • GetConfigPanel - use super
    • choiceMenu drop from list patcher - it's complicated enough without it - get_set_choice for the ride (and also selectCommands and all other cruft only used in lists merger harder, later)
    • Rename "text" to patcher_text (duh) and move it along with tip to the gui_patchers -> done: 10e680c
    • Remove #--Config Phase ----------------------------------------------------------- when done -> done: 10e680c
    • isActive (data) vs isEnabled (gui) -> see 10e680c, but needs more work, reduce isActive, bring to wrapper methods that raise if isActive == False
    • unresolved: autokey, getConfigChecked (configChecks, configItems), configItems, configChecks, patches_set, etc -> done: 10e680c
    • SpecialPatcher(_Abstractpatcher) ? -> done: 10e680c
  • ModReader.recHeader is now redundant we should use RecordHeader directly (or not?) - done: 6844284
  • Mopy/bash/basher/mod_links.py: Mod_ListPatchConfig, move isinstance to gui_patchers
  • Rebase the fallout branch on the refactored dev
  • patchName class variable duh
    • executing_patch rip off -> 1e14dbe
  • bodyTags to game/GAME/init -> done: 10e680c
  • Patcher dialog cleanup (partly done: 57c6950)
  • Investigate why we instantiate the gui panels once
  • PatcherPanel(_Abstractpatcher) ?
  • Docs ! Docs ! And Docs !(03baf2b)
  • CSV files were showing for any and all patchers, even ones that have no way to read or write CSV files - done: 9697b64
  • Continuing the above trend, split importers.py into mergers.py and preservers.py, then absorb as much as possible into _AMerger and _APreserver (see Patchers Refactoring #312 (comment)) - done: 9697b64
  • The tweaks need work -> done: b25727b
    • They should be as static as possible
    • There's a ton of duplicate boilerplate between them
    • They could be much faster if they just stopped copying over so much junk data
  • If we created new GMSTs instead of injecting, we could drop the whole stupid pickle nonsense, local imports, gmstEids, etc. -> done: b25727b
  • AMultiTweak key (duh, rename, note it's used as b'' in some...) and probably even choices - move this to class variable, those must be made as static as possible -> done: b25727b
  • Absorb the last few preservers into _APreserver:
    • CellImporter (hard) -> some work in eaeb831
    • GraphicsPatcher (see 480-pt3)
    • RoadImporter (will absorb itself once we absorb CellImporter)
    • Readd DIAL patching to the NamesPatcher?
  • Rewrite _AMerger, then absorb all remaining mergers:
  • parsers are badly in need of a base class - 426db77 introduced a (very) half-baked attempt at one
    • Specifically, CSV export/import is a tough nut, especially if we want some semblance of type safety
    • Post-parsers: try to get rid of all _parse_sources usages? Would absorb yet more importers - done: 9697b64, but see next point
    • Post-importers-split, we now have four very ugly _parse_csv_sources overrides -> d5f7a72 but finally 2b7f46c
  • Make sure Importer.srcs are paths and drop GPath call - see: _parse_csv_sources
    • unify Pbash logs - start from the importers (see the _inner_loop methods type_count -> type _mod_count, use defaultdicts) - is this relevant now that CBash is gone?
    • add an indication when random values are used (tweak pooling made all BP randomness deterministic, which helps greatly)
  • The fact that we store the GUI class names into pickle files is not great, means we can't rename the GUI classes to get a consistent naming scheme -> done: ac72709
    • Introduce a new class var (_config_key?) storing a permanent key for the pickle files (for backwards compat it should be identical to the existing GUI class names)
    • Then, rename both the GUI and model classes to some decent scheme (ideally one that has different names for the corresponding GUI and model patchers, e.g. two ImportScripts classes is terrible for Ctrl+N'ing)
  • _PatcherPanel.patcher_text is unused, but was probably intended to be used? -> done: d47fed5
  • The whole 'MergePatches must come first' thing stinks - find a better way to do that, then specify patchers as a set -> done: d7ded71
  • scanOrder == editOrder for all patchers -> done: 8a9b581
  • Finish off what Tweak Pooling started: Exterminate DynamicTweak and make all tweaks static -> done: f3b6578
    • Tweak Clothes - easy.
    • Tweak Settings - harder and very boring, but lots to gain.
    • Get the [] characters out of the translatable choice labels. That's internal syntax, adding or removing it in translations will break things. Instead, add a new tweak_default variable and add the brackets in __init__
    • Same with the Custom options. What if a language needs a different word order on something like Custom (in seconds)? Our custom dialog would break, because we check startswith(_(u'Custom')).
  • Race Records desperately needs to be split, this thing is more complex than the seven Import Actors patchers -> new issue: Split 'Race Records' into several different patchers #494
  • attrs are constant per loop (we loop once per record type, and attrs are set per record type) -> attrgetters are constant per loop too -> done: cf08053
    • Move all __attrgetter lookup out of the patcher loops and instead do them beforehand - keep in mind that the order of attrs must be preserved, so list[tuple] (or dict in py3)
  • Reduce copying of records in the BP #658 and more unnecessary copies:
    • we read the record twice and cache the raw blob in self.data - instead read once and unpack at least on read phase where keepAll == False
    • try and turn ins.unpack( ... ins.read(size ... to unpack_from( ins - we effectively copy the buffer maybe there is a way this is done inside struct
  • initData is horribly slow due to constantly load()ing the same files, once for each patcher
    • centralize this at one level above the patchers (i.e. PatchFile) -> 3db4bd8, 2cd690c
    • then take that cache and investigate if we could apply it to scanModFile as well - ideally we only want to open/load/convertToLongFids each file once!
  • scanModFile should really only have to be called on self.srcs to copy relevant records into the BP once. All other updating should be handled through PatchFile.update_patch_records_from_mod
  • backburner
    • self.classestemp -> should be local no ? Also srcClasses is probably good to go - note srcClasses is used to only load the record types that are actually present in src files. That said, see the initData point above which would definitely allow dropping this -> 5712443
    • Attack all the log nonsense - pick one of the formats (probably _plog1) and enforce it for all patchers (post-parsers/post-tweaks) - some work in 9697b64
    • Organize Patchers by Semantics Instead of Implementation #496

Closely interlinked with #480.
Sub-issues: #494 #497 and #658.
Blocks most new patchers, e.g. #492, #512 and #513.

Metadata

Metadata

Labels

A-patchersArea: Patchers (Everything in the patcher package)C-goalCategory: Long term goal. May be code-related or a meta goal.C-internalCategory: Keep track of internal refactoring. Implies the C-todo label except if it is a full C-goal

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions