-
Notifications
You must be signed in to change notification settings - Fork 591
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
Timestamp rework #117
Timestamp rework #117
Conversation
forge-game/src/main/java/forge/game/ability/effects/DamageEachEffect.java
Outdated
Show resolved
Hide resolved
forge-game/src/main/java/forge/game/ability/effects/CountersMultiplyEffect.java
Outdated
Show resolved
Hide resolved
d32d34c
to
f150f13
Compare
Had the AI play around 50 games on this branch, seems stable so far If we get the old issues back I think there should be some with timestamp stuff that might get solved Let me know if I should test for anything else |
41c9196
to
b33d0c7
Compare
I need rules check if that is true or not. |
probably need to check for cards that got released after i started working on this for their Triggered values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That must have been painful to rebase 😓
Still seems stable
38da1f2
to
369a5d5
Compare
This PR has not been updated in a while nad has been marked on stale. Stale PRs will be auto closed |
3cc5eb4
to
7e62540
Compare
7e62540
to
851533b
Compare
Big rebase, hopefully for the last time |
851533b
to
9720069
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imo it's big enough and should contain at least the most common effects
I'll test drive it one more time and then probably merge to see if we missed something?
@tool4ever one of my goals is to remove the "LKICopy" part again, once all the Effects have been updated to check their game states (and if allowed, fall back to LKI) |
Yea that's the dream |
@tool4ever yeah, after this MR we should look at the Discard Trigger and Replacements |
7cf2201
to
6e0112f
Compare
Draft for now, i need to update more Effects, so that the WrappedAbility Check can be removed
The Main reason for this MR, Split Timestamp into:
Also