Page 395 of 487

Re: Bug Reports (snapshot builds)

PostPosted: 24 Dec 2016, 07:11
by Hanmac
@Agetian i think it's good that way. Might me check if the SVar need to be done different on other cards too. (The ones which has this SneakAttack thing with adding the SVar)

Re: Bug Reports (snapshot builds)

PostPosted: 24 Dec 2016, 07:12
by Agetian
Okie doke, thanks for confirming! :)

- Agetian

Re: Bug Reports (snapshot builds)

PostPosted: 24 Dec 2016, 08:06
by Agetian
Chandra, Torch of Defiance emblem does not forget its target. For example, the first time you cast a spell, you pick a target to deal 5 damage to; for example, you pick a player (your opponent). Then, each time you cast subsequent spells (even in the next turns), that particular player is automatically picked as a target instead of letting the player pick a different target.

EDIT: Looks like the problem is that if the ability is a WrappedAbility (like for the emblem), then targets are not cleared on the WrappedAbility at HumanPlaySpellAbility::setupTargets. This can be solved by adding the clear code for WrappedAbility targets, like this:

Code: Select all
                clearTargets(currentAbility);
                if (currentAbility instanceof WrappedAbility) {
                    clearTargets(((WrappedAbility) currentAbility).getWrappedAbility());
                }
This definitely makes the game correctly clear the target on an emblem, but can this be done safely without the risk of breakage?
- Agetian

Re: Bug Reports (snapshot builds)

PostPosted: 24 Dec 2016, 08:57
by Hanmac
@Agetian: hm that might have something to do that the SpellAbility Objects are not created anymore each time a Trigger is made (only copied otherwise the Triggered Objects are wrong, i tried to fix that too but failed yet)

so i think the best way would be that the targets are either reset to nothing before a spell is added to stack, or when the spellability leaves the stack.

HMM there might be the bug that when a spellability is copied, the Target object is not copied too. (only reference is copied) imo that might need to be fixed too.

Re: Bug Reports (snapshot builds)

PostPosted: 24 Dec 2016, 09:02
by Agetian
Umm they are cleared, as far as I can tell (clearTargets is called on currentAbility whenever the "play spell ability" code is called), but the wrapped ability linked to the SA is not affected by the code that clears it.

I'll see if I can find the code where the SpellAbility leaves the stack is...

- Agetian

Re: Bug Reports (snapshot builds)

PostPosted: 24 Dec 2016, 09:07
by Hanmac
hey because clearTargets just calls resetTargets, why not have WrappedAbility do the thing?
Code: Select all
    @Override
    public void resetTargets() {
        sa.resetTargets();
    }
PS: hm might not be enough with DividedAsYouChoose :/ but its a beginning

Re: Bug Reports (snapshot builds)

PostPosted: 24 Dec 2016, 09:17
by Agetian
Hanmac wrote:hey because clearTargets just calls resetTargets, why not have WrappedAbility do the thing?
Code: Select all
    @Override
    public void resetTargets() {
        sa.resetTargets();
    }
PS: hm might not be enough with DividedAsYouChoose :/ but its a beginning
This passes the initial test (Chandra emblem starts working correctly). What exactly is the special case with DividedAsYouChoose btw?
(I see that clearTargets does an extra calculation related to that; wouldn't that be covered if the call to clearTargets for the WrappedAbility is made (like I suggested in the post above)?

- Agetian

Re: Bug Reports (snapshot builds)

PostPosted: 24 Dec 2016, 09:25
by Agetian
Another idea is to implement ClearTargets like something like this:

Code: Select all
    public final void clearTargets(final SpellAbility ability) {
        final TargetRestrictions tg = ability.getTargetRestrictions();
        if (tg != null) {
            ability.resetTargets();
            tg.calculateStillToDivide(ability.getParam("DividedAsYouChoose"), ability.getHostCard(), ability);
        }
        if (ability instanceof WrappedAbility) {
            final SpellAbility wrapSa = ((WrappedAbility) ability).getWrappedAbility();
            final TargetRestrictions wrapTg = wrapSa.getTargetRestrictions();
            if (wrapTg != null) {
                wrapSa.resetTargets();
                wrapTg.calculateStillToDivide(wrapSa.getParam("DividedAsYouChoose"), wrapSa.getHostCard(), wrapSa);
            }
        }
    }
This both clears the WrappedAbility targets and also makes the same calculation for DividedAsYouChoose as the SpellAbility itself. Not sure if this would work though. :/

- Agetian

Re: Bug Reports (snapshot builds)

PostPosted: 24 Dec 2016, 09:27
by Hanmac
i would probably call extend clearTargets with doing something like that:
Code: Select all
ability = ability instanceof WrappedAbility) ? ((WrappedAbility) currentAbility).getWrappedAbility() : ability;
hm moment WrappedAbility does overwrite getParam, so it might not even needed.

i say, it might be enough if you add WrappedAbility#resetTargets as i said above.

Re: Bug Reports (snapshot builds)

PostPosted: 24 Dec 2016, 09:30
by Agetian
Okie doke, let's try that! :) I'll commit then. Thanks a lot for advice!

- Agetian

Re: Bug Reports (snapshot builds)

PostPosted: 24 Dec 2016, 09:45
by Marek14
When processing the delayed trigger of Otherworldly Journey, I get splice dialogue.

Re: Bug Reports (snapshot builds)

PostPosted: 24 Dec 2016, 09:55
by Hanmac
Marek14 wrote:When processing the delayed trigger of Otherworldly Journey, I get splice dialogue.
Huch good to know, I might need to add a "isSpell" check to the places where I addthe splices.

PS: do you guys like what I did with the Splice Dialog?

Re: Bug Reports (snapshot builds)

PostPosted: 24 Dec 2016, 10:35
by Agetian
Hanmac wrote:
Marek14 wrote:When processing the delayed trigger of Otherworldly Journey, I get splice dialogue.
Huch good to know, I might need to add a "isSpell" check to the places where I addthe splices.

PS: do you guys like what I did with the Splice Dialog?
Yes, I find it more intuitive than the way it was before for sure! :)

- Agetian

Re: Bug Reports (snapshot builds)

PostPosted: 24 Dec 2016, 11:52
by Agetian
The AI is using Crossroads Consecrator to buff my (!) attacking Human creatures while not really buffing its own. I thought it'd be a matter of maybe having the IsCurse flag wrongly set, but that wasn't it. :/

EDIT: I think I kinda figured this one out (r32788) but there's still something that is kind of bothering me: many other cards, e.g. Ulrich's Kindred, specify the valid target as just "Werewolf+attacking" instead of "Creature.Werewolf+attacking" and that works perfectly fine. However, when I tried specifying the target on Crossroads Consecrator as "Human+attacking", the AI just stopped using the pump ability altogether. :/

- Agetian

Re: Bug Reports (snapshot builds)

PostPosted: 26 Dec 2016, 06:12
by tjtillman
r32759

CPU played Eliminate the Competition targeting and destroying 4 of my creatures but didn't sacrifice any of its 4 creatures.