Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Oubliette does not return exiled creature with counters on it #1842

Closed
drmDev opened this issue Apr 11, 2016 · 10 comments
Closed

Oubliette does not return exiled creature with counters on it #1842

drmDev opened this issue Apr 11, 2016 · 10 comments
Labels
bug Bugs and errors

Comments

@drmDev
Copy link
Contributor

drmDev commented Apr 11, 2016

Oubliette oracle text:
"When Oubliette enters the battlefield, exile target creature and all Auras attached to it. Note the number and kind of counters that were on that creature.
When Oubliette leaves the battlefield, return that exiled card to the battlefield under its owner's control tapped with the noted number and kind of counters on it. If you do, return the other exiled cards to the battlefield under their owner's control attached to that permanent."

The tooltip text does not reflect the Oracle text either. Looking at the code, all it does is exile a creature and then return the creature if Oubliette leaves the battlefield - so all the parts of Oubliette dealing with exiling Auras attached to it, and returning the creature with the Auras and counters are not doing anything.

Simple test:
Give +1+1 counter to a creature with the spell Battlegrowth
Exile creature with Oubliette
Disenchant the Oubliette
Creature returns without +1+1 counter on it.

@drmDev drmDev added the bug Bugs and errors label Apr 11, 2016
@Marco-Marin
Copy link
Member

Fixed it.
Still going to figure out the counters, but I think I got a solution.
The auras and the tooltip should be fixed tho.

Going for counters now.

@Marco-Marin
Copy link
Member

Fixed. Tho as you'll see, I'm not very comfortable with the solution. Yet, the permanent should still be around while this effect resolves, so the data should still be available. And I do check for null, so no NPE this time (at least not for this reason, lol)
Going for a pull request now.

@drmDev
Copy link
Contributor Author

drmDev commented Apr 11, 2016

Have you gotten to try out the manual testing "cheats"? Or taken a look at
automatic testing? You should be confident this is likely to resolve the
issue just by testing the change
On Apr 11, 2016 1:58 PM, "Marco-Marin" notifications@github.com wrote:

Fixed. Tho as you'll see, I'm not very comfortable with the solution. Yet,
the permanent should still be around while this effect resolves, so the
data should still be available. And I do check for null, so no NPE this
time (at least not for this reason, lol)
Going for a pull request now.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#1842 (comment)

@Marco-Marin
Copy link
Member

I was hoping to get a new card coded as guinea pig to motivate learning about that. Oubliette seemed too urgent (I don't know how long I'm going to take to get to it). Feel free to ignore the request.

@Marco-Marin
Copy link
Member

hmm.. I have no idea how to "cherry pick" the Oubliette one, since the ATQ commit is under scrutiny (that's ok :)). Maybe you can just check it out in my repo and copy it over, if you feel it's urgent enough to call for it? I'm going to try and learn about the testing process.

@drmDev
Copy link
Contributor Author

drmDev commented Apr 11, 2016

It's by no means urgent. It won't be able to be released until whenever the
next version is being pushed out. I understand Oubliette will be a
difficult card to complete, so whenever you have some time to devote to it,
go for it. Nonetheless, please do test your cards and fixes.

On Mon, Apr 11, 2016 at 2:10 PM, Marco-Marin notifications@github.com
wrote:

hmm.. I have no idea how to "cherry pick" the Oubliette one, since the ATQ
commit is under scrutiny (that's ok :)). Maybe you can just check it out in
my repo and copy it over, if you feel its urgent enough to call for it? I'm
going to try and learn about the testing process.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#1842 (comment)

Derek Monturo

@Marco-Marin
Copy link
Member

Good news. Managed to figure out the testing set up. \o/
Furthermore, thanks to the latest post on the bug thread, which mentions a "debugger", figured out I could as well use it :)

Discovered that my suspicion was right in that my code isn't able to get the target of oubliette. It seems to not yet be set, even though the game has already asked for it.
Would you give me a hand here? I'm using getFirstTarget() from Ability, iterating through them all, it does get to the triggered one but don't find a target. I inspected inside modes but couldn't find it there either (maybe didn't look thoroughly?), but I'd guess getFirstTarget() should get a target regardless (specially since it doesn't use modes here, well, you know, more than the default)

Should I get the stack instead to get to it? OH, maybe the "source" argument is the one I'm looking for?

@Marco-Marin
Copy link
Member

Yes. got it. :) Sry to 'bug', now things will roll smoother...

@Marco-Marin
Copy link
Member

I need some help. Exiling works but returning doesn't. I can see it DOES get registered as an Ability in the card but it isn't triggered.

I "copied" the effect from Flickerform and it registered directly on the game object, but I can't do that here because it isn't a "at Phase" trigger but depends on the permanent leaving.

@kevinwshin
Copy link
Contributor

I didn't fix this, but I did just test it as part of another issue, and it seems to work just fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs and errors
Projects
None yet
Development

No branches or pull requests

3 participants