Card-Forge / forge

An unofficial rules engine for the world's greatest card game.
https://card-forge.github.io/forge/
GNU General Public License v3.0
992 stars 566 forks source link

Spell Refactoring Additional Cost #2472

Open Hanmac opened 1 year ago

Hanmac commented 1 year ago

Currently: Spells that has "As an additional cost to cast this spell" like Discard a Card like Tormenting Voice use this as part of their cost for the spell

Name:Tormenting Voice
ManaCost:1 R
Types:Sorcery
A:SP$ Draw | Cost$ 1 R Discard<1/Card> | CostDesc$ As an additional cost to cast this spell, discard a card. | NumCards$ 2 | Defined$ You | SpellDescription$ Draw two cards.

Maybe better?

Name:Tormenting Voice
ManaCost:1 R
Types:Sorcery
S:Mode$ RaiseCost | ValidCard$ Card.Self | Activator$ You | Type$ Spell | Cost$ Discard<1/Card> | EffectZone$ All | Description$ As an additional cost to cast this spell, discard a card.
A:SP$ Draw | NumCards$ 2 | Defined$ You | SpellDescription$ Draw two cards.

it might help us for coping cards if the "intrinsic" cost of the spell is only its mana cost?

then we might better differ from which static ability the additional cost is coming from in case of "linked abilities"?

Hanmac commented 1 year ago

A bit tricky might be a possible refactoring for optional additional costs like Dragon Presence:

Draconic Roar {1}{R}

Instant

As an additional cost to cast this spell, you may reveal a Dragon card from your hand.

Draconic Roar deals 3 damage to target creature. If you revealed a Dragon card or controlled a Dragon as you cast this spell, Draconic Roar deals 3 damage to that creature’s controller.

maybe a way to make RaiseCost Optional there?

tool4ever commented 1 year ago

We especially need optional because there are 7 cards with "cost up to" (while it's ReduceCost it should probably work the same)

github-actions[bot] commented 1 year ago

This issue has not been updated in a while and has now been marked as stale. Stale messages will be auto closed.

github-actions[bot] commented 1 year ago

This issue has not been updated in a while and has now been marked as stale. Stale messages will be auto closed.

tool4ever commented 6 months ago

@Hanmac We also need to think about if we need extra static for this. Technically "cost increase" and "additional cost" are not the same right? But I'm not sure if there's a corner case where it would matter 🤔

Hanmac commented 6 months ago

@Hanmac We also need to think about if we need extra static for this. Technically "cost increase" and "additional cost" are not the same right? But I'm not sure if there's a corner case where it would matter 🤔

i think "cost increase" and "additional cost" should be the same. S:Mode$ RaiseCost should work for them

The corner case that might matter if that additional cost does something intrinsic to the card

Hanmac commented 6 months ago

@tool4ever one thing that might matter if we could refactor this into a RaiseCost?

https://github.com/Card-Forge/forge/blob/2bb068789f85d5983f860ff63c76eac07845ff23/forge-game/src/main/java/forge/game/GameActionUtil.java#L658-L663

As an additional cost to cast creature spells, you may pay any amount of mana. If you do, that creature enters the battlefield with that many additional +1/+1 counters on it.

for Chorus of the Conclave

we might checkout the other Keywords there like Squad too ... but for these i would need to debug more first