Implement persistence analyzer - #641
kieranknowles1 wants to merge 9 commits into
Conversation
These will all be covered by the new unified analyser
ReferencesSelf required ref to be inside a cell. Test is failing as ref is used by cell, possible issue in usage cache
|
|
||
| public class IsFullLodAnalyzer : IContextualRecordAnalyzer<IPlacedObjectGetter> | ||
| { | ||
| public static readonly TopicDefinition FullLodWithoutPersistence = MutagenTopicBuilder.FromDiscussion( |
There was a problem hiding this comment.
Is this implemented in the PersistenceAnalyzer somehow? I don't see the topic there at all
Even if it were, i don't mind having analyzers that focus on one thing. Is there a reason we're moving topics over to PersistenceAnalyzer that I'm missing? I think that's actually preferable if it's not horribly optimized due to some internal reason.
But this logic here seems fine to remain in its own class like it was before?
There was a problem hiding this comment.
IsFullLod is checked for in PersistenceAnalyzer as one of the reasons for requiring persistence, same for package PersistentTargetAnalyzer via the usage cache and map marker not persistent.
I considered having both redundant as it's necessary for UnnecessaryPersistent to check all reasons an object may need to persist, which makes NotPersistent an effectively free addition
|
Tests look like they're failing |
They're currently failing in the Mutagen's nightly builds aren't currently working, otherwise I would update in this PR. See Mutagen-Modding/Mutagen#694 |
Usage cache fixes required for PersistenceAnalyzerTest.ReferencesSelf to pass
2 participants
Implement #250 unnecessary persistence and add #655 not persistent as its counterpart. Closes #95
#655 replaces #286 map marker not persistent, #475 full LOD not persistent, #519 forced ref not persistent and #534 package target not persistent, as persistence analyzer already needs to know all reasons for persistence being necessary in order to correctly detect unnecessary persistence.
Attempts to replicate CK behavior, which means XMarkers, water, and decals are expected to persist. I am unsure if this is essential or what the consequences of these records not persisting would be.
Analyzer requires construction of an unscoped usage cache, which comes with a significant CPU and memory overhead
Original description:
Implement #250 unnecessary persistence, repurpose
#63#286 map marker not persistent as general not persistent, removes #534 persistent package target and #475 full lod without persistence as these are fully covered by the new analyzer. Closes #95.Persistence requirement is determined based on the CK's rules for flagging a reference.
Analyzer requires construction of an unscoped usage cache, which comes with a significant CPU and memory overhead.