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

Pokemon Emerald: Rework tags/dynamically create item and location groups #3263

Merged
merged 20 commits into from
Nov 29, 2024

Conversation

Zunawe
Copy link
Collaborator

@Zunawe Zunawe commented May 4, 2024

What is this fixing or adding?

Currently, items and locations are both assigned "tags" in the data files that define them. For items, these tags are used to balance the item pool and to avoid creating duplicates of items with the "Unique" tag, as duplicates would have no use to the player. For locations, these tags are used to enable and disable locations. Location tags are essentially used as an enum. All locations have exactly one tag that corresponds with whether they should be included in randomization based on some player option.

So this PR actually converts those existing location tags into a category enum. A script was run on worlds/pokemon_emerald/data/locations.json to map the tag to a category field, and to empty the tags list. And that category is used in the same way the tag was. Tags are also no longer copied to actual Locations in favor of a key that can be used to index into the dataclasses.

For both items and locations, tags are now converted directly into item and location groups in worlds/pokemon_emerald/groups.py. Some item tags were changed to look nicer to humans. Existing item groups were converted to tags and are no longer explicitly defined. The same is true for the "Gym TMs" location group, which is the only change made to worlds/pokemon_emerald/data/locations.json that wasn't done automatically.

For locations, location categories are converted to location groups in worlds/pokemon_emerald/groups.py by mapping the enum to a human-friendly label. Every in-game map is also associated with a group name, and since every location belongs to a region, and every region belongs to a map, it can automatically associate every location with a group representing some geographical area. (Pokedex locations are the odd ones out and have to be filtered; the locations themselves are "in Littleroot" because they're locked by events that get placed dynamically based on wild encounter randomization.)

A sanity check was added to make sure the maps being used for location groups exist. In case of changes made to the names that get automatically pulled into extracted_data.json, or accidental misspellings.

Thanks to @Tsukino-uwu for categorizing all the maps.

This PR supersedes the need for #3234.

How was this tested?

Generated a few times with a random yaml and while enabling/disabling options to check that the reported location count made sense. Also scrolled through the data package.

@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label May 4, 2024
@Zunawe Zunawe added the is: enhancement Issues requesting new features or pull requests implementing new features. label May 4, 2024
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 PR is most a refactor. Merged into main. Read through the datapackage and looked at the options pages on a local WebHost as well as adding in some groups and verifying that they worked properly. Had some question about group names/titles and the specific items included in some of them which were all answered. Though I think setdefault or defaultdicts could be nicer in groups.py it's not something I think is really needed. Compared spoiler logs before and after as well and ran 100 test generations.

@NewSoupVi NewSoupVi self-requested a review August 21, 2024 00:20
@NewSoupVi
Copy link
Member

MORE LOCATION AND ITEM GROUPS YESSSSSSSSSSSSSSSSSS I'll try to remember to review this soon

Copy link
Contributor

@nicholassaylor nicholassaylor left a comment

Choose a reason for hiding this comment

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

One minor comment, but everything appears to work correctly after generating a game and testing out hints in a CommonClient

worlds/pokemon_emerald/CHANGELOG.md Show resolved Hide resolved
@Exempt-Medic Exempt-Medic added waiting-on: author Issue/PR is waiting for feedback or changes from its author. and removed waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Nov 27, 2024
@Exempt-Medic Exempt-Medic 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 Nov 27, 2024
@NewSoupVi NewSoupVi merged commit 6f2464d into ArchipelagoMW:main Nov 29, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: author Issue/PR is waiting for feedback or changes from its author. 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.

5 participants