Adds function AI_CalcMoveScore to more easily control score increases

[AlexOn1ine]

Description

Often times scores in AI_CheckViability were increased uncontrollable through various functions, e.g. IncreaseStatUpScore. This isn't necessarily bad because some moves are really good and deserve a high score but it collides with other functions that control score because we won't exactly know by how much a score will be increased for a given move which makes it harder to adjust scores.

My possible solution with AI_CalcMoveScore is to calculate a temp score and return a final score based on the moves viability. This allows for more control because we will always know the threshold by which a score can be increased. We are also free to increase a score of an effect by any amount without worries to disrupt any other interaction.

For now I simply created the function and moved the checks to it and haven't written any tests but I would be glad to write some if specific scenarios need testing.

Also in the future, if I get the okay, I would like to phase out AI_CheckBadMove. I think we can manage everything through score increases. Having a function that also decreases scores seems redundant.

[mrgriffin]

I really like the idea, it should hopefully improve the AI's strength :)

I've approved this because I think it all looks good, but there is one suggestion about a comment that I think shouldn't have been changed that I'd like you to apply before merging please!

I've left a bunch of comments about the AI's logic that are all non-blocking because afaict none of them are about your changes, they're about how the AI worked before this PR. If there's any ideas you agree with then please raise an issue and we can look at getting them done in the future!

[mrgriffin]

I wonder if this should also use IncreaseStatUpScore? It's a bit annoying because then V-Create onward need to be split out—although maybe they should, V-Create should probably do IncreaseStatUpScore twice for the defenses, and then a +3 for the speed if it's slower?

---

It would be useful if V-Create could be decomposed into MOVE_EFFECT_DEF_MINUS_1, MOVE_EFFECT_SPD_MINUS_1, and MOVE_EFFECT_SP_DEF_MINUS_1, flipped into the PLUS versions by Contrary, and then passed through the switch statement... But I think this would be a big scope creep for this PR, so I'm just mentioning it as an idea for the future.

[mrgriffin]

This logic was probably already here, but I'm not convinced we should give -6 to a move just because one of its secondary effects doesn't do anything. Imagine we have Tackle and Rapid Spin in gen 8+, we should go for Spin (especially if slower) because comparable damage and a +1 Speed.

[AlexOn1ine]

I 100% agree.

[mrgriffin]

Another "for the future" comment, we should probably have some way to judge how good/bad an opponent's stat change is for us, and then adjust the score based on how bad a +1 is when the opponent has Contrary. (And also how good a -1 is if they don't—for example if we don't have any physical moves then a -1 Defense isn't that useful).
EDIT: Is this IncreaseStatUpScore with attacker and target flipped, plus making it be a - instead of a +? Or if IncreaseStatUpScore returned a number instead of adjusting &score, ADJUST_SCORE(-IncreaseStatUpScore(battlerDef, battlerAtk, ...)).

[mrgriffin]

Not a problem necessarily, but it's interesting that we don't distinguish between +1 and +2 in IncreaseStatUpScore. Are there examples where Howl and Swords Dance get the same score, or is that caught elsewhere? Of course in any serious battle-based hack you wouldn't give both of those moves to a 'mon so even if this does happen, it's low priority imo.

[mrgriffin]

This code already existed, but I think Spectral Thief should also get the Clear Smog bonus because it resets the target's (positive?) stats?

Conceptually this, Clear Smog, Haze, etc could all be expressed in terms of IncreaseStatUpScore (if it also supported lowering stats and returned the number as suggested in an earlier "for the future" comment).

[AlexOn1ine]

yep, agreed.

[mrgriffin]

Not a PR comment at all, but I wonder if this condition has ever been true—for non-Omniscient AI wouldn't you need to have a player use Fling, then Thief an Iron Ball? 😅

[mrgriffin]

Another "for the future" comment based on Iron Ball/Lagging Tail/Sticky Barb being here—an AI score in the item table which says how valuable this item is to hold. Then Knock Off is worth that many points, Trick would be worth the difference in their points, Thief is worth twice that many points (we take it from them and give it to us), etc.

Of course there's contextual information about which items are helpful, a defensive Pokémon doesn't want Choice Band even though it's abstractly valuable, Knocking Off a Toxic Orb that was held since the start of the battle is probably 0 or positive (because they held it for a reason), etc, but I digress!

EDIT: I see that Thief's move effect also mentions a set of items that might be worth holding, and a lot of those are conditional. I suspect a function that asks "how many points is holding worth?" would be helpful, similar to my suggestion for IncreaseStatScore because then we can use it to work out how many points it's worth in context (rather than relying on the static table I suggested before the EDIT).

EDIT 2: Also, Unburden could be considered when it comes to removing items.

[mrgriffin]

Throat Chop lasts for two turns, so I wonder if this should be something like:

Suggested change

	if (HasSoundMove(battlerDef) && gBattleMoves[predictedMove].soundMove && AI_WhoStrikesFirst(battlerAtk, battlerDef, move) == AI_IS_FASTER)
	ADJUST_SCORE(3);
	if (HasSoundMove(battlerDef))
	{
	if (gBattleMoves[predictedMove].soundMove && AI_WhoStrikesFirst(battlerAtk, battlerDef, move) == AI_IS_FASTER)
	ADJUST_SCORE(3);
	else
	ADJUST_SCORE(1);
	}

---

And then "for the future" it might be helpful to change HasSoundMove into something more like IsLikelyToUseSoundMove—so not quite predictedMove which is "is likely to go for it this turn", but "is likely to go for it vs our Pokémon". I suspect that kind of thing comes up a lot, e.g. Taunt is very good if we're faster and can prevent their move this turn, or if they have a status move that we think they might go for. Same for Substitute and status moves that target us, etc etc.

I suspect the logic for "is likely to go for it" could be based on what the AI scores their moves vs our Pokémon. Only moves with good scores are likely to be used. No point Taunting to prevent Growl if we're a special attacker—but we could probably see that because Growl should score low vs special attackers.

[AlexOn1ine]

agreed

[AlexOn1ine]

Thanks for the helpful comments. I commented on a couple where it was immediately obvious to me. I'll go through the rest later.
So the options are. Either merge the PR right now, and address the comments in a follow up PR or address them here. I'm fine with both.

/Edit: for some reason the review got dismissed. Might be because I applied to comment change.

[mrgriffin]

I merged it because it's possible that the follow-up stuff might end up needing more discussion, whereas these bits are all uncontroversial (imo!) :)