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.

Sign up for GitHub

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

Jump to bottom

The Witness: Add more panels to the "doors: panels" mode #2916

Merged
merged 14 commits into from
Dec 10, 2024
Merged

The Witness: Add more panels to the "doors: panels" mode #2916

merged 14 commits into from
Dec 10, 2024

Conversation

NewSoupVi
Copy link
Member

@NewSoupVi NewSoupVi commented Mar 7, 2024 •
edited
Loading

There were more door panels that were kinda weird not to have items.
This adds those to both the individual panels and regional panels modes.

New client: https://github.com/NewSoupVi/The-Witness-Randomizer-for-Archipelago/releases/tag/v5.0.0p16

Tested:
A little

@NewSoupVi NewSoupVi added the is: enhancement Issues requesting new features or pull requests implementing new features. label Mar 7, 2024
@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Mar 7, 2024
@NewSoupVi NewSoupVi marked this pull request as draft March 7, 2024 22:49
@NewSoupVi
Copy link
Member Author

A weird edge case was found where two door items will exist for the same door (not in a buggy way, but in a confusing way). I'll fix that

@NewSoupVi NewSoupVi marked this pull request as ready for review March 8, 2024 00:48
Exempt-Medic
Exempt-Medic approved these changes Mar 9, 2024
View reviewed changes
Copy link
Member

@Exempt-Medic Exempt-Medic left a comment

Choose a reason for hiding this comment

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

The changes all LGTM code-wise. I checked that the correct panel items were created under all twenty-four option-combinations and generated 1000 random games just to be safe.

@Exempt-Medic Exempt-Medic mentioned this pull request Apr 10, 2024
Closed
@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 added waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. and removed waiting-on: author Issue/PR is waiting for feedback or changes from its author. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Apr 11, 2024
@NewSoupVi NewSoupVi added waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. and removed waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. labels Apr 11, 2024
@NewSoupVi NewSoupVi mentioned this pull request Jun 24, 2024
Open
@NewSoupVi NewSoupVi marked this pull request as draft July 6, 2024 13:15
@NewSoupVi
Copy link
Member Author

Drafting because I want to add a unit test for this

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

Unit test has been added, this is ready for review again

@Exempt-Medic
Copy link
Member

Conflicts

@Exempt-Medic Exempt-Medic added the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label Dec 6, 2024
@NewSoupVi NewSoupVi removed the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label Dec 7, 2024
@NewSoupVi
Copy link
Member Author

Conflict resolved

@NewSoupVi NewSoupVi merged commit 9a37a13 into ArchipelagoMW:main Dec 10, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Exempt-Medic Exempt-Medic Exempt-Medic approved these changes

Assignees
No one assigned
Labels
is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@NewSoupVi @Exempt-Medic @Berserker66