Page 1 of 1

Removal of isSick

PostPosted: 20 Sep 2011, 18:05
by friarsol
Max,

Was there any real reason you decided to remove isSick? It seems to me that this was a common utility function that you've decided to remove.. and I can't tell why. Now instead of having the comparison in one location, you have it in multiple locations...

Re: Removal of isSick

PostPosted: 20 Sep 2011, 19:59
by Max mtg
isSick() was the same as hasSickness(), except for it extra checked for isCreature. Having both functions is confusing, you cannot decide which to pick unless you check its source code.

isSick was not common at all, there were only 4 ocurrencies, compared to some 10 of already existing calls to hasSickness.

I can restore if you miss it,

hasSickness = !hasKeyword("haste") && sickness;
isSick = !hasKeyword("haste") && sickness && isCreature() = hasSickness() && isCreature();


Speaking about good intentions, I'd like to implement that two sided cards... but found myself unable to understand the whole mechanics behind that transitions between stack, zones, triggers. Tried to remove some excessive code to make the whole thing simplier. Succeeded in AllZoneUtils, but found too little to do with card. Tried with isSick as some obvious target, but that change was not very welcome.

Re: Removal of isSick

PostPosted: 20 Sep 2011, 20:18
by friarsol
Well, you've been making a big deal about people repeating code, but now you are going about undoing a case where the exact opposite was occurring. So now the handling of Summoning Sickness is more verbose and more difficult to maintain.

I would think the only time hasSickness() should be checked is if a non-creature might be animated. In most cases, isSick() would be preferred. I don't really see how removing this function makes things "simpler" it's just a different way of accessing the same data. If isSick() isn't descriptive enough for you, then change it to something like isAffectedBySickness().

Also, someone (slapshot maybe) already stated they were going to attempt two-sided cards so I'm not sure why you are duplicating that effort.

Re: Removal of isSick

PostPosted: 20 Sep 2011, 21:48
by Max mtg
There is already hasSickness, no need for another sickness-checker function.
The only place where isSick was really needed were the Costs, however
Code: Select all
return source.isUntapped() && (!source.hasSickness() || !source.isCreature());
seems more logical there than source.isUntapped() && !source.isSick().

Yes, you don't know for sure who has taken that task. Niether do I know, but at least I can always take a try implementing them.

Re: Removal of isSick

PostPosted: 20 Sep 2011, 21:55
by Hellfish
Slapshot said he was tackling Doublefaced and Flip cards over in the Innistrad scripts thread.

Re: Removal of isSick

PostPosted: 28 Sep 2011, 01:55
by slapshot5
Hellfish wrote:Slapshot said he was tackling Doublefaced and Flip cards over in the Innistrad scripts thread.
Was going to, but I don't have the time to do it right. If someone else wants to try, go for it. I'm not actively working on it.

-slapshot5