Bug Reports (snapshot builds)
Moderators: timmermac, Blacksmith, KrazyTheFox, Agetian, friarsol, CCGHQ Admins
Re: Bug Reports (snapshot builds)
by Agetian » 29 Dec 2016, 15:32
So far I narrowed this down to the fact that WinSubAbility, LoseSubAbility, HeadsSubAbility and TailsSubAbility additional abilities are apparently not created somewhere in the code, and thus they are returned as "null" later on when the game queries them from SAs. Not sure why this is happening yet though.Marek14 wrote:I played Mana Clash, got heads for opponent and tails for myself, took 1 damage, and then the effect ended. Mana Clash should continue until both players get heads.
Fiery Gambit doesn't work for me (no coin flips appear). I had Krark's Thumb and cast Fiery Gambit vie Eye of the Storm.
EDIT: Next attempt to cast Fiery Gambit worked, except that when I guessed tails and both Krark's Thumb flips were heads, I won the flip anyway.
EDIT2: I just can't get Fiery Gambit to deal any damage, no matter how I win or lose the flips. Also, the effect doesn't stop after I lose the flip, I still can flip more coins, which is wrong.
EDIT: Ok, I figured out what was wrong, it was similar to the Charm API situation where the additional abilities were not filled for the API and that made the relevant abilities fail. I committed a similar solution in r32867, though I'm not sure about a couple things: is there a reason why we aren't just filling in additional abilities by default (for all APIs)? And also, maybe there's a better solution? :/ (not very familiar with this part of code tbh).
- Agetian
- Agetian
- Programmer
- Posts: 3487
- Joined: 14 Mar 2011, 05:58
- Has thanked: 683 times
- Been thanked: 569 times
Re: Bug Reports (snapshot builds)
by Hanmac » 29 Dec 2016, 16:18
@Agetian: i removed all special cases.
i added them because of that card where the spellability tree was broken in a self-loop. and because i did rewrote that card to DONT do that anymore, the special cases are not needed anymore.
i added them because of that card where the spellability tree was broken in a self-loop. and because i did rewrote that card to DONT do that anymore, the special cases are not needed anymore.
Re: Bug Reports (snapshot builds)
by fmartel » 30 Dec 2016, 01:08
Description: [While having Archangel of Thune OTB, played, consecutively, Nomads' Assembly and Deploy to the Front ]
- OutOfMemoryError | Open
- fmartel
- Posts: 281
- Joined: 31 Dec 2013, 19:27
- Location: Québec City
- Has thanked: 8 times
- Been thanked: 4 times
Re: Bug Reports (snapshot builds)
by Agetian » 30 Dec 2016, 05:09
Apparently Dragon Presence no longer does anything useful (if anything at all). Cards like Draconic Roar, Foul-Tongue Invocation, Dragonlord's Prerogative and other cards with Dragon Presence always operate as if there is a Dragon present or revealed, even if there is none at all. :/ Trying to debug this I found out that the condition for Dragon Presence is never actually checked (the relevant code is never called at all, none of my breakpoints at anything Presence-related ever fired). Fix attempted in r32878.
- Agetian
- Agetian
- Agetian
- Programmer
- Posts: 3487
- Joined: 14 Mar 2011, 05:58
- Has thanked: 683 times
- Been thanked: 569 times
Re: Bug Reports (snapshot builds)
by Hanmac » 30 Dec 2016, 05:49
@Agetian: i don't know if that is the right fix.
the check is done in CardTraitBase.meetsCommonRequirements ... i think it would be better to look why that part is not called.
PS: StaticAbility#checkConditions should be rewritten to use meetsCommonRequirements too.
Edit: okay now i see why that is done ... SpellAbilityRestriction & SpellAbilityCondition don't use meetsCommonRequirements .. (that seems to be only used for Replacement and Trigger)
i think some day in the future we might rework that, so that the code isn't at multiple places at once.
the check is done in CardTraitBase.meetsCommonRequirements ... i think it would be better to look why that part is not called.
PS: StaticAbility#checkConditions should be rewritten to use meetsCommonRequirements too.
Edit: okay now i see why that is done ... SpellAbilityRestriction & SpellAbilityCondition don't use meetsCommonRequirements .. (that seems to be only used for Replacement and Trigger)
i think some day in the future we might rework that, so that the code isn't at multiple places at once.
Re: Bug Reports (snapshot builds)
by Agetian » 30 Dec 2016, 06:17
Yeah, sounds like it's a good target for refactoring in the future. Seems like it's working correctly so far though, should we leave it this way for now until one of us or someone else finds an opportunity to refactor this not to duplicate the checks?
- Agetian
- Agetian
- Agetian
- Programmer
- Posts: 3487
- Joined: 14 Mar 2011, 05:58
- Has thanked: 683 times
- Been thanked: 569 times
Re: Bug Reports (snapshot builds)
by friarsol » 30 Dec 2016, 14:02
IIRC Conditions for SpellAbilities existed first. Then Triggers and Replacements were redone about the same time and used lots of the same parts.Hanmac wrote:Edit: okay now i see why that is done ... SpellAbilityRestriction & SpellAbilityCondition don't use meetsCommonRequirements .. (that seems to be only used for Replacement and Trigger)
i think some day in the future we might rework that, so that the code isn't at multiple places at once.
- friarsol
- Global Moderator
- Posts: 7593
- Joined: 15 May 2010, 04:20
- Has thanked: 243 times
- Been thanked: 965 times
Re: Bug Reports (snapshot builds)
by toomadtoplay » 30 Dec 2016, 23:26
hey there, new to the forum's but been using Forge for about a year, its amazing.
I have a Gisa and Geralf commander deck, and somewhere in the last few weeks, their ability as Commander to cast zombie cards from the graveyard no longer works.
I am currently using snapshot 32887. No error or anything comes up, used to be you could click a zombie in the GY and it would prompt the casting cost popup, but now when you click, nothing happens.
Searched the forum for a Gisa and Geralf related bug post but didn't find anything, so hopefully this isn't old news!
Thanks!
I have a Gisa and Geralf commander deck, and somewhere in the last few weeks, their ability as Commander to cast zombie cards from the graveyard no longer works.
I am currently using snapshot 32887. No error or anything comes up, used to be you could click a zombie in the GY and it would prompt the casting cost popup, but now when you click, nothing happens.
Searched the forum for a Gisa and Geralf related bug post but didn't find anything, so hopefully this isn't old news!
Thanks!
- toomadtoplay
- Posts: 4
- Joined: 30 Dec 2016, 23:19
- Has thanked: 1 time
- Been thanked: 0 time
Re: Bug Reports (snapshot builds)
by Agetian » 31 Dec 2016, 06:00
Figured this outtoomadtoplay wrote:hey there, new to the forum's but been using Forge for about a year, its amazing.
I have a Gisa and Geralf commander deck, and somewhere in the last few weeks, their ability as Commander to cast zombie cards from the graveyard no longer works.
I am currently using snapshot 32887. No error or anything comes up, used to be you could click a zombie in the GY and it would prompt the casting cost popup, but now when you click, nothing happens.
Searched the forum for a Gisa and Geralf related bug post but didn't find anything, so hopefully this isn't old news!
Thanks!
Fixed (r32899).EDIT: Hmm, no cards actually use PlayerTurn$ You, so maybe a different thing is actually wrong, will try to update the fix, sec...
EDIT 2: Most likely r32901 is how it was intended to be (if there is a Defined parameter, look in it for who should be the ability activator, if not (e.g. PlayerTurn$ True and no Defined), assume Defined == You. This fixes Gisa and Geralf, but I'm not completely sure if this was the intended way for it to work.
- Agetian
- Agetian
- Programmer
- Posts: 3487
- Joined: 14 Mar 2011, 05:58
- Has thanked: 683 times
- Been thanked: 569 times
Re: Bug Reports (snapshot builds)
by Agetian » 31 Dec 2016, 06:22
Hmm, I'm not sure about Condition... The line around StaticAbility.java:520 is making a call to look for defined players (usually the Defined parameter is passed there, which, if absent, returns "You"), but for some reason it's passing in PlayerTurn instead of Defined there, which results in "True" being processed incorrectly as a definition of a player.Hanmac wrote:Hm that PlayerTurn$ is probably for something different. I think you look for "Condition$ PlayerTurn"
Btw, it looks like the card can be implemented via Condition$ PlayerTurn as well, I don't know if either implementation is preferable though.
Looks like for Gisa and Geralf there will be no difference actually, but the actual difference (for potentially other cards) is that Condition$ PlayerTurn always assumes that it has to be *your* turn, while PlayerTurn$ True with a Defined parameter allows to specify someone else (e.g. "during opponent's turn only" or whatever).
EDIT: Grr, not sure actually
There are only two cards in Forge that use PlayerTurn$ True on a static ability - Gisa and Geralf and Karador, Ghost Chieftain. I'll update both to use Condition$ PlayerTurn instead, which is what everything else seems to be using anyway, and I'll revert my change to StaticAbility for now since God only knows what that can affect (still don't understand why it's looking for the defined players in PlayerTurn though, I don't think that's ever true unless someone actually implements a card (ab)using this feature in the future) :/- Agetian
- Agetian
- Programmer
- Posts: 3487
- Joined: 14 Mar 2011, 05:58
- Has thanked: 683 times
- Been thanked: 569 times
Re: Bug Reports (snapshot builds)
by Hanmac » 31 Dec 2016, 06:39
@Agetian hm yeah but now with your change "Gisa and Geralf" might crash because the Defined parameter is missing.
So that change with "PlayerTurn$ True" might be wrong and the cards should use "Condition$ PlayerTurn" instead.
So that change with "PlayerTurn$ True" might be wrong and the cards should use "Condition$ PlayerTurn" instead.
Re: Bug Reports (snapshot builds)
by Agetian » 31 Dec 2016, 06:43
The getDefinedPlayers function is actually testing if Defined is null and then assumes that it defaults to "You" in that case, so it shouldn't crash, but anyways, I updated it to use Condition$ PlayerTurn instead and for now let the PlayerTurn parameter operate in its own weird way (apparently it's possible to code things such that PlayerTurn points to an opponent or something), I guess let it be that way for now since if it's changed to look in Defined, then it may potentially interfere with other stuff that may already use Defined for other purposes, perhaps...Hanmac wrote:@Agetian hm yeah but now with your change "Gisa and Geralf" might crash because the Defined parameter is missing.
So that change with "PlayerTurn$ True" might be wrong and the cards should use "Condition$ PlayerTurn" instead.
- Agetian
- Agetian
- Programmer
- Posts: 3487
- Joined: 14 Mar 2011, 05:58
- Has thanked: 683 times
- Been thanked: 569 times
Re: Bug Reports (snapshot builds)
by toomadtoplay » 31 Dec 2016, 14:46
Gisa and Geralf is confirmed casting from GY again, no crash! Thanks for such a quick reply! 
- toomadtoplay
- Posts: 4
- Joined: 30 Dec 2016, 23:19
- Has thanked: 1 time
- Been thanked: 0 time
Who is online
Users browsing this forum: No registered users and 19 guests