Skip to content

TUNIC: Deal with other games placing stuff in other worlds during pre_fill #5164

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

Closed

Conversation

ScipioWright
Copy link
Collaborator

What is this fixing or adding?

Yes yes, I know, you're not supposed to place stuff in other worlds during pre_fill (presumably, idk if the docs even say that anywhere). But, we've got like 300 non-verified apworlds around. So I'd rather just be more resilient than pout and say "but muh spec".

During the local filler stuff that Tunic does for grass and breakable shuffle, during pre_fill it'll grab a set of locations to fill during stage_pre_fill. If another world fills any of those locations between that Tunic slot's pre_fill and stage_pre_fill, it'll crash when doing place_locked_item. So, this will make it more resilient to that sort of crash by placing it in the item pool. I know we're not supposed to place stuff in the item pool during pre_fill, but you also aren't supposed to fill other worlds' locations during pre_fill. It's something that is out of spec that only happens if something else is out of spec, so I'd say that's fair game.

How was this tested?

LlisandurFF4.txt
Ixrec Tunic.txt

Test gens with FF4FE. Yamls are attached (converted to .txt since github doesn't let you just upload .yamls).

@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Jul 4, 2025
Copy link
Contributor

@Rosalie-A Rosalie-A left a comment

Choose a reason for hiding this comment

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

Tested this with a world that places items offworld in pre_fill, and did not encounter any issues in a dozen generations.

@ScipioWright ScipioWright added the is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. label Jul 4, 2025
@Exempt-Medic
Copy link
Member

This violates expectations and the test_itempool_not_modified unit test. This should not be merged

@Exempt-Medic
Copy link
Member

https://github.com/ArchipelagoMW/Archipelago/blob/11130037fe9a88d8be70a1624c35aea17daab680/docs/world%20api.md#generation

create_items(self) called to place player's items into the MultiWorld's itempool. By the end of this step all regions, locations and items have to be in the MultiWorld's regions and itempool. You cannot add or remove items, locations, or regions after this step. Locations cannot be moved to different regions after this step. This includes event items and locations.

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