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

[Move] Update Tera Starstorm (still Partial), Readd Partial tag to Tera Blast #4549

Merged
merged 10 commits into from
Oct 3, 2024

Conversation

torranx
Copy link
Collaborator

@torranx torranx commented Oct 2, 2024

What are the changes the user will see?

Tera Starstorm updated implemention
Why (P)? - both do not ignore abilities that affect stats

Why am I making these changes?

partially implemented move, missing the ff when used by Terapagos in its Stellar Form:

What are the changes from a developer perspective?

  • Changed TeraBlastCategoryAttr class name to TeraMoveCategoryAttr to align its usecase
  • new class TeraStarstormTypeAttr to handle type changing logic for the move
  • added special targeting for the move
  • misc: updated magic numbers to instead use enums for expanding force

Screenshots/Videos

How to test the changes?

Checklist

  • I'm using beta as my base branch
  • There is no overlap with another PR?
  • The PR is self-contained and cannot be split into smaller PRs?
  • Have I provided a clear explanation of the changes?
  • Have I considered writing automated tests for the issue?
  • If I have text, did I make it translatable and add a key in the English locale file(s)?
  • Have I tested the changes (manually)?
    • Are all unit tests still passing? (npm run test)
  • Are the changes visual?
    • Have I provided screenshots/videos of the changes?

@torranx torranx added the Move Affects a move label Oct 2, 2024
@torranx torranx marked this pull request as ready for review October 3, 2024 08:56
@torranx torranx requested a review from a team as a code owner October 3, 2024 08:56
@Xavion3
Copy link
Contributor

Xavion3 commented Oct 3, 2024

Fusions exist, so this will need to account for those.

That and also this is still partial for the same reasons tera blast is, TeraBlastCategoryAttr was only a partial implementation when I first implemented, and it doesn't look like anyone has fixed it since.

@torranx torranx marked this pull request as draft October 3, 2024 09:31
@torranx torranx changed the title [Move] Fully implement Tera Starstorm [Move] Update Tera Starstorm (still Partial) Oct 3, 2024
@torranx torranx marked this pull request as ready for review October 3, 2024 10:01
@torranx
Copy link
Collaborator Author

torranx commented Oct 3, 2024

Fusions exist, so this will need to account for those.

That and also this is still partial for the same reasons tera blast is, TeraBlastCategoryAttr was only a partial implementation when I first implemented, and it doesn't look like anyone has fixed it since.

addressed 👍

@torranx torranx requested a review from DayKev October 3, 2024 10:05
DayKev
DayKev previously approved these changes Oct 3, 2024
src/data/move.ts Outdated Show resolved Hide resolved
@torranx torranx requested a review from DayKev October 3, 2024 10:39
@torranx torranx changed the title [Move] Update Tera Starstorm (still Partial) [Move] Update Tera Starstorm (still Partial), Readd Partial tag to Tera Blast Oct 3, 2024
DayKev
DayKev previously approved these changes Oct 3, 2024
Copy link
Collaborator

@Tempo-anon Tempo-anon left a comment

Choose a reason for hiding this comment

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

Mostly nits, looks good otherwise

src/data/move.ts Show resolved Hide resolved
src/data/move.ts Outdated Show resolved Hide resolved
src/data/move.ts Show resolved Hide resolved
@Tempo-anon Tempo-anon merged commit 76e25a6 into pagefaultgames:beta Oct 3, 2024
14 checks passed
@torranx torranx deleted the move/tera-starstorm branch October 4, 2024 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Move Affects a move
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants