Roost suppresses the user's Flying-type rather than remove and re-add it. Added tests for EFFECT_ROOST.

[BLourenco]

• Changed how the engine handles Roost and it's effect on Flying types.
• Added numerous tests for many different Roost interactions.

Description

This PR initially just fixed a single issue with Roost clearing the user's type3, however looking into it further it became apparent that Roost's type changing effect needed to be handled differently than other forms of type changing. Roost acts more like a filter or suppression of the user's Flying type, where it doesn't actually remove the Flying type, and does not need to re-apply it at the end of the turn.

This is important for Roost to work correctly with other type changing moves:

• If a Pokemon gains the Flying type after it Roosted, its newly-aquired Flying typing is still suppressed.
• If a Pokemon has it's type changed in any other way after it Roosted, such as Soak, Magic Powder, Forest's Curse, Trick-or-Treat, and Color Change, those changes should remain after the end of the turn.

Tests have been added to cover a lot of other interactions and mechanics of Roost as well:

• Roost fails when at full HP or when under the effects of Heal Block.
• Roost's healing is rounded down in Gen. 4, but rounded up in Gen. 5. (Rounding up is Known Failing, no healing is rounded up currently.)
• Roost will make the user treated as either a Normal type or typeless depending on the generation and its typing.
• Roost prevents a Flying-type user from being protected by Delta Stream.
• Roost ends after Grassy Terrain's end-of-turn healing.
• Roost prevents the user's Flying typing from being copied by Reflect Type.
• Roost doesn't suppress the ungrounded effect of Levitate, Air Balloon, Magnet Rise, or Telekinesis.

This change is implemented by introducing a new GetBattlerType(u32 battler, u8 typeIndex) function and using that to retrieve a battler's type instead of accessing it directly from the battler's BattlePokemon struct. The function will return the requested type, filtering out the Flying-type if the battler has the RESOURCE_FLAG_ROOST flag set, and will return either TYPE_NORMAL or TYPE_MYSTERY depending on whether the B_ROOST_PURE_FLYING config is set to <= Gen. 4 or >= Gen 5.

Videos

Below are videos of my original fix for the user's type3. This issue is still resolved in my latest changes, but it's now handled properly.

Before Fix

roost-bug.mp4

After Fix

roost-bug-fixed.mp4

Issue(s) that this PR fixes

Adds missing tests for EFFECT_ROOST listed in #2706

Discord contact info

blourenco

[BLourenco]

WIP. This is my first PR, so let me know if anything needs to be changed.

I do have some questions:

1. Should I look at the other usages of SET_BATTLER_TYPE and confirm whether they should be clearing type3? Would that be a separate PR?
2. Forest's Curse and Trick-or-Treat are Gen 6+ moves. Should my change only apply in Gen 6+ configs?
3. I used the Absorb test as a starting point for my test (never wrote a test before). Should the Heal Block interaction be part of the Roost test, or should that be left for a Heal Block test?
4. Do I test for a Pokemon becoming grounded if its Flying typing was the only thing making it ungrounded?
5. Do I test the Z-Power effect of Z-Roost? Does that go in the same file or a separate file?
6. There's some interactions that I thought of that I'm not sure how they work and aren't listed on Bulbapedia. Specifically, what happens at the end of the turn if a Pokemon that's already roosted has its type changed through an effect of a move or ability. I can probably test some of the DS games myself, but don't have the means to test the newer games.

[AgustinGDLV]

WIP. This is my first PR, so let me know if anything needs to be changed.

I do have some questions:

1. Should I look at the other usages of SET_BATTLER_TYPE and confirm whether they should be clearing type3? Would that be a separate PR?
2. Forest's Curse and Trick-or-Treat are Gen 6+ moves. Should my change only apply in Gen 6+ configs?
3. I used the Absorb test as a starting point for my test (never wrote a test before). Should the Heal Block interaction be part of the Roost test, or should that be left for a Heal Block test?
4. Do I test for a Pokemon becoming grounded if its Flying typing was the only thing making it ungrounded?
5. Do I test the Z-Power effect of Z-Roost? Does that go in the same file or a separate file?
6. There's some interactions that I thought of that I'm not sure how they work and aren't listed on Bulbapedia. Specifically, what happens at the end of the turn if a Pokemon that's already roosted has its type changed through an effect of a move or ability. I can probably test some of the DS games myself, but don't have the means to test the newer games.

1. I think this is the most important thing to investigate. If you can find an instance where the type3 change in the macro is needed, then we can see how to proceed. If there isn't, I think changing the macro makes the most sense :) great catch!
2. I wouldn't put it behind a config since this behavior isn't generational (although I suppose there's no way to be certain.
3. The tests look fine. I think the Heal Block test is fine to include since honestly there is probably no such thing as too many tests.
4. If you could test for that, that would be wonderful. If not, just slap a TODO_TEST.
5. I don't think we have any other Z-effect tests, or at least not that many, so don't feel it's that important to.
6. Like with the rest, feel free to test yourself and include but don't feel obligated to. I think everybody appreciates it, but the most important thing in the PR is the fix (even if it's not major).

Very detailed first PR!

[BLourenco]

This PR fix kinda grew in scope after realizing that there were still a lot of issues with how Roost is currently implemented. On cart, Roost acts more of a suppression rather than a type change, where it simply filters out the Flying-type whenever the game tries to read the user's typing. This allows for other type changing effects to remain after the end of the turn. It also means that if a Roosted Pokemon gains the Flying-type after using Roost, it will still be suppressed.

There's still some other interactions that need to be tested.

[DizzyEggg]

These are nice changes. could you solve conflicts?

[BLourenco]

Resolved the conflicts.

[LOuroboros]

Now that I think about it, shouldn't this have been PR'd to Master and not Upcoming? It's kind of a big refactor 😅 ...

[AsparagusEduardo]

Now that I think about it, shouldn't this have been PR'd to Master and not Upcoming? It's kind of a big refactor 😅 ...

It wasn't that big I feel, from what I could tell, most changes were related to changing uses to GetBattlerType.

[LOuroboros]

Now that I think about it, shouldn't this have been PR'd to Master and not Upcoming? It's kind of a big refactor 😅 ...

It wasn't that big I feel, from what I could tell, most changes were related to changing uses to GetBattlerType.

The change is simple in concept, but the function is called an awful lot of times in multiple places 👀

Well, whatever.