fix for v1.5 UnnaturalCorpse event

[CodeOptimist]

I'm in the Discord as @CodeOptimist, ping me any time. Tested and appears to work.

I tried to copy the existing style, I thought the Ldc_I4_1 Or used by DrawTrackerTickPatch beneath this was clever and that also transpiles a CellRect.Contains(), so I used the same though here we return false with Ldc_I4_0 And. I put this patch next to that patch because of the similarity, though perhaps move it if it belongs with other v1.5 patches.

~I don't know much about this actual event, hopefully having the corpse move with one (or both) players watching doesn't ruin the whole gimmick, as doing things unwatched seems a little special/unique to this? Checking if it's unwatched by all clients sounds... complex.~

Here's the unpatched vanilla methods:

UnnaturalCorpse

private bool IsOutsideView()
{
	return (!base.SpawnedOrAnyParentSpawned || !base.MapHeld.reservationManager.IsReservedByAnyoneOf(this, Faction.OfPlayer)) && (Find.CurrentMap != base.MapHeld || !Find.CameraDriver.CurrentViewRect.ExpandedBy(1).Contains(base.PositionHeld));
}

AnomalyUtility

public static bool IsValidUnseenCell(CellRect view, IntVec3 cell, Map map)
{
	return (map != Find.CurrentMap || !view.Contains(cell)) && !cell.Fogged(map) && cell.Walkable(map) && cell.GetFence(map) == null && cell.GetDoor(map) == null;
}

As my first PR: You're always free to aggressively reject/refactor/rewrite. 👍 Cheers. 🍻

[CodeOptimist]

Oh just for more info, I looked at patching it at a lower level but they didn't really seem relevant. More like UI stuff that didn't need patched?

(Obviously you can't see the uses of cellRect = from this screenshot, but I investigated them.) Interesting to see that though CellRect has many methods, many of which return bools, I (think) I only see .Contains used for CurrentViewRect... which makes sense.

[Zetrith]

If you think about it, to ensure determinism, a method like IsValidUnseenCell can't ever return true in MP. The methods you patch either check the camera rect or Find.CurrentMap both of which are allowed to vary in multiplayer so these methods should just get a destructive prefix to always return false I think.

This will drastically affect the event though so I'll have to play around with it to decide how to handle it.

[CodeOptimist]

I forgot about Find.CurrentMap 😅

[CodeOptimist]

I think they're just combining tests of "validity" for the event but also "unseen" for spookiness. More like IsValidAndIsUnseen(). I think I can just patch out the map part also. They just don't want the player to "see" it teleport, but who cares. I think these are only used here. I can give it another go and report back. 👍

[CodeOptimist]

Updated and tested in-game. Corpse teleports around now like it should.

Here's a diff of the before and after opcodes. https://gist.github.com/CodeOptimist/454832f119239fbf3e8e4d4c6098a98e/revisions (I was really hoping it would embed that link. I'm a little disappointed.) (The diff is misaligned slightly; obviously patch adds lines 32 & 33, not 33 & 34.)

[SokyranTheDragon]

Thinking about this code again, I think only one patch would be enough, either to CellRect.Contains or Find.CurrentMap check. The condition in both cases could be simplified as (isTargetValid) && (map != Find.CurrentMap || !CellRect.Contains(target)).

We obviously cannot change the first part of the code, since this could likely introduce bugs (and same could be said about making a prefix to always return true). However, patching the second part would require changing at least one of the 2 conditions, as only 1 of them needs to be true (and this PR patches both).

Now, it is more of an observation I've made looking through some of the PRs that are both active or were merged. I'm wondering if we should keep both patches, or should we keep only the simpler of the 2?