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

AmassEffect: use Effect in Command Zone for Type Change #6797

Merged
merged 2 commits into from
Jan 20, 2025
Merged

Conversation

Hanmac
Copy link
Contributor

@Hanmac Hanmac commented Jan 19, 2025

Part of #5642 to reduce the amount of addChangedCardTypes

@tool4ever
Copy link
Contributor

I'm not sure there exists a dependency where not applying this one first (or rather always like before) would be required.
So for a short while until we support them this might behave a bit worse?
I guess ideally it would just call AnimateEffectBase.doAnimate but that's a pretty big refactor 😅

@Hanmac
Copy link
Contributor Author

Hanmac commented Jan 20, 2025

@tool4ever effects that removes all creature types except chosen ones for example should apply first, and then the add Creature Type after

I might look into it refactoring AnimateBase later

But first I want to remove the use of addChangedCardTypes

@tool4ever
Copy link
Contributor

If you cast something like Mercurial Transformation after Amass it should just be applied by timestamp order and overwrite everything.

tool4ever
tool4ever previously approved these changes Jan 20, 2025
@Hanmac
Copy link
Contributor Author

Hanmac commented Jan 20, 2025

If you cast something like Mercurial Transformation after Amass it should just be applied by timestamp order and overwrite everything.

No, i mean using Conspiracy to transform some non Army into an Army. And then use the Amass Effect to turn it onto a Zombie/Orc/Sliver.

That's why this doesn't work:

I'm not sure there exists a dependency where not applying this one first (or rather always like before) would be required.

@tool4ever
Copy link
Contributor

But that works already, this doesn't lead to a dependency by itself and you will have an Army Zombie

@Hanmac
Copy link
Contributor Author

Hanmac commented Jan 20, 2025

But that works already, this doesn't lead to a dependency by itself and you will have an Army Zombie

Yes, but i don't want it to be broken later by applying this one first

@Hanmac
Copy link
Contributor Author

Hanmac commented Jan 20, 2025

@tool4ever should we apply #6792 first, so I can hide the Amass Effect?

@Hanmac Hanmac merged commit 3f605aa into master Jan 20, 2025
6 checks passed
@Hanmac Hanmac deleted the amassWithEffect branch January 20, 2025 12:05
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.

2 participants