Standardize accessing X values

[ssk97]

This PR does two things:

1. Replace most Costs.getX() calls with Costs Tags system

This should not be a functional change, since the costs tag system should behave identically to the current system of searching through costs for non-permanents. However, making almost everything use the Costs Tags system for consistency seems like a good idea.

There are still two uses of getX() from places that don't have easy access to the Game object, so I can't remove that code entirely.

2. Combines ManacostVariableValue's three enum values.

They already used the same logic with the Costs Tags system rework, this makes that official by replacing all of them with instance. This one definitely has no functional changes at all, as the exact same code will be executed.

1st commit: Mostly a find/replace regex, but also Disrupting Shoal added a "Game" parameter to the function to make it work
2nd commit: Manually grab the remaining uses that the regex missed
3rd commit: Mostly just rename, also combined one "if" statement that now had both branches identical.

[JayDi85]

There are still two uses of getX() from places that don't have easy access to the Game object, so I can't remove that code entirely.

1. Write more details about that problem (what miss);
2. Are you talking about removing getX() method at all (also see code comment below about other x methods)?

[JayDi85]

1. What about containsX? Why not replaced?

2. There are many other methods with X value. Explain: a. is it same/compatible with coststag? b. Is it require refactor/replace too?

[ssk97]

containsX should probably be removed, yeah. I'll do that in a bit, drafting it for now until I do so.

setX is still necessary: the remaining uses of getX need it to function correctly, and there are some card implementations that access the X value in other ways that this PR doesn't cover (such as implementing their own search for the cost). I would like to convert those eventually, but I figured it would be better to adjust these relatively easy ones now with a find/replace and get to the others later. All Costs.getX() calls used basically the same pattern, which made it easy and safe to do a mass replacement like this.

[JayDi85]

and there are some card implementations that access the X value in other ways that this PR doesn't cover

What you mean by "other ways"? Write examples here. If tags return wrong values then something wrong and must be improved.

[ssk97]

Tags would provide the right values, just there's sometimes complex logic involved. Example: ExplosiveSingularityCostReductionEffect

"real cast" here could use the tag system, but I couldn't easily get ones like this with a simple find/replace regex and there's a bunch of cards that use a similar style of code where they do the loop through the costs to find the variable one.

[JayDi85]

It's uses getCosts, not getX here. I still don't see examples/reason why you keep that method (getX) in current PR

[ssk97]

That was for setX, which sets the value looked up here.

Looking into it further I think I can actually remove the last few getX calls, though I will have to access the ability's tag map directly instead of going through the standard CardUtil function (which requires the Game object). I'll investigate further before un-drafting.

Specifically: SpellAbility.getConvertedXManaCost(Card card) which is by Spell.getManaValue(). This is a problem because the MageObject.getManaValue() interface function does not take the Game object, but the spell's mana value needs to depend on the value chosen for X. However the information is available, so it should be fine.

[ssk97]

Also it should be noted that the logic above with getCosts is the same logic that getX uses, so even though it isn't calling the function it's still basically equivalent.

[JayDi85]

Are you planning to remove VariableCostImpl usage or do something with VariableManaCost?

[ssk97]

Eventually I would like to remove VariableCost.getAmount(), since I think all uses of it can be replaced with tags. Right now it's duplicated between the tag system and inside the cost. This is not likely to be done in the near future, though.

[ssk97]

On further investigation, containsX is still necessary. It is used to check if a spell has {X} in its mana cost, whereas the tags are for any spell with X involved in its casting, such as [[Harvest Pyre]]. However, that does make me think that CardView's check for X should probably be changed to include those sorts of spells.