Page 18 of 32

Re: Developing Bugs

PostPosted: 15 May 2013, 14:36
by Max mtg
RedDeckWins wrote:Sorry, to clarify, there is no exception:

The bug is that the effect is put into the command zone of the wrong player when it resolves. There is no exception. The pasted callstack just a little insight into how/where the activating player is being set incorrectly before the EffectEffect resolves.
Ah... that's easy to explain.

The spellability instances are reused, that is there is only one per ability per card. AI tends to set activating player to itself when it... 'thinks' - I don't know exactly what it does, either evaluates threat from abilities on stack or their costs. But this often involes assigning "activating player" with AI's player.

To restore the correct player as the ability resolves, it's store in SpellAbilityStackInstance. Yesterday being unaware of the above described effect I deleted that field - that lead to a wide variety of bugs. Today I had to revert that change - r21483. So this should have solved issues with wrong player that recieves an effect.

Re: Developing Bugs

PostPosted: 15 May 2013, 14:46
by friarsol
Max mtg wrote:The spellability instances are reused, that is there is only one per ability per card. AI tends to set activating player to itself when it... 'thinks' - I don't know exactly what it does, either evaluates threat from abilities on stack or their costs. But this often involes assigning "activating player" with AI's player.
Just for some additional clarification, the AI is determining if it can use that SA and then what would it do if it's legal to use it. To do both of these, it needs to set the ActivatingPlayer to pass through SARestrictions and canPlayAI().

Re: Developing Bugs

PostPosted: 18 May 2013, 03:21
by RedDeckWins
The diamond operator (java 7 only) is in ManaCostBeingPaid. The pom file still has the source and target at 1.6. One of these should change to match the other I think.

Re: Developing Bugs

PostPosted: 18 May 2013, 05:27
by Max mtg
RedDeckWins wrote:The diamond operator (java 7 only) is in ManaCostBeingPaid. The pom file still has the source and target at 1.6. One of these should change to match the other I think.
Yes, I have switched the local system to java 7 (including that settings in pom.xml) but still compile for 1.6 platform. So the compiler didn't consider the empty braces a problem... thus it got to SVN.

If a mistake is obvious, like this one, you may correct it and commit

Re: Developing Bugs

PostPosted: 20 May 2013, 11:21
by moomarc
I was testing out Boros Battleshaper and it works perfectly except for this exception occasionally that doesn't necessarily always happen under the same circumstances.

RuntimeException | Open
Code: Select all
Forge Version:    SVN
Operating System: Windows 7 6.1 x86
Java Version:     1.6.0_35 Sun Microsystems Inc.

java.lang.RuntimeException: Trying to unlock input which is not locked! Do check when your threads terminate!
   at forge.control.input.InputQueue.unlock(InputQueue.java:207)
   at forge.FThreads$1.run(FThreads.java:116)
   at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(Unknown Source)
   at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
   at java.lang.Thread.run(Unknown Source)
Both Pump effects have TargetsMin$0 and TargetsMax$1, so I don't know if that is maybe causing issues with the input somehow. Start a game, drop in a Boros Battleshaper via dev console and proceed to Begin Combat. Sometimes it will work smoothly and other times it crashes with the above exception after targeting (or not targeting).

I'm committing the card for now seeing as it works aside from this occasional error. If it's not fixed before the next release though we should probably remove it.

Re: Developing Bugs

PostPosted: 20 May 2013, 12:12
by swordshine
moomarc wrote:I was testing out Boros Battleshaper and it works perfectly except for this exception occasionally that doesn't necessarily always happen under the same circumstances.

RuntimeException | Open
Code: Select all
Forge Version:    SVN
Operating System: Windows 7 6.1 x86
Java Version:     1.6.0_35 Sun Microsystems Inc.

java.lang.RuntimeException: Trying to unlock input which is not locked! Do check when your threads terminate!
   at forge.control.input.InputQueue.unlock(InputQueue.java:207)
   at forge.FThreads$1.run(FThreads.java:116)
   at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(Unknown Source)
   at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
   at java.lang.Thread.run(Unknown Source)
Both Pump effects have TargetsMin$0 and TargetsMax$1, so I don't know if that is maybe causing issues with the input somehow. Start a game, drop in a Boros Battleshaper via dev console and proceed to Begin Combat. Sometimes it will work smoothly and other times it crashes with the above exception after targeting (or not targeting).

I'm committing the card for now seeing as it works aside from this occasional error. If it's not fixed before the next release though we should probably remove it.
This's why I did not add Boros Battleshaper :D I remember this card is working for AI in my test.

Re: Developing Bugs

PostPosted: 20 May 2013, 12:53
by moomarc
swordshine wrote:This's why I did not add Boros Battleshaper :D I remember this card is working for AI in my test.
That's what I get for being too busy with work 8-[ Did you ever bring up the issue on the forum here so that one of the guys familiar with inputs could see if they could track down the cause?

Re: Developing Bugs

PostPosted: 20 May 2013, 12:57
by swordshine
moomarc wrote:
swordshine wrote:This's why I did not add Boros Battleshaper :D I remember this card is working for AI in my test.
That's what I get for being too busy with work 8-[ Did you ever bring up the issue on the forum here so that one of the guys familiar with inputs could see if they could track down the cause?
I posted the exception in the DGM spoiler. :D http://www.slightlymagic.net/forum/viewtopic.php?f=52&t=10020&start=45#p116149

Re: Developing Bugs

PostPosted: 20 May 2013, 13:17
by moomarc
swordshine wrote:
moomarc wrote:
swordshine wrote:This's why I did not add Boros Battleshaper :D I remember this card is working for AI in my test.
That's what I get for being too busy with work 8-[ Did you ever bring up the issue on the forum here so that one of the guys familiar with inputs could see if they could track down the cause?
I posted the exception in the DGM spoiler. :D http://www.slightlymagic.net/forum/viewtopic.php?f=52&t=10020&start=45#p116149
Urgh #-o Missed tht somehow. I'll remove it from svn before the next build.

Re: Developing Bugs

PostPosted: 20 May 2013, 14:04
by Max mtg
Repro for Boros Battleshaper:
1 Get one at you disposal.
2 Have AI control any creature able to attack you (no sickness)
3 On AI's turn target its creature with Boros Battleshaper 1st ability.

Nature of bug: race condition.
AI somehow got priority while human player is choosing targets for Boros Battleshaper's ability


As the input for first part of target selection is gone, the code call update on input queue, the latter is empty and for that reason it calls AI.getDefaultInput

Re: Developing Bugs

PostPosted: 20 May 2013, 15:08
by Max mtg
Fixed! r21558

Re: Developing Bugs

PostPosted: 20 May 2013, 16:22
by moomarc
Thanks Max! That's one closer to our first 100% complete block.

Re: Developing Bugs

PostPosted: 22 May 2013, 03:34
by RedDeckWins
This is my first attempt a significant change to the rules engine. Someone please look at it. The patch is to make the "cast for free" portion of suspend happen via a trigger. This will fix a bug with a few cards such as Fungal Behemoth, as well as make the trigger stiflable like it should be.

https://dl.dropboxusercontent.com/u/129 ... gers.patch

Re: Developing Bugs

PostPosted: 22 May 2013, 04:11
by Max mtg
I didn't see anything criminal in it. (though I am not a big expert in those triggers)
Feel free to apply, we've plenty of time to fix any bugs

Re: Developing Bugs

PostPosted: 22 May 2013, 09:25
by Sloth
RedDeckWins wrote:This is my first attempt a significant change to the rules engine. Someone please look at it. The patch is to make the "cast for free" portion of suspend happen via a trigger. This will fix a bug with a few cards such as Fungal Behemoth, as well as make the trigger stiflable like it should be.

https://dl.dropboxusercontent.com/u/129 ... gers.patch
This is indeed a necessary change. Go ahead.