Improve Typescript definition for OngoingEffect impls#2269
Improve Typescript definition for OngoingEffect impls#2269
Conversation
Frugghi
requested review from
AMMayberry1,
CDMayberry,
adbronico,
brianries,
danbastin and
moyerr
as code owners
Frugghi
force-pushed
the
ongoing-effects-ts
branch
from
2c8c8a8 to
398d8c9
Compare
| title: 'This unit gains Raid 1 for each damage on him', | ||
| ongoingEffect: AbilityHelper.ongoingEffects.gainKeyword( | ||
| (target) => ({ keyword: KeywordName.Raid, amount: target.damage }) | ||
| (target: this) => ({ keyword: KeywordName.Raid, amount: target.damage }) |
There was a problem hiding this comment.
If at all possible I'd prefer to make it so that this type annotation isn't mandator. I think that should be doable, since we've done something similar for the templating on GameSystems attached to abilities. If we make IOngoingEffectGenerator a generic and then e.g. for IConstantAbilityProps we change the property to something like this:
ongoingEffect: IOngoingEffectGenerator<TSource> | IOngoingEffectGenerator<TSource>[];everything should flow through naturally. I haven't dug all the way down on that rabbit hole though, so I may be missing something
There was a problem hiding this comment.
I don't think this is possible. The problem is that ongoing effects are not always targeting the source (e.g. they could target a player or other cards) so using TSource wouldn't be correct.
I've updated the implementation to infer that modifyStats is only applied to units because it's true, but the same cannot be done for gainKeyword.
3 participants
No description provided.