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

TUNIC: Location groups for each area of the game #3024

Merged
merged 24 commits into from
May 2, 2024

Conversation

ScipioWright
Copy link
Collaborator

@ScipioWright ScipioWright commented Mar 24, 2024

What is this fixing or adding?

Creates location groups for each area of the game, so every location is now in a second location group. We had this originally, but removed it because it was jank. But now it's not jank.

Also very very slight optimization in the hint code to use name = string.split(" - ", 1)[0] instead of name, _ = string.split

And scope creeping it a bit to add another item group for the wand, because I keep typing !hint wand and it keeps not working. Also adding an item group "fire rod" for the magic wand, since someone hinted that and fuzzy matching decided they were hinting for ice rod, which is icebolt, which is wrong

How was this tested?

Test gens, adding location groups to priority locations and seeing what the spoiler thinks the priority locations are.

If this makes graphical changes, please attach screenshots.

N/A

@ScipioWright ScipioWright requested a review from NewSoupVi March 24, 2024 13:34
@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Mar 24, 2024
@ScipioWright ScipioWright added the is: enhancement Issues requesting new features or pull requests implementing new features. label Mar 24, 2024
Copy link
Collaborator

@hatkirby hatkirby left a comment

Choose a reason for hiding this comment

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

Generated weighted-options.json and saw that the location groups do exist. LGTM.

@hatkirby hatkirby 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: peer-review Issue/PR has not been reviewed by enough people yet. labels Mar 26, 2024
Copy link
Member

@NewSoupVi NewSoupVi left a comment

Choose a reason for hiding this comment

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

Looks good to me. I don't like the pattern chosen for a dict of sets, I would prefer setdefault or defaultdict, but it's not breaking.

worlds/tunic/locations.py Outdated Show resolved Hide resolved
@NewSoupVi NewSoupVi merged commit c64c80a into ArchipelagoMW:main May 2, 2024
16 checks passed
@ScipioWright ScipioWright deleted the tunc-location-groups branch May 2, 2024 13:38
jnschurig pushed a commit to Tranquilite0/Archipelago-SoulBlazer that referenced this pull request Jun 13, 2024
* huzzah, location groups

* scope creep pog

* Apply suggestion to the other spot it is applicable at too

* apply berserker's suggestion

Co-authored-by: Fabian Dill <[email protected]>

* Remove extra location group for shops

* Fire rod for magic wand

* Capitalize itme name groups

* Update docs to capitalize item name groups, remove the little section on aliases

since the aliases bit is really more for someone misremembering the name than anything else, like "fire rod" is because you played a lot of LttP, or Orb instead of Magic Orb is clear.

* Fix rule with item group name

* Capitalization is cool

* Fix merge mistake

* Add Flask group, remove Potions group

* Update docs to detail how to find item and location groups

* Revise per Vi's comment

* Fix test

* fuzzy matching please stop

* Remove test change that was meant for a different branch

---------

Co-authored-by: Fabian Dill <[email protected]>
qwint pushed a commit to qwint/Archipelago that referenced this pull request Jun 24, 2024
* huzzah, location groups

* scope creep pog

* Apply suggestion to the other spot it is applicable at too

* apply berserker's suggestion

Co-authored-by: Fabian Dill <[email protected]>

* Remove extra location group for shops

* Fire rod for magic wand

* Capitalize itme name groups

* Update docs to capitalize item name groups, remove the little section on aliases

since the aliases bit is really more for someone misremembering the name than anything else, like "fire rod" is because you played a lot of LttP, or Orb instead of Magic Orb is clear.

* Fix rule with item group name

* Capitalization is cool

* Fix merge mistake

* Add Flask group, remove Potions group

* Update docs to detail how to find item and location groups

* Revise per Vi's comment

* Fix test

* fuzzy matching please stop

* Remove test change that was meant for a different branch

---------

Co-authored-by: Fabian Dill <[email protected]>
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: 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