Hanmac's Refactoring Thread

[Hanmac]

I am currently trying to do Gisela, Blade of Goldnight.
But only damage amount changes, not redirect or prevention yet.

My thoughts i am unsure if i should try to fix the current System or if i should try to rewrite it new.
Because I probably need to change the whole damage and combat damage System.

For example having a lifelink creature and then play Chandra's Ignition on it. How many lifegain trigger should happen?

We need some test cases where I can see if it works correct.

[Agetian]

Good luck with it! Hmm, I'm not sure which approach would be better, but I'll see if I can come up with some test cases to playtest it.

I don't think Lifelink uses the trigger/stack system anymore (I think it used to - before Magic 2010 changes or so), now it's a static effect, so it doesn't really cause triggers to happen, but a life gain event should happen for each creature that damage was dealt to (so, if you played Chandra's Ignition on Kalitas, Traitor of Ghet while the opponent had three creatures in play, I think it should gain life for each creature that opponent had + for dealing damage to the opponent as well). At least that's my understanding of it, I can't say I'm an expert in the in-depth rules :/

Here's some stuff I used to check for reference:
http://mtgsalvation.gamepedia.com/Lifelink (see 702.15a, 702.15e et al.).
https://www.reddit.com/r/magicduels/com ... his_a_bug/ (discussion of a Chandra's Ignition + Kalitas, Traitor of Ghet interaction).

- Agetian

[Hanmac]

Yes it deals damage and you gain life from that, but how many times should Ajani's Pridemate trigger? (Thats what I mean with lifegain trigger)

And is it different from combat damage? (In combat only one lifegain trigger does happen)

A creature with lifelink dealing combat damage is a single life-gaining event.

Ps: for that I might need to checkout the Replacement Effect for GainLife too.

[Agetian]

Hmm I have no idea how many Ajani's Pridemate counters you should get, to be honest :/ I assume once per creature and per opponent that damage was dealt to, but I may be wrong here.

- Agetian

[Marek14]

Basically, I'd say that there is as many lifegain events as the instances of the word "damage" in the ability. Chandra's Ignition on a lifelink creature would count as a single lifegain event, just like Pyroclasm with Soulfire Grand Master -- on the other hand, Cone of Flame with Soulfire Grand Master should probably count as three lifegain events because it deals damage in three separate bunches, sequentially.

It's related to "deals damage" triggers which should fire only once even if the damage is dealt to multiple things, and "is dealt damage" triggers which should fire only once even if the damage comes from multiple sources.

[Agetian]

Yep, I talked to people at chat.mtgjudges.org/mtgrules and they confirm that Ajani's Pridemate should trigger once because it's one life-gaining event from one source (and thus Forge currently gets it wrong actually, making it trigger once per creature dealt damage to).

- Agetian

[Hanmac]

okay while thinking about Lifelink + Damage,
i came to the conclusion that Lifegain should be totally removed from GameEntity/Card/Player damage functions.
and be done in the Damage Events (maybe excluding if they are replacement for now?)

for that i might need to rewrite everything around addDamage.
(it need to return int so i can sum it)

[Agetian]

It sounds like a good plan worth pursuing, especially if it'll eventually allow Forge to be rule-correct in other damage-related spheres (e.g. triggering cards like Ajani's Pridemate only once per lifegain event as opposed to multiple times as it does now).

- Agetian

[Hanmac]

i am currently reworking the damage redirection stuff to make it working for lifelink and lifegain stuff.

for now are full redirection effects.

===

problematic are damage split redirection like:
the next 1 damage which is dealt to X is dealt to Y instead.

because it does split the damage in two DamageDeal Effects:

• 1 damage to X
• N - 1 damage to Y

so far so good, but the lifelink damage are only allowed to cause ONE lifegain trigger. (it does need to calc it)
Maybe i need to add the SpellAbility to he addDamage functions, and add a count variable to SpellAbility. (or use some other kind of count var?)

then maybe i move it to some kind of different PreventEffect, to shortn the code of all cards.

then i might add a way for the controller to select in which order the damage need to be done. Thats important for such Damage prevention cards.

and final if that works correct we might do cards like Refraction Trap.

[Hanmac]

Did some massive rewrite of everything damage related but didn't commited it yet. Need to test it more if it works correct. (We might need more test cases)

Make a new CardDamageMap which is a Table<Card, GameEntity, Integer> use it to store damage information and do it as parameter to all damage fuctions.

With that I managed to get simple damage redirect working.
(All damage to X instead)

After commiting of that working state, I will try the "Next X damage to Y"

[friarsol]

Did some massive rewrite of everything damage related but didn't commited it yet. Need to test it more if it works correct. (We might need more test cases)

It'd be nice to get some automated cases for all of these larger rewrites. That way we can slowly build up our test suite and be more resilient to breaking changes

[Agetian]

Did some massive rewrite of everything damage related but didn't commited it yet. Need to test it more if it works correct. (We might need more test cases)

It'd be nice to get some automated cases for all of these larger rewrites. That way we can slowly build up our test suite and be more resilient to breaking changes

Yeah, agreed. With a program attempting to implement a rule set as huge as MTG has, it's near impossible to hand-test all the situations that may arise related to different abilities and interactions and stuff... :/

And very nice to hear the damage update is progressing, btw! Refraction Trap and Harm's Way would be nice to have since they're used in a good number of decks that are currently unsupported by Forge coming from sites like MTGDecks and MTGTop8

- Agetian

[Hanmac]

i commited the first thing:
its just the CardDamageMap, but so you guys can look at it what i mean.

i use it to pass it through all damage methods and redirections to keep track what dealt damage to what thing.
and then later i can call #dealLifelinkDamage to gain the lifegain from it at one place.

the class might be extended with more helper functions later too.

Combat already has its own MapTable i will use, with that i can get a better check if a creature was damaged. Even through redirections. (and later split damage too)

before i had Table<Card, GameEntity, Integer> used everywhere, but an extra class might be designed more clean.

I also use the Table to get the RememberDamaged right.

PS: while doing the Redirection damage i might look how to get Lava Burst working. but i don't think that has high priority.

[Marek14]

If you try Lava Burst, bear in mind that it only stope damage redirection to creatures and players, i.e. redirection to planeswalkers (as long as they are not simultaneously creatures) is still allowed. Same for Whippoorwill.

Another problem with prevention/redirection effects that work on fixed amount of damage (like Harm's Way or Healing Salve) is that if damage is dealt simultaneously by several sources, the player should be allowed to choose what exact damage to prevent or redirect (for example get rid of combat damage from Hypnotic Specter to avoid the trigger, while letting another creature come through).

[Hanmac]

yeah Lava Burst is currently on low priority

i currently did the all damage redirect yet.
like with Palisade Giant. (All Damage)

fixed Amount damage will be done later after i commited the current state.
(and yeah for that i need to find a way for the controller to order the Replacement effects ... thats also a point there the AI would need some logic)


PS: i also coded Elderscale Wurm to be like Worship
i don't know why it wasn't done before. (no need for replacement effect)