eqPump (was VanillaEquipment) keyword

[Chris H.]

I took a peek at the VanillaEquipment code in CardFactory_Equiptment.java and see that it calls a CardFactoryUtil.vanila_{equip/onequip/unequip} method. There is a section in CardFactoryUtil which states:

Code: Select all

public void execute() {
    if(sourceCard.isEquipping()) {
        Card crd = sourceCard.getEquipping().get(0);
        if(!(Ab1 == "none")) crd.addExtrinsicKeyword(Ab1);
        if(!(Ab2 == "none")) crd.addExtrinsicKeyword(Ab2);
        if(!(Ab3 == "none")) crd.addExtrinsicKeyword(Ab3);
        crd.addSemiPermanentAttackBoost(Power);
        crd.addSemiPermanentDefenseBoost(Tough);
    }
}//execute()

`
At a quick glance it looks like "none" should not be added as a non-usable keyword. So, not knowing much about java I thought that a minor mod might do it:

Code: Select all

public void execute() {
    if(sourceCard.isEquipping()) {
        Card crd = sourceCard.getEquipping().get(0);
        if(!(Ab1.equals ("none"))) crd.addExtrinsicKeyword(Ab1);
        if(!(Ab2.equals ("none"))) crd.addExtrinsicKeyword(Ab2);
        if(!(Ab3.equals ("none"))) crd.addExtrinsicKeyword(Ab3);
        crd.addSemiPermanentAttackBoost(Power);
        crd.addSemiPermanentDefenseBoost(Tough);
    }
}//execute()

`
I ran a test and no more of those pesky "none" keyword entries show in the card detail panel. And the real keywords are infact added. Good!

Except that I got the "Flying" keyword twice. So, back to the drawing board.

[Chris H.]

OK, I now have the second copy of a Keyword prevented, no more Ornithopters with Flying, Flying. I would like to thank the person responsible for the code that I borrowed and modified.

The onEquip command now looks like this:

Code: Select all

Command onEquip = new Command() {
           
  private static final long serialVersionUID = 8130682765214560887L;
           
  public void execute() {
    if(sourceCard.isEquipping()) {
      Card crd = sourceCard.getEquipping().get(0);
      if(!(Ab1.equals ("none")) && (!crd.getKeyword().contains(Ab1))) crd.addExtrinsicKeyword(Ab1);   //prevent Flying, Flying
      if(!(Ab2.equals ("none")) && (!crd.getKeyword().contains(Ab2))) crd.addExtrinsicKeyword(Ab2);
      if(!(Ab3.equals ("none")) && (!crd.getKeyword().contains(Ab3))) crd.addExtrinsicKeyword(Ab3);
      crd.addSemiPermanentAttackBoost(Power);
      crd.addSemiPermanentDefenseBoost(Tough);
    }
  }//execute()
};//Command

`
If no one sees anything wrong with this change I will commit this to our SVN. It appears to be working correctly.

[DennisBergkamp]

If it works, it works Commit it.

[zerker2000]

In my opinion, all those "if hasKeyword don't add" checks should be moved to getText, with a list of the keywords that aren't cumulative. The current treatment keeps giving me concerns of e.g. enchanting something with Flight, equipping it with Neurok Hoversail, and then unequipping to find its "Flying" gone. I don't think that's currently the case(and I can surely think of a more realistic situation), but it is something that has been nagging the back of my mind ever since I first saw such code.

[Marek14]

In my opinion, all those "if hasKeyword don't add" checks should be moved to getText, with a list of the keywords that aren't cumulative. The current treatment keeps giving me concerns of e.g. enchanting something with Flight, equipping it with Neurok Hoversail, and then unequipping to find its "Flying" gone. I don't think that's currently the case(and I can surely think of a more realistic situation), but it is something that has been nagging the back of my mind ever since I first saw such code.

Multiple instances of the same keyword is actually correct, ruleswise. If it looks bad, it should be just suppressed in the text output, but not in the engine.

[zerker2000]

Thank you for translating .
On the topic of VanillaEquipment, it seems most of CardFactory_Equipment should be keywordable:
Lightning Greaves, Loxodon Warhammer, Behemoth Sledge, Fireshrieker, Bonesplitter, Trailblazer's Boots, Blight Sickle, Spidersilk Net, Whispersilk Cloak, Trusty Machete, No-Dachi, Shuko, and even Skullclamp(whose tricky code lies elsewhere) all seem to be coded manually. In fact, the only other code in there is three cards(Umbral Mantle, Sword of the Meek, Umezawa's Jitte) and the VanillaEquipment parser(with most of the code being in CardFactory_Util), and those should probably simply be moved back to CardFactory.

[Rob Cashwalker]

From the Bug thread:

Why is it "P:T:{X}:keywords" instead of "{X}:P:T:keywords"? The cost looks kind of weird in the middle, not to mention it might be better off using "Equip ", "SVar:PT:+P/+T", and "SVar:addkeywords:[...]".

Here's what I want to know, why not use the syntax parsing straight from the abPump keyword? Something like:

eqPump {EquipCost}:p/t/k1&k2&...&kn:spDesc:stDesc

First split by ":".
Equip cost is in k[0].
Action in k[1].
Descriptions if necessary.
(I don't think there are any drawback-based equipment... but it would easily integrate in there)

k[1] is then split by "/".
Based on the number of resulting elements, narrow it down to kw, p/t, p/t/kw.
Parse p/t for "+-X" and "+-Y" to retrieve the SVar.

If kw present, then split by "&" for as many multiple keywords as required, not just two.
(Now that I've commented on it, I'll look into adding this to spPump and abPump, I think that adds a couple more cards, but I like to be consistent!)

[Chris H.]

When Triadasoul commit'ed his VanillaEquipment keyword I realized that many of the existing equipments could then be converted from hard coded to keyword only … it was on my todo list with other maintenance type projects … I took care of a few things and then lost steam and became preoccupied with other things.

Looking at Zerker's list, I think all but the Skullclamp can be converted to the present keyword. I do not believe that the keyword in it's current form can handle the + or - sign.

I have the cards.txt portion ready:

Code: Select all

Behemoth Sledge
1 G W
Artifact Equipment
Equipped creature gets +2/+2 and has lifelink and trample.
VanillaEquipment:2:2:3:Lifelink:Trample:none

Blight Sickle
2
Artifact Equipment
Equipped creature gets +1/+0 and has wither.
VanillaEquipment:1:0:2:Wither:none:none

Bonesplitter
1
Artifact Equipment
Equipped creature gets +2/+0.
VanillaEquipment:2:0:1:none:none:none

Fireshrieker
3
Artifact Equipment
Equipped creature has double strike.
VanillaEquipment:0:0:2:Double Strike:none:none

Lightning Greaves
2
Artifact Equipment
Equipped creature has haste and shroud.
VanillaEquipment:0:0:0:Haste:Shroud:none

Loxodon Warhammer
3
Artifact Equipment
Equipped creature gets +3/+0 and has lifelink and trample.
VanillaEquipment:3:0:3:Trample:Lifelink:none

No-Dachi
2
Artifact Equipment
Equipped creature gets +2/+0 and has first strike.
VanillaEquipment:2:0:3:First Strike:none:none

Shuko
1
Artifact Equipment
Equipped creature gets +1/+0.
VanillaEquipment:1:0:0:none:none:none

Spidersilk Net
0
Artifact Equipment
Equipped creature gets +0/+2 and has reach.
VanillaEquipment:0:2:2:Reach:none:none

Trailblazer's Boots
2
Artifact Equipment
Equipped creature has nonbasic landwalk.
VanillaEquipment:0:0:2:Nonbasic landwalk:none:none

Trusty Machete
1
Artifact Equipment
Equipped creature gets +2/+1.
VanillaEquipment:2:1:2:none:none:none

Whispersilk Cloak
3
Artifact Equipment
Equipped creature is unblockable. Equipped creature has shroud.
VanillaEquipment:0:0:2:Unblockable:Shroud:none

`
I think that I should be able to convert one or two over per day. They all should be ready for the next beta release. Well, my wife is scheduled for her wrist surgery this thursday … so I can't make any promises.

I like the other ideas stated by Zerker and Rob. I am not sure if I will be of much help though. I am slowly moving up through the ranks from tester -> staff support -> Jr. coder trainee.

I went through the smilies and I looked for a face with a beanie cap ... with a spinning propeller ... it would have been perfect right here.

[Rob Cashwalker]

The basic skeleton for parsing this type of effect is already done, just copy it from abPump... practically line for line. The only addition is the split for "\&", which really should be trivial, after everything else.

[zerker2000]

Actually, including Skullclamp, as I think the skullCamp_destroy code is in GameAction, and basically counts the number of Skullclamps on any creature that gets sent to the graveyard from the field.

[Chris H.]

It looks like I can change the keyword name at the start of the CardFactory_Equipment.java file. The keyword parsing code section is located at the end of this file.

It looks like I should be able to handle some of Rob's suggestions. There is one thing that concerns me at this time. At the end of the file we have the parsed data being used to provide input parameters to several functions located in CardFactoryUtil:

Code: Select all

card.addSpellAbility(CardFactoryUtil.vanila_equip(card, Power, Tough, Ab1, Ab2, Ab3, manacost));
card.addEquipCommand(CardFactoryUtil.vanila_onequip(card, Power, Tough, Ab1, Ab2, Ab3, manacost));
card.addUnEquipCommand(CardFactoryUtil.vanila_unequip(card, Power, Tough, Ab1, Ab2, Ab3, manacost));

`
I guess this explains why we had "none" being used as a dummy keyword ability. I think that there may be a way to define a method as having a variable number of inputs rather than having a set number.

Hm, what if we pass all of keywords as a single string and then parse the string on the "&" character from within CardFactoryUtil? Currently, Lightning Greaves passes it's keywords as:

Ab1 = Haste
Ab2 = Shroud
Ab3 = none

I could possibly pass this as a single input parameter:

Abs = Haste&Shroud

I will think about it.

[zerker2000]

Or we could just pass the parsed array. I do think that a different delimiter should be used between the keywords, or possibly between the keywords and the "body" of VanillaEquipment. I think it should be split into:
"{Equipped/Enchanted}[Gets:+X/+Y][,Has:Keyword[&Another[&Third]]", which adds attach/unattach Commands;
"Equip X", which adds equip ability,
"Enchant Type", which adds a CIP. I think these could actually be coded completely independently of each other: after all, why Do you pass manacost to the commands or all the other artributes to vanilla_equip? Possibly equipped/enchanted could even be the same keyword(though they'd have to be split in the program, both for text's sake and for strange cases: I can visualize some combo giving abilities from an equipment to an enchantment, and those shouldn't activate).

[Rob Cashwalker]

Zerker beat me to it, but the keywords should just be passed as is (after splitting by "\&").

But I think the AI for Auras and Equipment are inherently different enough that their keywords should be different.

[Chris H.]

Yeah, I would like to thank Zerker for his idea. No need to pass the individual keyword abilites as individual strings. An array of strings will do the job. And I can get rid of the "none" keyword abilities and there will be no need to try to hide them.

I currently have all of the easy equips moved to the original keyword. Once I have the code in place to parse p/t for plus and minus signs I can then move Scullclamp over. I think the only thing holding it back at this time is that pesky -1 it gives to toughness and I do not think that the original code can handle this … although I could be wrong.

Before Dennis' next beta release I hope to change the keyword name. I would also like to group all of the equip via keyword cards together as this will assist me as I make a few mods. I will try to make sure that each of my commits will continue to work … so Dennis can release the next beta when he feels that it is good to go.

[zerker2000]

True, inherently they are different, both in AI and effect(technically, a card is only "enchanting" if it was attached by an Enchant ability, "equipping" by an equip one, and "attached" only matters for exile via e.g. Tawnos's Coffin; I think all of these are currently implemented correctly); however, I still think the format/parser should be equivalent, using the appropriate command based on type(and addEquipmentCommands(card, P, T, Keywords[]), and same with enchantments, should be voids in CardFactoryUtil, and execute "card.setText(s + card.getText())").