chinhodado / persona5_calculator

A tool to help calculate fusions in Persona 5.
https://chinhodado.github.io/persona5_calculator/
Apache License 2.0
326 stars 133 forks source link

Pagination calculation has off-by-one edge case #63

Closed ronaldsmartin closed 3 years ago

ronaldsmartin commented 3 years ago

Hello! Thanks for your great work on this app, it's been amazingly helpful throughout my P5R playthrough! :pray:

I found a minor bug with in the persona view when the number of fusion list results is exactly divisible by the page size. One way to reproduce it is on Raoul's page with all DLC Persona's enabled in the settings:

Pagination Bug repro

What I expected: using the ⏩ button to skip to the last page would go to page 10. What happened: using the ⏩ button went to page 11, which is empty since there should just be 10 pages.

What I think is happening

The click handler for the ⏩ button sets the page index to the lastPage:

https://github.com/chinhodado/persona5_calculator/blob/76f139956145a68b0b10ed1596b1022a70681954/view/persona.html#L166

lastPage isn't 0-indexed:

https://github.com/chinhodado/persona5_calculator/blob/76f139956145a68b0b10ed1596b1022a70681954/src/PersonaController.ts#L98

In the Raoul repro, lastPage = Math.floor(200 / 20) = 10, but presumably the last page index should be 9 since the pageNum starts at 0.

Probable solution

Update the calculation of lastPage to

fusionFromTo.lastPage = Math.max(0, Math.floor(fusionFromTo.recipes.length / this.$scope.perPage) - 1);

I'd put up a PR, but it's not totally obvious how to run the app locally so I could actually test this! If you added some setup/contributing instructions to the repo, I'm happy to give it a shot.

Thanks again for sharing with the community! This issue is pretty minor and really only noticeable late in the game while trying to fuse all the high level personas, haha.

chinhodado commented 3 years ago

Sorry I totally missed this, did not receive any notification for this.

If you can make a PR that would be great! Building this is very simple, you just need to compile the typescript files.

ronaldsmartin commented 3 years ago

I took a while to get back to this! I've put up a PR to fix this, and will also put up another one with build instructions. Thanks!

chinhodado commented 3 years ago

Merged, thanks!