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

Yacht Dice: Fix logic (again) so that score doesn't drop when receiving item #4044

Merged
merged 142 commits into from
Oct 22, 2024

Conversation

spinerak
Copy link
Contributor

@spinerak spinerak commented Oct 9, 2024

15/10/2024: ready for review

What is this fixing or adding?

There was an instance of score in logic dropping when receiving an item...
After investigating, there were two ways this could happen:

  • Getting an extra Roll would shift the order of categories in a bad way, so that the order becomes worse and score decreases.
  • I trimmed the simulation results, which made some categories with x dice and y rolls, better than the category with x dice and y+1 rolls.

To counter the above issues, we:

  • now fix the number of rolls to 4 when calculating the order of categories.
  • ran the trimmed simulation results through a script that fixes consecutive score distributions that were not stochastically dominant ("strictly better").

As a bonus, I swapped two for-loops in add_distributions which leads to a 10% decrease in total running time.

How was this tested?

This was tested on 29000+ default yaml single-player games and a couple of non-default single-player games.
The website and game are ready for either version :3

@spinerak
Copy link
Contributor Author

@NewSoupVi I should probably ping you here just to let you know :)
Let me know if there are some strict deadlines for submitting this one!

In the trimming of the weights, sometimes it having 4 rolls would be better than having 5 rolls.
I did a check that this does not happen for any dice increment or roll increment
@NewSoupVi
Copy link
Member

Ftr, I'm not that bothered about this one making 0.5.1, because it is quite rare. But if we can make it happen it would be preferrable ofc

@spinerak
Copy link
Contributor Author

Agreed, the issue is rare and it feels strange to have all generations be a bit slower to avoid a 1 in 10000+ error.
For 0.5.1, I only find #3967 important, this fixes stuff in the setup guide.

This method is faster if the first for-loop contains fewer items.
Since the function is called with, typically, `dist2` having less items, let's loop over `dist2` first. This makes the entire program 10% faster.
@spinerak spinerak marked this pull request as ready for review October 15, 2024 19:53
@Exempt-Medic Exempt-Medic added is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Oct 17, 2024
@spinerak
Copy link
Contributor Author

Update:
There were some scores in the weights that had probability 0 to happen. I've removed them.
Did 1000 default-yaml test runs too.

@NewSoupVi NewSoupVi merged commit 703e339 into ArchipelagoMW:main Oct 22, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants