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

[Ability] Implement Mirror Armor #4769

Open
wants to merge 20 commits into
base: beta
Choose a base branch
from

Conversation

PrabbyDD
Copy link
Contributor

@PrabbyDD PrabbyDD commented Oct 31, 2024

Should be working now, and passes all test

When magic bounce is implemented, should probably add a test for how mirror armor interacts with magic bounce, but
I feel like how its implemented now should be fine, since magic bounce shouldnt reflect stat moves directly, rather status moves (some of which are stat moves). But still a test should be made.

What are the changes the user will see?

Mirror Armor will work as intended in the game

Why am I making these changes?

Adding new gameplay features

What are the changes from a developer perspective?

Added several variables to keep track of which pokemon used certain moves or abilities so mirror armor can properly
target

Added a new ability attr

changed certain phase files to implement this behavior,

I dont really like using all these extra variables and tried to keep the additions to a minimum, but I just couldnt find enough
tools to do what I needed to do so I had to hard code in some of these variables to make the ability work.

Screenshots/Videos

Video 1
2024-11-19.13-20-40.mp4
Video 2
2024-11-19.13-19-58.mp4
Video 3
2024-11-19.13-19-03.mp4
Video 4
2024-11-19.13-17-21.mp4
Video 5
2024-11-19.13-00-22.mp4
Video 6
2024-11-19.13-04-07.mp4
Video 7
2024-11-19.13-04-53.mp4
Video 8
2024-11-19.13-05-39.mp4
Video 9
2024-11-19.13-06-20.mp4
Video 10
2024-11-19.13-07-30.mp4
Video 11
2024-11-19.13-08-09.mp4

How to test the changes?

Override Files and unit tests, and debugger in browser

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?

@PrabbyDD PrabbyDD requested a review from a team as a code owner October 31, 2024 23:26
@PrabbyDD
Copy link
Contributor Author

I would also like if someone updated the ability draft with this PR as well...I dont think I can. Thanks :)

@PrabbyDD PrabbyDD marked this pull request as draft October 31, 2024 23:35
@Tempo-anon Tempo-anon added the Ability Affects an ability label Nov 5, 2024
@PrabbyDD PrabbyDD marked this pull request as ready for review November 19, 2024 21:23
@DayKev DayKev added the Enhancement New feature or request label Dec 1, 2024
@DayKev DayKev changed the title [Ability Implementation] Implementing Mirror Armor [Ability] Implement Mirror Armor Dec 1, 2024
Copy link
Collaborator

@DayKev DayKev left a comment

Choose a reason for hiding this comment

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

For ArenaTags, they should have a sourceId field that contains the ID of the pokemon that set it. If that's not viable for whatever reason, you should be able to add a field to the class used by Sticky Webs etc to store the necessary information instead.

src/data/ability.ts Outdated Show resolved Hide resolved
src/data/ability.ts Outdated Show resolved Hide resolved
src/data/ability.ts Outdated Show resolved Hide resolved
src/data/ability.ts Outdated Show resolved Hide resolved
@PrabbyDD
Copy link
Contributor Author

I changed the code to use the source ID field from arena tag. I dont know why there is an issue with public/locales in this repo - and I am not sure how to fix it. I dont think I touched public/locales in this branch.

@DayKev
Copy link
Collaborator

DayKev commented Dec 10, 2024

Try running git submodule update --progress --init --recursive, then commit and push.

@innerthunder
Copy link
Contributor

git submodule update --remote --recursive --force usually does the trick for me

@PrabbyDD
Copy link
Contributor Author

I tried running those commands and updating the submodule but it hasnt changed the conflict in github. Not really sure how to proceed, never seen something like this before.

@PrabbyDD
Copy link
Contributor Author

Should I just make a new branch with these changes and try to redo it? I dont think I manually made changes to public/locales at any point in this branch

@DayKev
Copy link
Collaborator

DayKev commented Dec 10, 2024

Fixed it by merging beta locally, not sure what GitHub's problem was.

@DayKev
Copy link
Collaborator

DayKev commented Dec 10, 2024

Oh wait, that made it pull the latest remote commit...? What is even going on.

@DayKev DayKev force-pushed the prabbydMirrorArmor branch from cd241fe to 46affa2 Compare December 10, 2024 23:56
@DayKev
Copy link
Collaborator

DayKev commented Dec 10, 2024

Okay there we go, now it's fixed. Git can be very annoying sometimes...

@PrabbyDD
Copy link
Contributor Author

PrabbyDD commented Dec 10, 2024

Appreciated. Do you know how I can fix this locally?
When I run git submodule update --remote --recursive --force

I get
fatal: Needed a single revision
Unable to find current origin/master revision in submodule path 'public/locales'

I have tried

rm -rf public/locales
git submodule sync --recursive
git submodule update --init --recursive

and

git submodule update --remote --recursive --force

but it hasnt really changed anything...

@DayKev
Copy link
Collaborator

DayKev commented Dec 11, 2024

Hmmm... not sure. Submodules are weird. Try the following:

git reset --hard 07cea3f
git pull
git submodule update --progress --init --recursive --force

@PrabbyDD
Copy link
Contributor Author

When I do that then I have to commit public/locales and once that is done then my local branch is ahead of upstream/beta by 16 commits which doesnt make sense to me - that means I have extra stuff in my repo right? I tried to delete the beta repo and re pull but same issue with public/locales exists...

@DayKev
Copy link
Collaborator

DayKev commented Dec 11, 2024

Oh wait you were trying to fix your local beta branch? In that case change the SHA1 in my above message to f2ef3620. So like this:

git reset --hard f2ef3620
git pull
git submodule update --progress --init --recursive --force

Sorry I thought you were trying to fix your local version of this PR branch. Hopefully that should fix your local version of the beta branch.

@PrabbyDD
Copy link
Contributor Author

Got it, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ability Affects an ability Enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

4 participants