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

Allow selection of item quantity when transferring items #723

Closed
wants to merge 8 commits into from

Conversation

dielle000
Copy link
Contributor

This PR expands the item transfer functionality allowing to select how many items to transfer instead of being forced to transfer the entire stack.

The transfer menu is also changed so that after a single transfer operation the user is not kicked out of the menu. This is particularly helpful when moving around a lot of items.

Changes

  • Added UI for selection of item quantity if stackCount > 1. Introduced PartyUiMode.MODIFIER_TRANSFER_QUANTITY, PartyOption.TRANSFER_QUANTITY and private variables transferQuantity and transferQuantityMax in party-ui-handler.ts;

  • Changed method tryTransferHeldItemModifier parameter transferStack (boolean) into transferQuantity (integer, default value 1). Every call to this method has been changed to keep the behavior consistent. This change has been tested with the move Covet, abilities Magician and Pickpocket, the item Baton and the fusion mechanic (items of both Pokemon are kept);

  • Changed type PartyModifierTransferSelectCallback to include the itemQuantity;

  • Changed method startTransfer in party-ui-handler.ts so that it doesn't automatically update the variable transferOptionCursor. Without this change, the transfer would always involve the first item in the list because of the addition of the new UI element (which has only one option).

Screenshot of the new UI

transfer_quantity

I hope that this is helpful and that I didn't break anything. Have a nice day!

@Flashfyre
Copy link
Contributor

I appreciate the contribution, but this UI is a bit inconsistent. There's a lot of empty space around the quantity select and it isn't centered either. I'd like to see this fixed before it gets in. Also, having this available from the item select itself would be even better, if that can be made to look good.

@Madmadness65 Madmadness65 added the Enhancement New feature or request label May 10, 2024
@dielle000
Copy link
Contributor Author

Thank you for the feedback. Selecting the item and the quantity at the same time would definitely work better, so I'll make the change.

@dielle000
Copy link
Contributor Author

Now the UI is the same as before but it is possible to change the quantity by pressing left or right if the cursor is on an item with stack bigger than 1

@Greenlamp2
Copy link
Collaborator

Could you post some screenshots/videos to show the latest changes please ?

@dielle000
Copy link
Contributor Author

Sure, here you go!

screen-recording-2024-05-21.mp4

@brain-frog
Copy link
Collaborator

I'd like it if there was any indication for players that they can increment/decrement. I see you had "<" and ">" on either side before, it might be good to have the cursor on either side mimicking that to imply you can change it

@bennybroseph
Copy link
Collaborator

I see you had "<" and ">" on either side before, it might be good to have the cursor on either side mimicking that to imply you can change it

This sounds a little clunky. Can we just add height to the existing message window on the left?

image

@bennybroseph
Copy link
Collaborator

looks like an improper merge or rebase messed this pull request up pretty badly. either revert the commit or make a new fresh one based on main.

@bennybroseph bennybroseph marked this pull request as draft May 26, 2024 09:05
@dielle000
Copy link
Contributor Author

Yeah, I'll make a new PR

bennybroseph pushed a commit that referenced this pull request May 30, 2024
…#1394)

* Transferring item does not kick out of transfer menu

* Select simultaneously the item to transfer and the quantity

* eslint fix

* eslint fix

* Reset quantity on scroll

* Documentation

* eslint fix
Korwai pushed a commit to Korwai/pokerogue that referenced this pull request Jun 14, 2024
…aultgames#723) (pagefaultgames#1394)

* Transferring item does not kick out of transfer menu

* Select simultaneously the item to transfer and the quantity

* eslint fix

* eslint fix

* Reset quantity on scroll

* Documentation

* eslint fix
SavGRY pushed a commit to SavGRY/pokerogue that referenced this pull request Jun 16, 2024
…aultgames#723) (pagefaultgames#1394)

* Transferring item does not kick out of transfer menu

* Select simultaneously the item to transfer and the quantity

* eslint fix

* eslint fix

* Reset quantity on scroll

* Documentation

* eslint fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants