Clear PortraitsCache post faction creation#596
Merged
Conversation
Fixes rwmt#591 Shortform: Fixes an issue where pawn textures lingered in `PortraitsCache` due to `thingID` changes during faction creation, causing hash mismatches. In multiplayer, this led to errors when accessing stale pawn maps during the update loop - now resolved by clearing the cache post faction creation. Longform: In multifaction gameplay, when a new faction is created, we use `Page_ConfigureStartingPawns`, which internally calls `PortraitsCache.Get`. The `PortraitsCache` is responsible for caching pawn textures, stored in a `Dictionary<Pawn, Texture>`. These textures are automatically cleaned up if they haven't been accessed for a few ticks. To manage this dictionary, `Pawn.GetHashCode` (or more generally `Thing.GetHashCode`) is overridden to return the `thingID`. However, starting with version 1.6, changes in how `thingID` is handled have introduced inconsistencies. As a result, cached textures in the `PortraitsCache` become orphaned due to mismatched hash codes. Specifically, when a pawn is sent over the network after faction creation has started, it is serialized and exposed to all players, including the issuer. During this process - particularly in `ScribeUtil.ReadExposable` - the `thingID` is initially a negative number (as expected, matching the value from `Page_ConfigureStartingPawns`). In version 1.5, these `thingID`s remained negative throughout this process (at least for the relevant duration). However, in 1.6, the `thingID` is replaced with a large positive number after `FinalizeLoading`. This change causes the pawn’s hash code to differ, so it can no longer be found in its original dictionary entry in the `PortraitsCache`. As a result, those cached textures are no longer properly tracked or removed. The main problem occurs when a new game is loaded and `PortraitsCache` still contains pawns from the previous session. During the update loop, the method `IsAnimated` is called, which we patch via the `PawnPortraitMapTime` class to access the pawn’s map. But because the pawn is from an old game, its map no longer exists - resulting in the error in this issue. **Fix:** The solution is to clear the portrait cache after the faction has been created. Although the cache is already cleared once before `Page_ConfigureStartingPawns` closes, the update loop continues for a few more frames - during which the affected pawns are regenerated in the cache. In singleplayer, this isn’t an issue, as those textures are removed again shortly afterward. However, in our multiplayer case, the timing causes problems.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Label: 1.6, Bug
That was a bit of a pain one for me - I kept chasing the wrong direction while the real issue was elsewhere. I had a fix early on, but understanding why it happened took a while. :'( :D
Fixes #591
Issue:
Shortform:
Fixes an issue where pawn textures lingered in
PortraitsCachedue tothingIDchanges during faction creation in multifaction, causing hash mismatches. This led to errors when accessing stale pawn maps during the update loop - now resolved by clearing the cache post faction creation.Longform:
In multifaction gameplay, when a new faction is created, we use
Page_ConfigureStartingPawns, which internally callsPortraitsCache.Get. ThePortraitsCacheis responsible for caching pawn textures, stored in aDictionary<Pawn, Texture>. These textures are automatically cleaned up if they haven't been accessed for a few ticks.To manage this dictionary,
Pawn.GetHashCode(or more generallyThing.GetHashCode) is overridden to return thethingID. However, 1.6 changes howthingIDis handled. As a result, cached textures in thePortraitsCachebecome orphaned due to mismatched hash codes.Specifically, when a pawn is sent over the network after faction creation has started, it is serialized and exposed to all players, including the issuer. During this process - particularly in
ScribeUtil.ReadExposable- thethingIDis initially a negative number (as expected, matching the value fromPage_ConfigureStartingPawns). In version 1.5, thesethingIDs remained negative throughout this process (at least for the relevant duration). However, in 1.6, thethingIDis replaced with a large positive number afterFinalizeLoading.This change causes the pawn’s hash code to differ, so it can no longer be found in its original dictionary entry in the
PortraitsCache. As a result, those cached textures are no longer properly tracked or removed.The main problem occurs when a new game is loaded and
PortraitsCachestill contains pawns from the previous session. During the update loop, the methodIsAnimatedis called, which we patch via thePawnPortraitMapTimeclass to access the pawn’s map. But because the pawn is from an old game, its map no longer exists - resulting in the error in this issue.Fix:
The solution is to clear the portrait cache after the faction has been created. Although the cache is already cleared once before
Page_ConfigureStartingPawnscloses, the update loop continues for a few more frames - during which the affected pawns are regenerated in the cache. I think in singleplayer, this isn’t an issue, as those textures are removed again shortly afterward. However, in our multiplayer case, the timing causes problems.