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

Core: Attribute per slot random directly to the World and discourage using MultiWorld's random #1649

Merged
merged 8 commits into from
Jul 2, 2023

Conversation

alwaysintreble
Copy link
Collaborator

@alwaysintreble alwaysintreble commented Apr 1, 2023

What is this fixing or adding?

Using the multiworld's random tends to give problems with multithreading etc. but per_slot_random is awkard to use. This adds the random object directly to AutoWorld for worlds to use. Next steps are to fully deprecate and remove multiworld random usage, then per_slot_random. Needs a decent solution for stage_callables and if the multiworld needs to do any randomization during output/multidata stages. Since stage_callable gets called through a set, using the multiworld random is already incorrect for those.

The biggest problem with this currently is that the worlds don't exist until after options get set... Wondering if as part of this PR those should be set up in a separate stage, the seed should be set after, or if we just leave it how I have it currently set.

How was this tested?

Added it to The Messenger and ran some generations.

@Zunawe
Copy link
Collaborator

Zunawe commented Apr 3, 2023

For what it's worth, I like this interface better. But I don't have the expertise to comment on functionality side effects.

@ThePhar
Copy link
Member

ThePhar commented Apr 11, 2023

image

# Conflicts:
#	worlds/messenger/__init__.py
@ThePhar ThePhar added is: documentation Improvements or additions to documentation. is: enhancement Issues requesting new features or pull requests implementing new features. is: refactor/cleanup Improvements to code/output readability or organizization. affects: core Issues/PRs that touch core and may need additional validation. labels May 31, 2023
Copy link
Member

@NewSoupVi NewSoupVi left a comment

Choose a reason for hiding this comment

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

Code seems good, and I agree with the change

@ThePhar ThePhar merged commit a6ba185 into ArchipelagoMW:main Jul 2, 2023
11 checks passed
@el-u el-u mentioned this pull request Jul 12, 2023
@alwaysintreble alwaysintreble deleted the slot_random branch July 19, 2023 21:46
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
…using MultiWorld's random (ArchipelagoMW#1649)

* add a random object to the World

* use it in The Messenger

* the worlds don't exist until the end of set options

* set seed in lttp tests

* use world.random for shop shuffle
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
…using MultiWorld's random (ArchipelagoMW#1649)

* add a random object to the World

* use it in The Messenger

* the worlds don't exist until the end of set options

* set seed in lttp tests

* use world.random for shop shuffle
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: documentation Improvements or additions to documentation. is: enhancement Issues requesting new features or pull requests implementing new features. is: refactor/cleanup Improvements to code/output readability or organizization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants