InscryptionModding / InscryptionAPI

Inscryption API
GNU Lesser General Public License v2.1
33 stars 24 forks source link

CardmodificationInfo not causing cards to die #92

Open LilyBwossom opened 2 years ago

LilyBwossom commented 2 years ago

I have a suggestion to make it so cards automatically die when their health becomes 0 as a result of adding a cardmodificationinfo (or in any other case really).

julian-perge commented 2 years ago

Here's a screenshot showing what Memez mentioned. The card itself does not die at all, even on turn end.

I'm using UnityExplorer to add the tempMod through code.

image

Pet-Slime commented 2 years ago

Just to explain the logic behind what Memez said: Most card games will cause a card to be removed from play if it has 0 hp the default. While there are no vanilla cases, there has been a few modders (when new) run into the issue of negative health modification leading to 0 hp. Such as when people are trying to evolve into a worse card.

So I agree that this is a 'bug' that should be fixed. Of course we should also have an extended property that allows cards to have 0 hp. As there are some situations where (even in other card games) that allow them to go beyond or sit at 0.

julian-perge commented 2 years ago

While there are no vanilla cases

Rereading this again, I think this is why vanilla doesn't handle it, because vanilla code has it setup so this situation isn't possible to achieve.

there has been a few modders (when new) run into the issue of negative health modification leading to 0 hp. Such as when people are trying to evolve into a worse card.

This sounds like they should handle their code better to not run into this situation, unless the sigil or some other factor explicitly states that it can be zero health and still live.

So I agree that this is a 'bug' that should be fixed. Of course we should also have an extended property that allows cards to have 0 hp. As there are some situations where (even in other card games) that allow them to go beyond or sit at 0.

I'm gonna go against implementing this in the main API. I say this because the more I think about it, the more confusing it will be to people who are new to modding their game, are going to assume that zero health cards will die when they reach zero health. Unless they install a mod knowing that the mod they're installing does something specifically with zero health cards.

Having it in the main API means that any mod that uses the API, which is going to be a lot, has the possibility of not killing cards when they reach 0 health through card mods, which is confusing to me. Even as a config option I'm still against it in the main API.

julian-perge commented 2 years ago

I will let @divisionbyz0rro and @Windows10CE weigh in if they wish to.

Pet-Slime commented 2 years ago

This sounds like they should handle their code better to not run into this situation, unless the sigil or some other factor explicitly states that it can be zero health and still live.

That's just people with jsonloader and vanilla sigils. Like I used in an example before: people used evolve into a worse card, which ended up at 0 hp and they were confused that it didn't die. Or they used health modification on the creature and it didn't die.

I say this because the more I think about it, the more confusing it will be to people who are new to modding their game, are going to assume that zero health cards will die when they reach zero health. Unless they install a mod knowing that the mod they're installing does something specifically with zero health cards.

assuming we patch it so cards would die at 0 health unless otherwise specified. How would that be confusing? Most cards that would achieve that 'does not die at 0 hp' would most likely do it through a sigil (and thus can read in the rulebook) or be a rare with a special property. like a card that says "the undying" or something.

julian-perge commented 2 years ago

Like I used in an example before: people used evolve into a worse card, which ended up at 0 hp and they were confused that it didn't die.

I think the setup up of this is:

  1. Card (with 4 health) that has evolve, has a mod that gives it -3 health.
  2. Now it has 1 health total.
  3. Card evolves into a weaker/lesser card with 1 health for example. Now it has 0 health (1 minus 3 == -2, game displayed 0) but still lives since it hasn't taken damage yet.

I think Evolve is really where the bug lies in this case, however, it's not really a bug since Evolve isn't meant to devolve into a less card. I'm not sure where else the patch would be, at least right now, to call the Die method in one place.

Or they used health modification on the creature and it didn't die.

This still feels like to me they should check for if the health mod is going to make it become zero and if so, they should call the Die method on the card. If a sigil or ability adds a card mod to a card, I feel like it's on the sigil creator to handle those cases.

Assuming we patch it so cards would die at 0 health unless otherwise specified. How would that be confusing?

I worded my response poorly, sorry about that. I do agree it would not be confusing if the API made it where having zero health on a card made it die. However, I still think it's on the sigil creator to call the Die method if they're adding card mods that would make the health become zero.

Outside of sigils, the only time Die gets called on a card is when TakeDamage or Sacrifice is called for the card.

Windows10CE commented 2 years ago

Where would the check for this even happen? Should the API check every card on every frame to see if one has zero HP?

Matz-42 commented 2 years ago

While there are no vanilla cases

Actually just realized that's not exactly true, there is in fact a situation where a card can lose health on the board without actually taking damage in vanilla, and that's gemified and losing a green gem that was on the board.

And making sure the card dies if at 0 hp or below is handled in the green gem ability behaviour.

public override IEnumerator OnDie(bool wasSacrifice, PlayableCard killer)
{
    yield return base.OnDie(wasSacrifice, killer);
    if (!base.Card.OpponentCard)
    {
        foreach (CardSlot slot in Singleton<BoardManager>.Instance.GetSlots(true))
        {
            if (slot.Card != null && slot.Card.Info.Gemified && !slot.Card.Dead && slot.Card.Health <= 0)
            {
                yield return new WaitForSeconds(1f);
                yield return slot.Card.Die(false, null, true);
            }
            slot = null;
        }
        List<CardSlot>.Enumerator enumerator = default(List<CardSlot>.Enumerator);
    }
    yield break;
    yield break;
}
Pet-Slime commented 2 years ago

Where would the check for this even happen? Should the API check every card on every frame to see if one has zero HP?

I would say after the trigger receivers fire and not every frame. we only need to check after something could have modified other things.

Pet-Slime commented 2 years ago

While there are no vanilla cases

Actually just realized that's not exactly true, there is in fact a situation where a card can lose health on the board without actually taking damage in vanilla, and that's gemified and losing a green gem that was on the board.

And making sure the card dies if at 0 hp or below is handled in the green gem ability behaviour.

public override IEnumerator OnDie(bool wasSacrifice, PlayableCard killer)
{
  yield return base.OnDie(wasSacrifice, killer);
  if (!base.Card.OpponentCard)
  {
      foreach (CardSlot slot in Singleton<BoardManager>.Instance.GetSlots(true))
      {
          if (slot.Card != null && slot.Card.Info.Gemified && !slot.Card.Dead && slot.Card.Health <= 0)
          {
              yield return new WaitForSeconds(1f);
              yield return slot.Card.Die(false, null, true);
          }
          slot = null;
      }
      List<CardSlot>.Enumerator enumerator = default(List<CardSlot>.Enumerator);
  }
  yield break;
  yield break;
}

Okay, so DM did think 0 hp = death. Just covered the one case it actually mattered in vanilla.

divisionbyz0rro commented 2 years ago

I'm just now looking over this conversation. I actually do believe it makes sense for cards with 0 health to die.

Where would the check for this even happen? Should the API check every card on every frame to see if one has zero HP?

I would say after the trigger receivers fire and not every frame. we only need to check after something could have modified other things.

It's not even that we don't need to check every frame - it's that if we did check every frame it could cause some really weird behavior. If we did it in PlayableCard.ManagedUpdate, we'd have to start a coroutine to handle the death animation and ensuing triggers - but that would break one of the fundamental game behaviors: only one thing happens at a time.

Let's imagine a world where I write an ability for a card that makes it give a -1 health card mod to all opposing cards when it enters the battlefield. And let's say that I have it do some sort of fancy animation (pseudo-code):

foreach (CardSlot slot in opposingSlots)
{
  yield return FancyAnimation();
  slot.Card.TemporaryMods.Add(new(0, -1));
}

When I yield for the FancyAnimation, the game starts "playing" again. At that point, ManagedUpdate will fire, see that the last card that I added a temporary mod to has zero health, and start a coroutine to make the card die. The card will die in the middle of my fancy animation! And let's imagine the worst case scenario: the card that dies has a some other fancy ability that makes every other card die at the same time. This could cause the original triggering card to die in the middle of FancyAnimation.

So it can't really be done in ManagedUpdate.

It would have to be done by patching the global trigger handlers.

TaoBoyce commented 2 years ago

I think the API should not cause 0 hp cards to die, if only because a modder may want to do something other than having the card die when it reaches 0 hp.

Pet-Slime commented 2 years ago

Game already makes cards die at 0 hp when done by combat. So modders would have to patch that function right now to avoid that from happening.

We were talking about making the card modification to 0 hp consistent with the rest of the game. As a gemmified card (a card modification) when it hits 0 due to a gem dying, also dies.

TaoBoyce commented 2 years ago

I know, I should have been more clear. I don't think the API should cause 0 hp cards caused by card modifications to die, since a modder may want to have the card do something other than dying. If the API automatically made any cards that have 0 hp due to card mods die, it could get in the way.

The main case where card mods leading to 0 health doesn't cause a card to die is evolve. Any case where it's caused by a modder should be handled by the modder.

Pet-Slime commented 2 years ago

You say " Any case where it's caused by a modder should be handled by the modder.", if that were to apply to the other bug fixes or modding situations, then there wouldn't be the hybrid cost render fix in the API, nor the pack mule fix in the API, or the multiple sigil render fix in the API.

As those are issues caused by the modder. Yet were included in the API to have a standardized version instead of every modder doing their own thing to fix those issues. And as stated before: we can infer that 0 hp = death is the intended interaction even with temporary mods since the checks on gemmified, which is a temporary mod.

Pet-Slime commented 2 years ago

all that said, it's been like two months since this was opened. I doubt it's going to happen in the API in any form, especially since juilian and Division couldn't agree on which side to take, with both of them being the senior contributors.

SaxbyMod commented 2 years ago

I feel when turn ends, they should die at least.