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

Raft: Fix item prefilling #1878

Merged
merged 5 commits into from
Jun 20, 2023
Merged

Conversation

SunnyBat
Copy link
Collaborator

What is this fixing or adding?

Raft is prefilling frequencies incorrectly when multiple Raft worlds with different frequencies are used in the same multiworld. This makes Raft worlds use their own items instead of any Raft world's items.

Currently, what Raft's prefilling is doing is it's looking for the first item with a given name in the item pool. When every Raft player in a multiworld has similar settings (in this case, everyone either has prefilled items or does not have prefilled items), this is not a problem -- players' items are inserted into the item pool and removed from the item pool in (hopefully) the same order, so their generations are correct. However, when there are different settings in different Raft worlds, problems arise. Now, the Raft worlds that prefill items are at risk of using other Raft worlds' items, since they're pulling the first item with a given name, rather than guaranteeing their own item.

This does three things:

  1. Correctly prefills frequencies on all Raft worlds, even with different settings
  2. Puts progressive-frequencies into the duplicate item pool. It reduces the odds of getting a progressive-frequency when filling the items (compared to any frequency in a non-progressive frequency mode) to avoid spamming a ton of progressive-frequency items.
  3. Tracks the items that will be prefilled per-world, meaning that we don't need to look through the entire multiworld's item pool to pull out the items we care about. This is likely a performance win, especially on larger multiworlds.

How was this tested?

Manually tested multiple different Raft configurations with multiple different settings for frequency placement in multiple multiworld generations, and verified that each Raft world's locations were in an expected spot.
I also mixed up the order of the frequency placements to verify that it wasn't just a "lucky order" in which players' items were prefilled.

@@ -129,15 +146,15 @@ def collect_item(self, state, item, remove=False):
return super(RaftWorld, self).collect_item(state, item, remove)

def pre_fill(self):
if self.multiworld.island_frequency_locations[self.player] == 0:
if self.multiworld.island_frequency_locations[self.player] == 0: # Vanilla
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if self.multiworld.island_frequency_locations[self.player] == 0: # Vanilla
if self.multiworld.island_frequency_locations[self.player] == "vanilla":

for future reference, you can just do that.

@Berserker66 Berserker66 merged commit afe9e12 into ArchipelagoMW:main Jun 20, 2023
Witchybun pushed a commit to Witchybun/Archipelago that referenced this pull request Jun 26, 2023
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants