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

Core: Minimal-Items Accessibility Fix #1888

Merged
merged 7 commits into from
Feb 13, 2024

Conversation

Alchav
Copy link
Contributor

@Alchav Alchav commented Jun 24, 2023

What is this fixing or adding?

The promises of locations accessibility ensuring "everything can be reached and acquired" and items accessibility ensuring "all logically relevant items can be acquired" imply that an items player may have locations they cannot reach, but that their own advancement items will always be reachable.

This is largely what the accessibility check ensures, except that if an items player has locations that are not reachable, a minimal players' unnecessary advancement items may be placed in the items player's unreachable locations and this will cause the accessibility check to fail.

Currently the Dexsanity option in Pokémon Red and Blue allows for logically inaccessible locations. In some cases they may be locations that you can in fact reach or that you may have to make a specific choice in order to reach, but need to be out of logic because it may be possible to permanently miss them. Right now this option forces accessibility off of locations (unless other options are set specifically that will prevent any from being out of logic) but in my WIP update, I am instead deleting the unreachable locations if locations accessibility is chosen.

This PR tweaks the accessibility check to allow items players' locations to contain minimal players' advancement items

How was this tested?

After seeing an accessibility check failure due to a minimal player's advancement item landing on an inaccessible items player's location, re-generated with same seed using this change.

@ThePhar ThePhar added is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. affects: core Issues/PRs that touch core and may need additional validation. labels Jun 25, 2023
@ThePhar
Copy link
Member

ThePhar commented Jun 29, 2023

Got an example yaml I can get it to reproduce? I can try to run through this later.

@Alchav
Copy link
Contributor Author

Alchav commented Jun 30, 2023

Got an example yaml I can get it to reproduce? I can try to run through this later.

With --seed 1

name: a
game: Pokemon Red and Blue
Pokemon Red and Blue:
  accessibility: minimal
  progression_balancing: disabled
  start_inventory: {HM03 Surf: 1, HM04 Strength: 1, Soul Badge: 1, Rainbow Badge: 1}
  badgesanity: on
  elite_four_condition: 0
  victory_road_condition: 0
---
name: b
game: Pokemon Red and Blue
Pokemon Red and Blue:
  accessibility: items
  progression_balancing: disabled
  start_inventory: {HM03 Surf: 1, HM04 Strength: 1, Soul Badge: 1, Rainbow Badge: 1}
  badgesanity: on
  elite_four_condition: 0
  victory_road_condition: 0
  dexsanity: on

@agilbert1412 agilbert1412 added the waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. label Feb 10, 2024
@Berserker66 Berserker66 merged commit 6f3bc3a into ArchipelagoMW:main Feb 13, 2024
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
TheLX5 pushed a commit to TheLX5/Archipelago that referenced this pull request Mar 2, 2024
TheLX5 pushed a commit to TheLX5/Archipelago that referenced this pull request Mar 2, 2024
EmilyV99 pushed a commit to EmilyV99/Archipelago that referenced this pull request Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants