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.

Sign up for GitHub

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

Jump to bottom

OoT Adjuster: Remove per_slot_randoms #4264

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

OoT Adjuster: Remove per_slot_randoms #4264

wants to merge 4 commits into from

Conversation

nicholassaylor
Copy link
Contributor

What is this fixing or adding?

Remove use of per_slot_randoms in the OoT Adjuster by changing it to the world's random
Addresses #3382 (comment)

How was this tested?

It was not

@github-actions github-actions bot added affects: core Issues/PRs that touch core and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Nov 27, 2024
@alwaysintreble
Copy link
Collaborator

Not tested, but looking at what it's doing here I think this line can just be straight up removed. Before we had per_world_random, per_slot_randoms was set in set_seed, which is skipped here, but it's initialized in the world constructor now. Random import can likely also be removed

@nicholassaylor
Copy link
Contributor Author

Not tested, but looking at what it's doing here I think this line can just be straight up removed. Before we had per_world_random, per_slot_randoms was set in set_seed, which is skipped here, but it's initialized in the world constructor now. Random import can likely also be removed

On second look, I would agree

@Exempt-Medic Exempt-Medic added is: enhancement Issues requesting new features or pull requests implementing new features. is: refactor/cleanup Improvements to code/output readability or organizization. labels Nov 27, 2024
alwaysintreble
alwaysintreble approved these changes Nov 28, 2024
View reviewed changes
Copy link
Collaborator

@alwaysintreble alwaysintreble left a comment

Choose a reason for hiding this comment

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

Still not tested but looks correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alwaysintreble alwaysintreble alwaysintreble approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features. is: refactor/cleanup Improvements to code/output readability or organizization. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nicholassaylor @alwaysintreble @Exempt-Medic