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 Witness: New solution to that plando issue #3041

Closed
wants to merge 24 commits into from

Conversation

NewSoupVi
Copy link
Member

@NewSoupVi NewSoupVi commented Mar 27, 2024

History

The Witness wants to do one simple thing.
"Pre-place a random symbol item onto the first check in the game, so that the player has things to do locally and sphere 1 has a bit of variety."
It should be simple, right? Just fill the item during create_items or something.
Well, it would be simple. If it weren't for plando.

Here's the problem. If a player uses plando_items with one of the eligible symbol items and uses from_pool: true, now their generation will inexplicably crash some of the time. Reason: That symbol item might already be gone from the itempool.

The current solution

We first check whether the item will be plandoed before placing it, before plando ever "resolves".
This is terrible, because people can have random plando. Also, plando blocks can take many different shapes.
Look at the code removed in items.py, then think about the fact that other worlds are starting to copy that code.

The new solution

Place this item during fill_hook. This basically uses the current plando code, but modified.

@NewSoupVi NewSoupVi changed the title New solution to that plando issue The Witness: New solution to that plando issue Mar 27, 2024
@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Mar 27, 2024
@NewSoupVi NewSoupVi added the is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. label Mar 27, 2024
@Exempt-Medic Exempt-Medic added the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label Apr 11, 2024
@NewSoupVi NewSoupVi removed the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label Apr 11, 2024
@NewSoupVi
Copy link
Member Author

Changed this to using pre_fill because I learned that fill_hook is after distribute_early_items

@NewSoupVi
Copy link
Member Author

Nvm

@NewSoupVi NewSoupVi force-pushed the that_plando_thing_again branch from 9b4f6c2 to c360f38 Compare May 13, 2024 21:13
@NewSoupVi NewSoupVi marked this pull request as draft July 6, 2024 13:17
@NewSoupVi
Copy link
Member Author

Drafting this for two reasons:

  • I wanna add unit tests for this.
  • I want to check for location.excluded.

@NewSoupVi NewSoupVi marked this pull request as ready for review July 26, 2024 20:24
@NewSoupVi
Copy link
Member Author

Undrafting because I've added a unit test and made it more robust in terms of things like excluded 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