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] Implement Fling #2200

Closed
wants to merge 10 commits into from

Conversation

AyushBarik
Copy link

@AyushBarik AyushBarik commented Jun 14, 2024

What are the changes?

The user can now utilize the move fling.

Why am I doing these changes?

It is an important move in the game.

Greater diversity of available moves, which enhances user experience.

What did change?

The move fling is now fully functional.

Screenshots/Videos

fling.mp4

How to test the changes?

I made use of src/overrides.ts to test different items. I did not introduce automated tests.
I carried out tests on various items and confirmed functionality. If there is an issue, it may be in
determining which items are able to be flung and which are not. You may want to check my arrays
to see if the items are appropriately sorted. You can also go through the code and try to see
if the logic makes sense. I left behind some comm

My Discord username is bgjormungandr, I'm on the pokerogue server. Feel free to contact me there if needed.

Checklist

  • [ yes] There is no overlap with another PR?
  • [ yes] The PR is self-contained and cannot be split into smaller PRs?
  • [ yes] Have I provided a clear explanation of the changes?
  • [ yes] Have I tested the changes (manually)?
    • [did not implement tests] Are all unit tests still passing? (npm run test)
  • [ yes] Are the changes visual?
    • [yes ] Have I provided screenshots/videos of the changes?

@hayuna
Copy link
Contributor

hayuna commented Jun 14, 2024

Could you update title of this PR?

[Move] Implement Fling

@Tempo-anon Tempo-anon added the Ability Affects an ability label Jun 14, 2024
@AyushBarik AyushBarik changed the title Working on the move fling. Currently a work in progress. [Move] Implement Fling Jun 15, 2024
@AyushBarik AyushBarik marked this pull request as ready for review June 15, 2024 19:12
@Arxalc
Copy link
Contributor

Arxalc commented Jun 15, 2024

I see why you included all the item names, but since most of them aren't currently in PokeRogue, I'd just comment them out and let someone else add them in once the item is implemented in the future.

You can completely get rid of evolutionary stones. Pokemon don't hold those.

src/data/move.ts Outdated Show resolved Hide resolved
src/data/move.ts Outdated 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 added Move Affects a move and removed Ability Affects an ability labels Jun 16, 2024
AyushBarik and others added 2 commits June 16, 2024 22:28
… heldItems check moved to the beginning, dealt with berries without importing berry-type.ts
@AyushBarik
Copy link
Author

Changes made as per the Suggestions:

  • Got rid of evolutionary items as flingable items
  • heldItems check moved to the beginning of the function
  • Dealt with berries without importing berry-type.ts

Changes not made:

  • I was suggested to move the validHeldItems check at the end of the else-if statements but I felt that may cause bugs so I have not implemented it for now

Additionally, as for commenting out unimplemented items from the list of flingable items - I am not sure which items have or have not been implemented. Perhaps someone more familiar with the game can change the arrays accordingly.

AyushBarik added a commit to AyushBarik/pokerogue that referenced this pull request Jun 17, 2024
src/data/move.ts Outdated Show resolved Hide resolved
src/data/move.ts Outdated Show resolved Hide resolved
src/data/move.ts Outdated Show resolved Hide resolved
… apply, Unnessesary comments removed, getMyHeldItems removed as a function and made inline.
Copy link
Author

@AyushBarik AyushBarik left a comment

Choose a reason for hiding this comment

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

Implemented all feedback from Tempo-anon:

  • tsdoc created for class and apply
  • unnecessary comments removed
  • getMyHeldItems removed as a function and made inline instead.

src/data/move.ts Outdated
Comment on lines 2690 to 2717
const arr10i = [
"Sitrus Berry", "Lum Berry", "Enigma Berry", "Liechi Berry", "Ganlon Berry", "Petaya Berry", "Apicot Berry", "Salac Berry",
"Lansat Berry", "Starf Berry", "Leppa Berry", "Choice Band", "Choice Scarf", "Choice Specs", "Air Balloon", "Absorb Bulb", "Amulet Coin", "Big Root",
"Destiny Knot", "Expert Belt", "Focus Band", "Focus Sash", "Lagging Tail", "Leftovers", "Mental Herb",
"Muscle Band", "Power Herb", "Red Card", "Shed Shell", "Silk Scarf", "Silver Powder", "Smooth Rock", "Soft Sand",
"Soothe Bell", "White Herb", "Wide Lens", "Wise Glasses", "Zoom Lens"
];
const arr30i = [
"Flame Orb", "Yellow Flute", "Amulet Coin", "Binding Band", "Black Belt", "Black Glasses", "Black Sludge", "Cell Battery",
"Charcoal", "Eject Button", "Escape Rope", "Exp. Share",
"Float Stone", "Heart Scale", "King's Rock", "Life Orb", "Light Ball", "Light Clay",
"Lucky Egg", "Luminous Moss", "Magnet", "Metal Coat", "Metronome", "Miracle Seed",
"Mystic Water", "NeverMeltIce", "Pearl", "Poké Doll", "Razor Fang", "Scope Lens", "Shell Bell",
"Smoke Ball", "Snowball", "Spell Tag", "Stardust", "Toxic Orb",
"Twisted Spoon"];
const arr40i = ["Eviolite", "Icy Rock"];
const arr50i = ["Sharp Beak"];
const arr60i = ["Damp Rock", "Heat Rock", "Macho Brace", "Rocky Helmet"];
const arr70i = ["Dragon Fang", "Poison Barb", "Power Anklet", "Power Band", "Power Belt", "Power Bracer",
"Power Lens", "Power Weight"];
const arr80i = [
"Assault Vest", "Quick Claw", "Razor Claw",
"Sachet", "Safety Goggles", "Sticky Barb",
"Weakness Policy"
];
const arr90i = ["Grip Claw"];
const arr100i = ["Hard Stone"];
const arr130i = ["Iron Ball"];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of making huge arrays of item names like this wouldn't it be better to just add the flingPower / weight of the item onto that item's class (ModifierType)?

src/data/move.ts Outdated Show resolved Hide resolved
@AyushBarik
Copy link
Author

AyushBarik commented Jun 27, 2024

NoValidHeldItems.2.mov

Video that displays the functionality of fling as well as the case if none of the held items possessed by the Pokemon are flingable. (For testing purposes flame orb was made unflingable temporarily).

@AyushBarik
Copy link
Author

noHeldItems.1.mov

Displays what happens if the user has no held items

Copy link
Collaborator

@OrangeRed OrangeRed left a comment

Choose a reason for hiding this comment

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

Since you're validating against the names of items does this work correctly in different locales?

Comment on lines +2775 to +2777
if (validHeldItems.length === 0) {
return "No valid held items!";
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this necessary? Doesn't the game just show "But it failed" if you don't have a valid item?

Copy link
Author

Choose a reason for hiding this comment

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

Not necessary, I just thought it would be useful. The user might have held items, but might be wondering why fling failed. this message tells them that their items are not flingable.

Copy link
Author

Choose a reason for hiding this comment

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

"Since you're validating against the names of items does this work correctly in different locales?"

I tried running the game in other languages and fling does not seem to work... How do i fix this issue?

@f-fsantos f-fsantos changed the base branch from main to beta July 10, 2024 17:47
AyushBarik added a commit to AyushBarik/pokerogue that referenced this pull request Jul 29, 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.

This does not work since it's checking for hardcoded english names. Could we not update modifier-type.ts to include the relevant type and damage info while just having the special effects in the FlingAttr?

@flx-sta
Copy link
Collaborator

flx-sta commented Sep 17, 2024

@AyushBarik closed for being stale (no updates for over a month)


Feel free to create a new PR or ask us to re-open this one.

@flx-sta flx-sta closed this Sep 17, 2024
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.

6 participants