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

secret deduction via game rules #1505

Closed
wants to merge 4 commits into from

Conversation

joshtab
Copy link

@joshtab joshtab commented Sep 21, 2015

This pull request is NOT ready in it's current state, but I'd like some feedback before I spend more time on it.

This code starts an effort to use process of elimination to deduce available secrets by looking at game events. The only game event I'm hooking into right now is the "opponent attacked" event.

An example:

before
after

Is this a feature you want? I did a little clean up of the secrets code, let me know if this looks like it's going in the right direction.

@joshtab
Copy link
Author

joshtab commented Sep 21, 2015

One thing that confused me: the desired behavior surrounding "stolen" secrets, and why you'd ever need to display secrets from multiple classes. (e.g. https://github.com/Epix37/Hearthstone-Deck-Tracker/blob/c052f196c1365e305048e668d5cba89b55fff780/Hearthstone%20Deck%20Tracker/Hearthstone/Secrets/OpponentSecrets.cs#L22)

Is it just for Kezan Mystic? Seems like you would know exactly what secret they had.

@azeier
Copy link
Member

azeier commented Sep 21, 2015

I would love to see that added, absolutely!

You can get secrets from other classes for example via Nefarian or Spellslinger.

@joshtab joshtab changed the title proof of concept: secret deduction via game rules secret deduction via game rules Sep 22, 2015
@joshtab
Copy link
Author

joshtab commented Sep 22, 2015

This is ready to be looked at.

@azeier
Copy link
Member

azeier commented Sep 22, 2015

That was quick! I will have a closer look later, very excited :).

What I see right now though: I don't think removing the ability to manually grey out secrets by clicking is a good idea. You might either not want to enable the auto-greyout or it could mess it for some reason and this then leaves you with no way to correct it.

@azeier
Copy link
Member

azeier commented Sep 25, 2015

I started writing unit tests for this, first thing I noticed is that there is no distinction between a hero attacking and a minion attacking. Freezing trap for example only triggers if it's a minion attacking.

Something I have not tested yet but have no idea what exactly happens:

  • Opponent played has Bear Trap and Misdirection.
  • Player hero attacks opponent hero - what happens?

I assume the opponent hero is set as DEFENDING, Misdirection triggeres and something else is then set as DEFENDING. In that case both traps are set as "not possible", but since the Opponent hero was never really attacked, Bear Trap was not actually tested for. (Is this the real behaviour? I am not even sure...)

The same thing should happen with e.g. Freezing and Explosive.

Also, order of play might matter in some cases...

@azeier
Copy link
Member

azeier commented Sep 25, 2015

Pushed this to branch feature-SecretDecuction. Please submit any new changes as a PR to this.

What I did so far:

  • Implemented HandlePlayerAttack in a way that considers attacker and defender (have not tested this at all yet but should be fine)
  • Implemented Test for all "single secret" HandlePlayerAttack scenarios.

@azeier
Copy link
Member

azeier commented Sep 25, 2015

@dantedog
Copy link

No idea what the actual blizzard policy on this is, but before pushing this out, I would make sure that this is ok for a 3rd party app. I can't think of anything deck tracker does that can't be done via pen/paper. This would possibly cross that line.

That said, this seems like an awesome feature.

@Eldenroot
Copy link

it is OK :)

@joshtab
Copy link
Author

joshtab commented Oct 3, 2015

Closing this for the other thread #1522

@joshtab joshtab closed this Oct 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants