SotMSteamMods / CauldronMods

Mod adaptations of the Cauldron decks for Sentinels of the Multiverse for Steam Workshop
MIT License
15 stars 9 forks source link

Tango One - Shinobi Assassin + Chameleon Armor/Critical Hit #1664

Closed wrhyme29 closed 8 months ago

wrhyme29 commented 8 months ago

Closes #1596

After thorough testing, I figured out that the infinite loop was happening during the Pretend phase.

I added standalone triggers in Pretend to try to get the Damage Preview to not show the Error message, but it didn't work. If others have thoughts on how to get that to show as expected, I'm happy to see those

jamespicone commented 8 months ago

You can force an ambiguous resolution in preview mode:

if (GameController.PreviewMode)
{
    e = GameController.MakeYesNoCardDecision(DecisionMaker, SelectionType.AmbiguousDecision, Card, cardSource: GetCardSource());
    if (UseUnityCoroutines)
    {
        yield return GameController.StartCoroutine(e);
    }
    else
    {
        GameController.ExhaustCoroutine(e);
    }
    yield break;
}

I've got a similar card in Parahumans that forces ambiguous resolution in the preview case and does a yield break in the pretend case; that seems to work; but it's not as extensively tested as it ought to be. I'm a bit worried about the behaviour with redirects. I think the correct behaviour is to act after redirects, which is what this should do, but in general doing something different in pretend and real is worrying.

wrhyme29 commented 8 months ago

You can force an ambiguous resolution in preview mode:

if (GameController.PreviewMode)
{
    e = GameController.MakeYesNoCardDecision(DecisionMaker, SelectionType.AmbiguousDecision, Card, cardSource: GetCardSource());
    if (UseUnityCoroutines)
    {
        yield return GameController.StartCoroutine(e);
    }
    else
    {
        GameController.ExhaustCoroutine(e);
    }
    yield break;
}

I've got a similar card in Parahumans that forces ambiguous resolution in the preview case and does a yield break in the pretend case; that seems to work; but it's not as extensively tested as it ought to be. I'm a bit worried about the behaviour with redirects. I think the correct behaviour is to act after redirects, which is what this should do, but in general doing something different in pretend and real is worrying.

I'm making this change, but it still doesn't change the Damage Preview from showing the Error for either CriticalHit or ChameleonArmor

EDIT: Actually the Shinobi Assassin test goes back to infinite loop with this new setup, so its a no go

jamespicone commented 8 months ago

Works for me: image

Test:

   [Test]
        public void TestChameleonArmorAmbiguousOutcome()
        {
            // Arrange
            SetupGameController("BaronBlade", DeckNamespace, "Ra", "Legacy", "TheTempleOfZhuLong");
            StartGame();

            PutOnDeck(tango, GetCard(FarsightCardController.Identifier));

            Card assasin = PlayCard("ShinobiAssassin");
            DecisionSelectLocation = new LocationChoice(tango.TurnTaker.Deck);
            DestroyCard(assasin);

            // Act
            PlayCard(ChameleonArmorCardController.Identifier);
            var preview = GetDamagePreviewResults(baron, tango, 5, DamageType.Cold);
            AssertDamagePreviewResultNotKnowable(preview, 0);
        }

The chunk of code I've modified:

public override void AddTriggers()
{
    base.AddTrigger<DealDamageAction>(dda => dda.Target.Equals(this.CharacterCard) && dda.Amount > 0,
        this.RevealTopCardFromDeckResponse,
        new TriggerType[]
        {
            TriggerType.RevealCard,
            TriggerType.CancelAction
        }, TriggerTiming.Before
    );
}

private IEnumerator RevealTopCardFromDeckResponse(DealDamageAction dda)
{
    if (GameController.PreviewMode)
    {
        var e = GameController.MakeYesNoCardDecision(DecisionMaker, SelectionType.AmbiguousDecision, Card, cardSource: GetCardSource());
        if (UseUnityCoroutines) { yield return GameController.StartCoroutine(e); }
        else { GameController.ExhaustCoroutine(e); }
        yield break;
    }

    if (dda.IsPretend) yield break;

    List<YesNoCardDecision> storedYesNoResults = new List<YesNoCardDecision>();
jamespicone commented 8 months ago

I haven't tested with redirects though.

wrhyme29 commented 8 months ago

Using the exact same code as you I still get the error:

image

jamespicone commented 8 months ago

That's super weird. Can you pull the exception out of Player.log?

wrhyme29 commented 8 months ago

Weird - I just tried again to pull up the logs and it showed correctly. I must have opened it up before the DLL had copied over

wrhyme29 commented 8 months ago

Changes have been added

wrhyme29 commented 8 months ago

I just tested redirects with Tango One (Chameleon Armor out, 8 HP), Benchmark (Intervening Path Calculator out, 31 HP), and Writhe (Shadow Cloak, Lies in the Shadow out, 19 HP), damage directed to Tango One, redirected to Benchmark, redirected back to Tango One. Each time, damage preview showed the error. Here's the exception in the Player.log

Exception calling GetDamagePreviewResults! NullReferenceException: Object reference not set to an instance of an object at Handelabra.Sentinels.View.UIDecisionWindow.UpdateCalculationTable (Handelabra.Sentinels.View.UICardView cardView, System.Int32 damagePage) [0x0049d] in :0 UnityEngine.DebugLogHandler:Internal_LogException(Exception, Object) UnityEngine.DebugLogHandler:LogException(Exception, Object) UnityEngine.Logger:LogException(Exception, Object) UnityEngine.Debug:LogException(Exception, Object) Handelabra.Sentinels.View.UIDecisionWindow:UpdateCalculationTable(UICardView, Int32) Handelabra.Sentinels.View.UIDecisionWindow:OnAnswerSelected(UIDecisionAnswer, Boolean) Handelabra.Sentinels.View.d__28:MoveNext() UnityEngine.SetupCoroutine:InvokeMoveNext(IEnumerator, IntPtr)

I observed the same behavior when the damage went from Writhe > Tango One > Benchmark

wrhyme29 commented 8 months ago

I would say, lets write up a new issue for it and close this PR