Page 1 of 1

Stuck trying to fix Thrumming Stone

PostPosted: 28 Jul 2015, 10:33
by klayhamn
On my last pull-request I added the card Thrumming Stone and the "Ripple" effect.
However, I discovered later that although my implementation works fine, it does not behave EXACTLY according to the rules.

In my old implementation, Thrumming Stone HAS (itself) a triggered ability that triggers when spells are cast, and this ability causes a ripple effect (which targets the spell that was cast).

However, according to the rules and the text of the card, Thrumming Stone's effect should be a STATIC CONTINUOUS EFFECT that applies to all spells that are cast, and GIVES them a Triggered Ability (that triggers as they are cast) which is Ripple.

I can't think right now of a scenario where this distinction would matter, but I want to adher to the rules rather than do a workaround.

So, to fix this, I did the following:

1. Created a "RippleAbility" (triggered ability) to wrap the "RippleEffect"
2. Made the Thrumming Stone have a static ability (instead of a triggered ability) which has a continuous effect (rather than a one-shot effect) that adds the RippleAbility to every (non-copy) spell on the stack. I used the example of Chief Engineer to do this, since it is essentially the same sort of behavior except that it gives spells "Convoke" instead of "Ripple".


So -
it works when a spell is cast from the hand,
however - it does not work correctly when a spell is revealed and cast (as part of the Ripple effect) from the library.

I've been trying to debug this for several hours and have yet come to any useful conclusion on why this is happening...

What I'm basically seeing is that ALL the trigger-checks for the RippleAbility always have the original card that was cast from the hand as the source of the ability, and this souce is never the card cast from the library.
However, it does seem like the triggered ability is added correctly (by the Thrumming Stone continuous effect) to the spell on the stack that was cast from the library - so I'm unsure why this is not working.

My suspicion is that it might be an issue of timing (i.e. the trigger being added after the point in time where it would have been triggered and checked), but I couldn't manage to figure out if that's the case or not - and on the face of it, it seemed like this isn't the case.

I would greatly appreciate if anyone could take a look at my code and maybe suggest a solution :D

My fork can be found in Github at:
https://.../klayhamn/mage

Thanks

Re: Stuck trying to fix Thrumming Stone

PostPosted: 28 Jul 2015, 12:09
by LoneFox
Code: Select all
public ThrummingStoneGainAbilitySpellsEffect(final ThrummingStoneGainAbilitySpellsEffect effect) {
        super(effect);
        this.ability = effect.ability;
        this.filter = effect.filter;
    }
Should be effect.ability.copy() and effect.filter.copy() .
This kind of bug tends to cause really strange misbehavior, and may be the source of your problem.

Edit: A working link to the repository, in case someone else wants to take a look. The one in the OP is broken enough to make my browser dump core...

Re: Stuck trying to fix Thrumming Stone

PostPosted: 28 Jul 2015, 12:17
by klayhamn
Thanks I'm gonna try it now

Regarding the link --- it simply won't let me post any links...

Re: Stuck trying to fix Thrumming Stone

PostPosted: 28 Jul 2015, 14:44
by klayhamn
Turns out this wasn't the cause - LevelX found it.
The problem was found in the way that "addOtherAbility(...)" was implemented in the GameState.
It's actually a very similar problem, but it just happened elsewhere (where the Ripple ability is added to the spell) and not upon the continuous effect of ThrummingStone (since this effect doesn't get copied under the normal course of events, as far as I understand)

But still you're right that this is a potential bug (might arise if something copies the Thrumming Stone, perhaps?) so I added your suggested fix

Thanks!

Re: Stuck trying to fix Thrumming Stone

PostPosted: 28 Jul 2015, 19:49
by LevelX
The way objects and their included objects are copied is mainly important for rollbacks or AI simulation.
If there are objects included that can change their values while the game goes on it's needed to copy the objects. Otherwise the rollback won't reset to the previous value or the AI simulation changes values of objects of the main game leading to strange behaviour. Often this is a problem when a List or Set object e.g. from a watcher object is not copied correctly.

As long as the object stays permanent unchanged (e.g. an effect with an filter that doesn't change during the game) it should be no problem to only copy the reference.

So according your copy method
Code: Select all
  public ThrummingStoneGainAbilitySpellsEffect(final ThrummingStoneGainAbilitySpellsEffect effect) {
    super(effect);
    this.ability = effect.ability.copy();
    this.filter = effect.filter.copy();
  }
the ability and also the filter does not change the state during the game and it should also work to only copy the reference.

Code: Select all
  public ThrummingStoneGainAbilitySpellsEffect(final ThrummingStoneGainAbilitySpellsEffect effect) {
    super(effect);
    this.ability = effect.ability;
    this.filter = effect.filter;
 }
More insights are very welcome.