shadorki / genshin-impact-wish-simulator

A React web application to simulate Genshin Impact gacha in the browser
713 stars 162 forks source link

Fix Venti bug, added test and an extra message #10

Closed lauslim12 closed 4 years ago

lauslim12 commented 4 years ago

Hello!

This PR addresses the following:

Oh, and I think I found a bug:

Here is an example of the bug. As you can see, I have fixed it so the second SSR is Venti. However, if I tested it many times and added a console.log to print the drops to the screen, an SSR is always on the 90th pull, even if we had gained one SSR beforehand (please ignore the true and false, it's just I printed it to the console so I can see the variable values for easier debugging).

Capture4

I think I can fix that if you want, what do you think? I'll make another issue and another pull request, as they say that smaller PR's are better 😅.

Thank you very much!

Resolves #2

shadorki commented 4 years ago

Reviewing now, as for the bug you found, I may have not understood properly

When we receive an SSR that is not from the 90 pull, I believe the attemptsCount variable is not reset to zero, which is the expected behavior from Genshin Impact's gacha. In other words, because of this bug, we are guaranteed to get an SSR every 90 attemptsCount, even if we had gained one SSR beforehand.

Are you saying that we should not get a guaranteed 5 star pull every 90 attempts? It states atleast once per 90 attempts image

The setter in the class sets the guaranteed5Star property dynamically.

image

Ah I believe it makes sense now, you want to reset the attempts count after recieving a 5 star to make sure the guarantee follows through,

The attempts count is used in the tests to provide further expandability in the future as well.

If you do update it, you will have to test the app vigorously to make sure it works properly but I like the idea.

lauslim12 commented 4 years ago

Okay, code updated to your review! Thank you!

Are you saying that we should not get a guaranteed 5 star pull every 90 attempts?

I actually mean that the '90 attempt' counter should be reset if we pulled an SSR (because I notice a pattern where we'll get an SSR every 90 rolls, even if we had gotten an SSR in the middle of the count). I think Genshin Impact's gacha works like that? Every time we get an SSR, the counter will be reset back to zero.

5-Star pity: If a player does not win a 5-star item 89 pulls in a row the 90th pull is guaranteed to be a 5-star item. This counter will reset if the player pulls any 5-star item.

Source is from the fandom wiki, but honestly, I actually believe that the gacha system is still quite mysterious, even to us...

lauslim12 commented 4 years ago

You're welcome! I always love contributing in open source projects!

I think I'll read more about the gacha before contributing the attemptsCounter so I can get a clearer picture on it. Do I have to make a new issue?

Thank you!

shadorki commented 4 years ago

We'll hold off on it for now, till after I finish Klees banner and we can do a refactor.