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

Add Frostbite status #2942

Merged
merged 21 commits into from
May 1, 2023
Merged

Conversation

Bassoonian
Copy link
Collaborator

@Bassoonian Bassoonian commented Apr 22, 2023

Adds Frostbite as status next to Freeze.

Description

This PR adds Frostbite as a new status to co-exist with Freeze. A config B_USE_FROSTBITE exists to quickly switch Freeze to Frostbite, but moves can be cherry-picked so a combination of both statuses can exist.

Images

pokeemerald 2023-04-22 18 28 18
pokeemerald 2023-04-22 18 29 23
pokeemerald 2023-04-22 18 29 31

Task list

  • Add status to the game
  • Make it function in-battle
  • Allow it to be healed with items
  • Allow moves to "thaw out"
  • Allow enemy moves to "thaw out"
  • Allow it to be healed by abilities
  • Allow it to be inflicted with relevant configs
  • Include other interactions with abilities
  • Include other interactions with moves
  • Integrate AI interactions with this status
  • Battle Pike functionality
  • New move descriptions if required

Tests

?

Bug/undesirable behaviour list

  • ITEM_EFFECT_CURE_FREEZE does not discern between strings depending on if it heals a freeze or a frostbite
  • There are some palette shenanigans going on when the opponent gets a status

Discord contact info

Jasper#5206

@Bassoonian Bassoonian marked this pull request as ready for review April 23, 2023 19:26
@Bassoonian
Copy link
Collaborator Author

Ready for review! This still needs extensive testing and the ITEM_CURE_EFFECT_FREEZE issue still needs to be addressed, but I personally have no clue how. If someone has a solution (we should wait for that before merging it probably), please feel free to PR it!

We'll probably need tests to be written too, but as previously mentioned on Discord, I use OSX and can't do it myself.

include/battle_ai_util.h Outdated Show resolved Hide resolved
@Bassoonian
Copy link
Collaborator Author

This should now be bugfree and behaviour should be as expected. Further testing is obviously recommended. Written tests would also be welcome if deemed necessary as I can't do them myself (see above).

@Bassoonian Bassoonian marked this pull request as draft April 25, 2023 20:39
@Bassoonian Bassoonian marked this pull request as ready for review April 28, 2023 11:30
@Bassoonian
Copy link
Collaborator Author

Ready for re-review, now with tests and no more bugs!

@Jaizu
Copy link

Jaizu commented Apr 30, 2023

Please give proper credits for fixing the status icons. I know RHH doesn't have a proper credits list yet (it should) but there is no harm into adding the credits into the PR or commit name.

@Bassoonian
Copy link
Collaborator Author

I attempted to add you as co-author to the commit, but it doesn't seem to have worked. I also mentioned you on the RHH Discord when I mentioned the fix:
image

@Jaizu
Copy link

Jaizu commented Apr 30, 2023

That sounds good enough to me o/

@floodryan
Copy link

Ive been looking to do this for a while. Is it available to add now? Also would it be hard to merge this with an existing project

@LOuroboros
Copy link
Collaborator

Is it available to add now?

It's code, and it's all here, so yes.

If you mean if it's available officially, then the answer is "No." though. As you can see above, this PR is still Open and not Merged.

Also would it be hard to merge this with an existing project

It depends on whether said project uses the expansion and whether it's kept up-to-date or not.

You're better off pulling the branch that was submitted in this PR and solving the merge conflicts, or alternatively implement the changes manually as they're shown in the Files Changed tab.

Also, for future reference, keep any support questions in the Discord server. The only discussions that should take place here are those that concern the PR itself and the changes it makes with the goal of getting it reviewed, approved and merged.

src/battle_ai_util.c Show resolved Hide resolved
src/battle_script_commands.c Outdated Show resolved Hide resolved
@Bassoonian Bassoonian requested a review from ghoulslash May 1, 2023 13:35
@ghoulslash ghoulslash merged commit 8cce130 into rh-hideout:upcoming May 1, 2023
@Bassoonian Bassoonian deleted the rhhupcomingdev branch May 1, 2023 13:55
@AsparagusEduardo AsparagusEduardo mentioned this pull request May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants