Card Development Questions

[Sloth]

Just added keywords to restrict a custom number of blockers.
Already implemented Pathrazer of Ulamog and Gorilla Berserkers.

Sloth, is there enough AI in there now for these cards to be scripted? I know the reason they weren't added before wasn't due to the complexity of the script, but the complexity of the AI needed to support them.

The reason they haven't been added yet is the interaction with "All creatures able to block CARDNAME do so.", "CARDNAME must be blocked if able.", "CARDNAME blocks each turn if able.", etc.

Not only does the AI need to make legal blocks, but we also have to make sure the blocking input for the human can't get stuck.

[Max mtg]

Looks like these need to be implemeneted.
Who is to take this assignment?

(I'll take it if noone else accepts this challenge)

[Sloth]

Looks like these need to be implemeneted.
Who is to take this assignment?

(I'll take it if noone else accepts this challenge)

I can take care of the AI.

For the human finishedMandatoryBlocks has to be tested a lot and fixed if necessary. I would be glad if you could do the fixing in this case Max.

EDIT: Be sure to check rule 509.1c.

[Max mtg]

I am not satisfied how Combat class is designed... will have to change it.
Why would AI directly assign new values to combat? AiAttackController thinks in terms of single opponent... What is use of Combat instance outside of Combat phase?

Attacking band - why would it know if it is blocked? It's just a set of cards attacking a given target.

I've found a good field to refactor a lot of aspects. )

[friarsol]

Attacking band - why would it know if it is blocked? It's just a set of cards attacking a given target.

Because if all blockers are removed it still is blocked. Or if a card (like Dazzling Beauty) is cast on a Band, it's blocked without blockers. Max, you do understand you don't need to rewrite every single file you touch, right?

[Max mtg]

Because if all blockers are removed it still is blocked. Or if a card (like Dazzling Beauty) is cast on a Band, it's blocked without blockers. Max, you do understand you don't need to rewrite every single file you touch, right?

No =)
Attacking Bands had some unneeded data - like blockers, defender etc. Blocked flag is not excessive, I see.

What do we have in Combat class? A lot of collections:
* List of bands
* Map: Attacker -> Band
* Map: Blocker -> List<Band>
* Map: Defender -> List<Card>

meanwhile all these data can be held in just 2 maps:

Code: Select all

    // Defenders, as they are attacked by hostile forces
    private final MapOfLists<GameEntity, AttackingBand> entitiesAttacked = new HashMapOfLists<GameEntity, AttackingBand>(CollectionSuppliers.<AttackingBand>arrayLists());
    // Blockers to stop the hostile invaders
    private final MapOfLists<AttackingBand, Card> bandsBlocked = new HashMapOfLists<AttackingBand, Card>(CollectionSuppliers.<Card>arrayLists());

So I cannot just leave that as it was.

[friarsol]

Attacking Bands had some data - like blockers, defender etc. Blocked flag is not excessive, I see.

What do we have in Combat class? A lot of collections:
* List of bands
* Map: Attacker -> Band
* Map: Blocker -> List<Band>
* Map: Defender -> List<Card>

meanwhile all these data can be held in just 2 maps:

Code: Select all

    // Defenders, as they are attacked by hostile forces
    private final MapOfLists<GameEntity, AttackingBand> entitiesAttacked = new HashMapOfLists<GameEntity, AttackingBand>(CollectionSuppliers.<AttackingBand>arrayLists());
    // Blockers to stop the hostile invaders
    private final MapOfLists<AttackingBand, Card> bandsBlocked = new HashMapOfLists<AttackingBand, Card>(CollectionSuppliers.<Card>arrayLists());

So I cannot just leave this as it is.

If you think that's a lot of collections, you should have seen before I started working on Banding.

Anyway, merging the List of Bands with the Defender->Bands should be fine (at least glancing at it quickly), but if you get rid of the Attacker->Band Map that slows down the lookup for removing attackers, and similar things that take single Attackers into account instead of AttackingBands as a whole.

(Unless I'm missing something, you would need to grab all Bands via the Defender map, and then check if each one contains the Attacker.)

[Max mtg]

It's fine then - each developer makes his iteration improving code over time. No wonder I didn't dare to change Combat before you cleaned it up and introduced bands. =D

How many creatures have to be involved into combat to notice the performance gain granted by the map from attacker card to AttackingBand?

Exactly.

Code: Select all

    public final AttackingBand getBandOfAttacker(final Card c) {
        for(Collection<AttackingBand> abs : entitiesAttacked.values()) {
            for(AttackingBand ab : abs) {
                if ( ab.contains(c) )
                    return ab;
            }
        }
        return null;
    }

[friarsol]

It's fine then - each developer makes his iteration improving code over time. No wonder I didn't dare to change Combat before you cleaned it up and introduced bands. =D

How many creatures have to be involved into combat to notice the performance gain granted by the map from attacker card to AttackingBand?

Exactly.

Code: Select all

    public final AttackingBand getBandOfAttacker(final Card c) {
        for(Collection<AttackingBand> abs : entitiesAttacked.values()) {
            for(AttackingBand ab : abs) {
                if ( ab.contains(c) )
                    return ab;
            }
        }
        return null;
    }

You don't find that triple nested loop ugly? I do.

[Max mtg]

Triple loop... well, it looks like one.

There is a field for doubts: first, index can be re-introduced. Meanwhile the performance gain is minimal and you have to maintain another map.

[swordshine]

Plane Jund has a trigger "Whenever a player casts a black, red, or green creature spell, it gains devour 5". Animate an etbreplacement effect? Currently the "Devour" keyword in Forge does not create a perfect replacement effect.

[jsv]

There was this report recently in the 1.4.2 beta thread:

- My Tolarian Entrancer is blocked by Village Ironsmith , but at end of combat, I don't get control of it.

I've fixed it by changing

Code: Select all

T:Mode$ AttackerBlocked | ValidCard$ Card.Self | ValidBlocker$ Creature

to

Code: Select all

T:Mode$ Blocks | ValidCard$ Creature | ValidBlocked$ Card.Self

Still I wonder, if it used to work as scripted and we have some regression there.

[swordshine]

There was this report recently in the 1.4.2 beta thread:

- My Tolarian Entrancer is blocked by Village Ironsmith , but at end of combat, I don't get control of it.

I've fixed it by changing

Code: Select all

T:Mode$ AttackerBlocked | ValidCard$ Card.Self | ValidBlocker$ Creature

to

Code: Select all

T:Mode$ Blocks | ValidCard$ Creature | ValidBlocked$ Card.Self

Still I wonder, if it used to work as scripted and we have some regression there.

These two scripts are different. "T:Mode$ Blocks" should only trigger once when it blocks any creatures (Lairwatch Giant and Guardian of the Gateless), this was implemented by ArsenalNut during GTC spoiler season. "T:Mode$ AttackerBlocked" should trigger multiple times for each blocking creature. I have not tested if these scripts are still working as supposed.

[Max mtg]

These two scripts are different. "T:Mode$ Blocks" should only trigger once when it blocks any creatures (Lairwatch Giant and Guardian of the Gateless), this was implemented by ArsenalNut during GTC spoiler season. "T:Mode$ AttackerBlocked" should trigger multiple times for each blocking creature. I have not tested if these scripts are still working as supposed.

This is a very useful comment.
If there are any problems with incorrectly firing triggers, I can fix it for you (because I changed Combat a couple of weeks ago and the very declare blockers step last night)

[Sloth]

These two scripts are different. "T:Mode$ Blocks" should only trigger once when it blocks any creatures (Lairwatch Giant and Guardian of the Gateless), this was implemented by ArsenalNut during GTC spoiler season. "T:Mode$ AttackerBlocked" should trigger multiple times for each blocking creature. I have not tested if these scripts are still working as supposed.

I didn't know about the new functionality of "T:Mode$ Blocks" and i think i broke it some month ago. But i think it can't really work that way.

We actually need three triggers:
1. One that triggers only once per attacker (currently "AttackerBlocked"). Example: Alley Grifters.
2. One that triggers for each pair of attacker-blocker (currently "Blocks"). Example: No Quarter, Righteous Indignation.
3. One that triggers only once per blocker (a new trigger?). Example: Lairwatch Giant.