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

The Messenger: Use fill hook to help prevent early swap entering and failures #2467

Closed

Conversation

alwaysintreble
Copy link
Collaborator

@alwaysintreble alwaysintreble commented Nov 16, 2023

What is this fixing or adding?

https://discord.com/channels/731205301247803413/731214280439103580/1174536333818089482
Basically it's super uncommon and unlikely, but it's possible for the itempool to be shuffled in such a way where both of a player's main movement items get shuffled to the front of the queue, causing them to get placed early and block off most of the multiworld as valid placements. This is a super complicated fix to prevent that situation by moving one of the items but only if both of that players' items are near the very beginning of the queue. The only other solution I thought of is to use the early_items API and force it, but that would require me to do that always which I don't like so this is the path I took. Open to alternatives, but unable to think of something that doesn't just strictly rely on swap.

How was this tested?

tested with following yaml and seed 32514211167490115829

game: The Messenger
The Messenger:
  shuffle_shards: true
  shop_price: 304

which fails without this change, and succeeds with this change.

@el-u
Copy link
Collaborator

el-u commented Nov 16, 2023

OK, so obviously I hate it. It's basically the same that SA2B is doing, only less brutish.

  • I think that things could get quite messy if more worlds started doing stuff like this, therefore ideally no world should do it.
  • The best solution would be to change the swap algorithm to handle this situation, but so far noone seems to know how to do that.
  • That aside, it still might be worth investigating this failure type further; maybe something new about swap errors can be learned after all.
  • Do you have any idea about the relation of "seeds that would fail without this change" and "seeds that trigger this mechanism"?
  • While I don't doubt that this change could help generation chance, the observation of "seed 32514211167490115829 works with this change" is not really a relevant indicator since reordering the itempool completely changes the meaning of the seed anyway.
  • Is there any indication that this problem might be confined to single player generations? In that case a workable solution might be to force exactly one of these items into early items if and only if the number of players is one.
  • In essence, you are trying to invent non_late_items here, but the mechanism is not generic. Can it be? Should it be?
    (In some sense, this goes against my first bullet since making it an official API would of course encourage more worlds to use it, which I'd consider bad;
    but on the other hand, having it handled for multiple worlds all in one go in a central place would be strongly preferable to individual worlds doing it separately and in different ways, which I consider worse.)

@alwaysintreble
Copy link
Collaborator Author

Kind of the issue I think is that I like when these items are later, it's just when both of them are too late that it becomes a problem, so I'm not really a fan of non_late_items. I don't know if this occurs with multiple players, and it's also likely that this is also only an issue when the shop cost is higher in combination with these conditions. This problem is so nebulous that it's hard to fix. While swap fixing it might might be a better solution, the way that it works out would require swapping to start very early on during fill, which would negatively affect it by quite a lot as that's slow. Then it would just place the problematic item again and need to swap again over and over, which is what's causing this failure. Maybe we have a mechanism such that when generic fill places an item, it checks the count of valid locations and bails out that previous item, pushing it further back into the queue?

@ScootyPuffJr1 ScootyPuffJr1 added the is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. label Nov 17, 2023
@alwaysintreble
Copy link
Collaborator Author

swapping this to a draft since i don't really want it merged but i deem the info and investigation done here useful

@alwaysintreble alwaysintreble marked this pull request as draft February 14, 2024 00:39
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants