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

Planechase inherent effects #4408

Merged
merged 5 commits into from
Dec 31, 2023
Merged

Conversation

Klisz
Copy link
Contributor

@Klisz Klisz commented Dec 19, 2023

Closes #4262, by moving the planeswalking ability and the static chaos trigger to an effect carried with the player in a planechase game. Technically the special action to roll the planar die should be part of it too, but then you either have to make the "Planechase Effect" card visible in the UI so that human players can actually click it to roll, or rework how the UI relates to planechase entirely

Draft to close Card-Forge#4262, by moving the planeswalking ability and the static chaos trigger to an effect carried with the player in a planechase game. Currently this effect is visible in the command zone much like the monarch or initiative (but with no image), which is something that needs fixing; I'd have it be a DetachedCardEffect but there's no linked card for it like there are for commander and companion effects (the active plane [or the face-up plane with the earliest timestamp, I suppose] would be a logical choice for the getCardForUi, but that would require updating it as the plane changes, and in any case because there's a separate effect for each player, the fact that the owner of a DetachedCardEffect is set as the owner of the linked card also adds some wrinkles. In any case, I've set the effects to be created in startGame after initPlane is run, rather than in initVariantZones as is the case for the commander and companion DetachedCardEffects, in anticipation of hopefully in the future making the effect be linked to an active plane).
@Klisz Klisz marked this pull request as ready for review December 20, 2023 15:48
@Klisz Klisz changed the title Planechase effects draft Planechase inherent effects Dec 20, 2023
@Northmoc
Copy link
Contributor

Nice solution. Definitely seems closer to the CR to me.

Seems like it would be easy enough to just put the new effect in the command zone so players could interact with it? Similar to things like the City's Blessing.

The card image should probably be some kind of image of a planar die or multiple planar dice
image

@Klisz
Copy link
Contributor Author

Klisz commented Dec 20, 2023

Seems like it would be easy enough to just put the new effect in the command zone so players could interact with it? Similar to things like the City's Blessing.

Indeed, that's easier than what I've already done - the only issue I had was the lack of an image, so I altered it to have the effect's getCardForUi method overridden. I do like the idea of using an image of one or more planar dice, though, and alternatively the thought just occurred to me of possibly using the cardbackused on physical planar cards.

f we go either of these routes and move the planar dice special action from the planes to the effect, it should also be noted that RollPlanarDiceAi will need to be altered (since currently it uses the AIRollPlanarDiceParams SVar of the SpellAbility's host card - although, as it stands, none of the MOC or WHO planes have such an SVar anyway [I've been meaning to get around to making a MR to just add reasonable params to each of them, but as of yet the planar dice AI params are only found on the pre-2023 planes, meaning the AI will just never roll the planar die when on a 2023 plane]).

@Klisz Klisz marked this pull request as draft December 22, 2023 17:06
@Klisz
Copy link
Contributor Author

Klisz commented Dec 22, 2023

Moved the planar die roll to the effect, and made an attempt to update RollPlanarDiceAi, but there's an issue where the AI is still never activating the ability - I even tested changing RollPlanarDiceAi's canPlayAI to just straight-up return true; and it still never did, which leads me to believe that something is preventing it from even getting that far.

@Klisz
Copy link
Contributor Author

Klisz commented Dec 22, 2023

Never mind, that issue was just because I didn't realize that SpellAbility didn't have a getGame method of its own

@Klisz Klisz marked this pull request as ready for review December 22, 2023 17:32
Copy link
Contributor

@tool4ever tool4ever left a comment

Choose a reason for hiding this comment

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

LGTM

@Agetian Agetian merged commit c02f7c6 into Card-Forge:master Dec 31, 2023
2 checks passed
Northmoc pushed a commit to Northmoc/forge that referenced this pull request Jan 8, 2024
* Planechase effects draft

Draft to close Card-Forge#4262, by moving the planeswalking ability and the static chaos trigger to an effect carried with the player in a planechase game. Currently this effect is visible in the command zone much like the monarch or initiative (but with no image), which is something that needs fixing; I'd have it be a DetachedCardEffect but there's no linked card for it like there are for commander and companion effects (the active plane [or the face-up plane with the earliest timestamp, I suppose] would be a logical choice for the getCardForUi, but that would require updating it as the plane changes, and in any case because there's a separate effect for each player, the fact that the owner of a DetachedCardEffect is set as the owner of the linked card also adds some wrinkles. In any case, I've set the effects to be created in startGame after initPlane is run, rather than in initVariantZones as is the case for the commander and companion DetachedCardEffects, in anticipation of hopefully in the future making the effect be linked to an active plane).

* Update Player.java

* Move planar dice special action to effect

* Update ComputerUtilAbility.java

* Move die roll chaos to PlanarDice.java
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.

Chaos and planeswalk abilities trigger several times if on multiple planes
5 participants