Keyword enhancement

[Hellfish]

Splitting this from the Developer Plans thread.
Work is proceeding apace, got all "proper"(ie that's been printed on actual cards) keywords formatted, tinkering with replacing some of the old access methods.(I feel geriatrically slow , these past few workdays have been to blame
)
One question I have is if we should remove "macro" keywords. Macro keywords are what I call keywords of ours that, upon being parsed, is expanded into proper Spellabilities,Triggers,ReplacementEffects and StaticAbilities without need for hardcoded handling; examples include Unleash,etbCounter,Level up. I think it would simplify a lot if we could remove the hardcoded handling of these keywords and put the full script that would be generated anyway in the script file to begin with. Ideally, I would like to see only "(Keyword)( Magnitude|Cost|Type)*" -"official"- type keywords remaining. Some, like Haunt, will require hardcoded handling but still.etbCounter and ETBReplacement are the primary targets.

[friarsol]

I'm not sure about this.. Since the templating is all the same, it's more about consolidating code/function reuse.

K:Morph <cost> is simple and how the card is written. A single block of code that expands this template for every Morph, is actually simpler than littering up every card script that would have Morph.

A:SP$ Morph | Cost$ 3
A:AB$ Demorph | Cost$ <cost> | ...

on every single Morph card is totally overkill.


As far as other ones, like etbCounter and etbReplacement. I think I'd be ok with those being scriptified. Since some of the more complex ones are already half script anyway, so then it would just be replacing the keyword line with the main replacement line.

Did I misunderstand what you were asking?

[Hellfish]

I probably muddled up something, it's kinda late here(blame blame blame)

Official keywords like Morph or basically any that are actually on cards would still be keywords, it's just, as you state, the ones like etbCounter and etbReplacement and others like them.

[friarsol]

Ok gotcha, you mentioned Level up, so then I wasn't sure. Maybe you meant the MaxLevel keyword that's associated with Level Up? Those should probably just be Constant SVars.

K:Level Up <Cost>
SVar:MaxLevel:10

And then in the templating of Level Up, it'll grab the MaxLevel (if it exists) for AI considerations.

[Max mtg]

Speaking of official keywords (Morph, Unleash, Level up) it looks better to leave only the minimal set of data in each card script and store the common parts (the abilities those keywords turn into) in code for this stage of development at least. Later they could be moved to external dictionary file to ease modding).

As for those etbCoutners - don't know, I didn't work with them enough. Judging by the bugs reported for Sphere of the sun, they tave to be scripted... or denoted with a different syntax.

[Sloth]

Speaking of official keywords (Morph, Unleash, Level up) it looks better to leave only the minimal set of data in each card script and store the common parts (the abilities those keywords turn into) in code for this stage of development at least. Later they could be moved to external dictionary file to ease modding).

I second that. Why change hundrets of scripts? That would only cause dozens of bugs.

As for those etbCoutners - don't know, I didn't work with them enough. Judging by the bugs reported for Sphere of the sun, they tave to be scripted... or denoted with a different syntax.

Only those cards that have an etb replacement effect with two effects, need to be scripted (Sphere of the Suns and ?). 99% work fine. Why change them?

[friarsol]

As for those etbCoutners - don't know, I didn't work with them enough. Judging by the bugs reported for Sphere of the sun, they tave to be scripted... or denoted with a different syntax.

Only those cards that have an etb replacement effect with two effects, need to be scripted (Sphere of the Suns and ?). 99% work fine. Why change them?

Vivid Lands are the only other thing I can think of that has multiple ETB effects.

[Hellfish]

Hmm, I wanted to keep parsing much simpler by having all keywords follow theoffical format but I seem to have found a way to do that anyway (WITHOUT resorting to a ridiculously long string of booleans that are unreadable at a glance ) So, uh, carry on, I guess?
Also progress: Still removing old access methods and introducing needed Keywordtypes as I go.

EDIT: Still, there are quite a few keywords that feel very redundant, whose effect we actually more often do as proper traits (At the beginning of the end step, exile/destroy CARDNAME. At the beginning of the end step of this turn, you lose the game. At the beginning of your upkeep, sacrifice CARDNAME unless X. If damage would be dealt to CARDNAME,prevent that damage. Remove a +1/+1 counter from CARDNAME)

[swordshine]

EDIT: Still, there are quite a few keywords that feel very redundant, whose effect we actually more often do as proper traits (At the beginning of the end step, exile/destroy CARDNAME. At the beginning of the end step of this turn, you lose the game. At the beginning of your upkeep, sacrifice CARDNAME unless X. If damage would be dealt to CARDNAME,prevent that damage. Remove a +1/+1 counter from CARDNAME)

I think most of these "keywords" should be converted.
At the beginning of the end step, exile/destroy CARDNAME - This should be a delayed trigger in most cases. Unfortunately, delayed triggers don't work quite well in Forge. This keyword can't be converted to script yet.
If damage would be dealt to CARDNAME,prevent that damage. Remove a +1/+1 counter from CARDNAME - This prevention effect should be converted to script in order to work with other replacement effects. - Done in r24845.

[Hellfish]

What's the problem with delayed triggers,out of curiosity?

EDIT:Progress:Taking a break from access methods, moving kw realization from CardFactoryUtil to KeywordInstance.

[swordshine]

What's the problem with delayed triggers,out of curiosity?

EDIT:Progress:Taking a break from access methods, moving kw realization from CardFactoryUtil to KeywordInstance.

These delayed triggers are usually applied to a specific object. This object is remembered in the source card currently. Actually, the trigger should remember the object not the source card. This is a long existing bug. For exmaple, Activate Necropotence several times in a turn, Necropotence remembers the cards. The first trigger gives your all the remembered cards and other triggers don't do anything.

[Max mtg]

EDIT: Still, there are quite a few keywords that feel very redundant, whose effect we actually more often do as proper traits (At the beginning of the end step, exile/destroy CARDNAME. At the beginning of the end step of this turn, you lose the game. At the beginning of your upkeep, sacrifice CARDNAME unless X. If damage would be dealt to CARDNAME,prevent that damage. Remove a +1/+1 counter from CARDNAME)

Are these keywords at all?

These phrases are macros (invented here at Forge) for static abilities, rather than real keywords that are subject to be converted to enums

[Hellfish]

Yeah, there are quite a few that are either macro's (for things that must be hardcoded or otherwise) or flags rather than official keywords. I'm up to 85 of those vs 120 official ones.

EDIT: Back to replacing old access methods: 144 138 133 125 95 71 56 9 compilation errors remaining >_>

[Hellfish]

Quick progress update: When I finally got down to 0 errors it turns out that that only included the forge-gui project; instead of working (at least partially) it gives me 278 new compilation errors in forge-ai >_> HOWEVER, I'm hopeful I've squashed all but one problem now. The last thing to need a rework is changedCardKeyword stuff. The plan is to create a branch after that and start fixing whatever bugs I've introduced and refine the system, turning keywords into traits or rule changes or vice versa and generally streamline things.

[swordshine]

I'll convert "Permanents don't untap during their controllers' untap steps" keywords to a static ability granting affected permanents "CARDNAME doesn't untap during your untap step."