Hanmac's Refactoring Thread

[Hanmac]

I commited the Second Part of my changes with Damage.
as you can see, CardDamageMap is used everywhere through the whole damage functions.

i also added a few test cases for:

• Damage + Lifelink
• Damage + Lifelink Multiplication (Gisela, Blade of Goldnight)
• Damage + Lifelink Redirection (Palisade Giant)
• Damage + Lifelink Multiple Damages Sources (Cone of Flame)

they all pass for now.

now i try the Fixed Amount Replacement, for that i might need a new Effect (and maybe a new Effect for the whole thing instead of a EffectEffect)

PS: hm maybe i do need to refactor getPreventNextDamage and getPreventNextDamageWithEffect but for that i am unsure yet.

[friarsol]

Hanmac with your recent Damage changes is there an easy way to lookup which Players have received Combat Damage in a turn for Tymna the Weaver?

[Hanmac]

There isnt already a way for that? Curious.
That should be added to xCount and is not connected to my other changes with damage. Just using DanageHistory might work.

[friarsol]

I didn't notice one, but haven't had that much IDE time. I think DamageHistory is only on Cards, not GameEntity. So it'd be awkward to handle that.

[Hanmac]

Hm okay. I will check it out later today, and if not add a variable for that. (I thought there was already a variable for CombatDamaged on Player)

Currently i don't have changes in player yet so I can easy include this fix.

[friarsol]

Yea I wasn't saying you needed to, just figured I'd ask since you were looking at that section recently.

[Hanmac]

@friarsol: i added wasDealtCombatDamageThisTurn to Player
then i noticed that Tymna does want all Players, even the ones that might lose the game already, so i added RegisteredOpponents

so for Tymna it probably should be
PlayerCountRegisteredOpponents$HasPropertywasDealtCombatDamageThisTurn

[Hanmac]

while going through all Damage Replacement cards with Damage Redirection and Damage splitting i found Glarecaster.
in most of the cases it does work as it should, but when damage is done to both the creature and the player, the Effect currently does not work correct.
Specially if a Spell does deal damage in two different lines.

@friarsol: what would be correct way how the card should work? and maybe an idea for that to fix that?

[friarsol]

while going through all Damage Replacement cards with Damage Redirection and Damage splitting i found Glarecaster.
in most of the cases it does work as it should, but when damage is done to both the creature and the player, the Effect currently does not work correct.
Specially if a Spell does deal damage in two different lines.

@friarsol: what would be correct way how the card should work? and maybe an idea for that to fix that?

Rulings
10/4/2004 If both you and this card would be dealt damage at the same time, or if either you or this card would be dealt damage from multiple sources at the same time, the redirection will apply to both chunks of damage.

This includes combat damage

[Hanmac]

yeah the problem is that currently "The next time" effects does exile them selves when they are done.

means that if a Damage source would deal X damage to the creature and you, only one of them is redirected.

what my problem is are Damage spells where the damage could not be done in one Effect, like X damage to creature and Y damage to you.
I think for that it might be even more complicated ...

[friarsol]

Ah.. I see what you mean. I'll think about the best way to handle that. Is there a way to delay the exiling of an effect is resolving until it finishes resolving?

[Hanmac]

currently there is no way to delay it ...
but i was thinking of adding a "OnlyOnce$ True" Param for the ReplacementEffect
with that the ReplacementHandler can "handle" that if the effect is not needed anymore. (and in case the host is a "Effect" can exile/remove it)

Edit:
another card to think about:
Channel Harm

One Spell does deal damage to multiple targets, Channel Harm does prevent them all and does deal them to target creature.

The problem there, does channel harm only deals one damage or for each of the prevented ones?

[Hanmac]

FINISH!

i commited all my changes to the Cards with DamageDoneReplacement to DealDamage effects.

all tests does pass, and i even added new ones (right place for that?)

[Agetian]

Congrats, Hanmac! It's an important milestone!

- Agetian

[Hanmac]

did a chat with chat.magicjudges.org/mtgrules

Two Problems:

First again Damage and lifelink
Cone of Flame while doing damage to three targets, only count as one damage Effect and causes only one lifegain
Fiery Confluence & Collective Defiance & Savage Alliance & Clan Defiance &Incendiary Command and only Charm spells (for now?) does cause multiple damage effects and multiple lifegain events

for Incite Rebellion i currently don't know.

for that i need to rewrite the Damage Effects again ... specially with thinking about the Prevention Effects.
(so first if its not part of a charm spell, combine the Damage Effects and then order them because of Prevention, and then deal the Damage at once with one DamageMap)


Aetherborn Marauder and similar like Spike Cannibal only causes one put counter effect. thats important because Hardened Scales only should add one extra counter, not one per creature.
For that i will need to extend the MoveCounter logic.
for Aetherborn Marauder probably:

Code: Select all

MoveCounter | ValidSource$ Permanent.YouCtrl+Other | Defined$ Self | CounterNum$ Any | CounterType$ P1P1

for Spike Cannibal:

Code: Select all

MoveCounter | ValidSource$ Creature | Defined$ Self | CounterNum$ All | CounterType$ P1P1