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

Global DamageHistory #622

Merged
merged 25 commits into from Jul 4, 2022
Merged

Global DamageHistory #622

merged 25 commits into from Jul 4, 2022

Conversation

tool4ever
Copy link
Contributor

@tool4ever tool4ever commented Jun 5, 2022

Small diagram to show the new structure:

image

a single call in GameAction.dealDamage creates all the data
because we keep a DamageHistory Collection in Game they're already grouped by source so no need to manually merge with timestamps

@tool4ever tool4ever marked this pull request as draft June 5, 2022 18:09
@Hanmac
Copy link
Contributor

Hanmac commented Jun 6, 2022

Check out the thing with Gnoll War Band https://www.youtube.com/watch?v=hvha1anEVag

some cards check if players where dealt damage this turn (even if they left the game)
some cards check if players currently in the game where dealt damage this turn (that didn't left the game)

so a global history alone might not be the right thing?

@tool4ever
Copy link
Contributor Author

Yea that's not too difficult to do now

Gonna add it

@tool4ever
Copy link
Contributor Author

I realized some corner cases need LKI of target too:

Creature A equipped with Thirsting Axe deals combat damage to B, but if B then gets turned into non-creature before EOT, the trigger would still sacrifice A

@tool4ever tool4ever marked this pull request as ready for review June 11, 2022 13:05
@tool4ever tool4ever requested a review from Northmoc June 11, 2022 14:15
@Northmoc
Copy link
Contributor

Exciting! Will this help with Zangief?

@tool4ever
Copy link
Contributor Author

Exciting! Will this help with Zangief?

Hmn, not that much I'm afraid. It still has the problem of the trigger condition timing with combat damage afair.
Do you still have that branch in your fork?

@Northmoc
Copy link
Contributor

Exciting! Will this help with Zangief?

Hmn, not that much I'm afraid. It still has the problem of the trigger condition timing with combat damage afair. Do you still have that branch in your fork?

Yes, I still have it on my local drive. I don't think it's on my GitHub fork.

Northmoc
Northmoc previously approved these changes Jul 3, 2022
Copy link
Contributor

@Northmoc Northmoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks for the big refactor!

@tool4ever tool4ever merged commit 3d634b6 into Card-Forge:master Jul 4, 2022
@tool4ever tool4ever deleted the damagehistory branch July 5, 2022 10:48
@Northmoc
Copy link
Contributor

Northmoc commented Jul 5, 2022

🥳

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

Successfully merging this pull request may close these issues.

Gnoll War Band Dragon Cultist Indulge // Excess
3 participants