Rework deleted records handling

[Utumno]

We must decide what we do with deleted records - at some point we thought we must ignore deleted ones (see for instance https://github.com/wrye-bash/wrye-bash/commit/88097f47c83f29808438bff3278b4b244e7dd667#commitcomment-50362630 and comments starting https://github.com/wrye-bash/wrye-bash/issues/480#issuecomment-778765402 ) but turns out we don't and since those are header only records we end up dumping "default" values which is uneeded and can lead to crashes in MelUnion elements that try to dump the wrong alternative. So:

• we either ignore everywhere (but shouldn't we carry the deletion in the BP ?)
• we revert the hasty and partial ignores - see:
  • d3d29cbc9244cc211156324f68d17b72d0456085 seems correct
  • 63e6c55de9654728516be22afa266dc7c1bbbce5 seems correct
  • 33afe3f54ff8d83cfffad7a669c9ce511b328500 (uses iter_present_records)
  • 2924d0246db19b5c7c3cc0eae61c96f2707568a4 getActive -> iter_present must be reverted and go ahead and only dump the header for those deleted records

This was triggered by trying to dump a MelUnion that was part of a deleted record and was setDefault to a _Tes4Fid while its flags prescribed an int - no action was run on dump - boom

[Infernio]

We should not touch deleted records at all. Things that cause crashes with these:

• Writing out a deleted record's header with subrecords after it (this is always a CTD IIRC).
• Referencing a deleted record by FormID from another record (at best it's a CTD, at worst this does unpredictable things).

We shouldn't carry them forward. Either they come from an official file (i.e. a Bethesda DLC), in which case the record in question has become de facto nonexistent: you can't reference it any more and it's best to pretend it never existed at all. Or they come from a third party file, in which case that file is broken and has to be fixed by the author. Carrying them forward would also cause our own Plugin Checker to (rightfully) complain about the presence of deleted records.

Really, the only time the Deleted flag should be used is when developing a game master, i.e. by Bethesda, since then the CK will truly be able to delete the record in question. Any other use case creates land mines.

[Utumno]

Oh I see - and I was about to errr delete iter_present_records

Writing out a deleted record's header with subrecords after it (this is always a CTD IIRC).

Turns out this is just what we would do with that Dibella record before the FormId commit (see discord discussion) - so high time we got rid of this - probably the _Tes4Fid in setDefault should become a DummyFid then - I see no valid usecase where we dump a default FID

[lojack5]

Writing out a deleted record's header with subrecords after it (this is always a CTD IIRC).

In this case, do we want to make MelRecord skip dumping subrecords if the deleted flag is set?

[Infernio]

As a safety precaution, in case we somehow do end up with deleted records inside a file we're writing, sure.

Hopefully it'll never come to that - the BP is currently ignoring deleted records from importing, merging, etc. IIRC - and if it doesn't, it should.

[lojack5]

Well (Oblivion) DibellasWatchAddons.esp with Tweak Assorted -> Default Icons does trigger writing a deleted record with subrecords filled in currently.

It should be easy enough to implement that precaution though.

[Infernio]

Yeah, turns out tweakers use getActiveRecords: https://github.com/wrye-bash/wrye-bash/blob/c79c03a3955c4f91aa79c0c50b3eafefc70169a0/Mopy/bash/patcher/patchers/base.py#L143

Whereas preservers use iter_present_records and so skip deleted records.

[Infernio]

Figured out the cause of the remaining BSB tracebacks: deleted CELL records.

This causes a BSB of ((0, 0), (0, 0)), which then blows when compared to normal CELL BSBs. See 71eb2cad1fa74d6b8f9fd6c1bf7312edf7dce694 for the fix.