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: Add support for non dictionary iterables for Region.add_exits #1698

Merged
merged 6 commits into from
Jul 1, 2023

Conversation

alwaysintreble
Copy link
Collaborator

What is this fixing or adding?

Missed this use case when creating these helpers and didn't notice until trying to convert The Messenger :)

How was this tested?

unit tests

@ThePhar ThePhar added is: enhancement Issues requesting new features or pull requests implementing new features. affects: core Issues/PRs that touch core and may need additional validation. labels May 31, 2023
Copy link
Collaborator

@el-u el-u left a comment

Choose a reason for hiding this comment

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

The two codepaths are basically identical, and this duplication should be eliminated. One way to achieve this could be to keep only the codepath for the dict case and upconvert non-dict input using

if not isinstance(exits, dict):
    exits = dict.fromkeys(exits)

(Another way would be to extract the shared code from inside the for loops into a separate method.)

BaseClasses.py Outdated Show resolved Hide resolved
BaseClasses.py Outdated Show resolved Hide resolved
BaseClasses.py Outdated Show resolved Hide resolved
BaseClasses.py Outdated Show resolved Hide resolved
BaseClasses.py Outdated Show resolved Hide resolved
BaseClasses.py Outdated Show resolved Hide resolved
@ThePhar ThePhar merged commit 9b15278 into ArchipelagoMW:main Jul 1, 2023
@alwaysintreble alwaysintreble deleted the region_helpers_oversight branch July 19, 2023 21:46
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
…ArchipelagoMW#1698)

* Core: Add support for non dictionary iterables for `Region.add_exits`

* some cleanup and duplicate code removal

* add unit test for non dict iterable

* use more consistent naming

* sometimes i just make stuff harder on myself :)
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
…ArchipelagoMW#1698)

* Core: Add support for non dictionary iterables for `Region.add_exits`

* some cleanup and duplicate code removal

* add unit test for non dict iterable

* use more consistent naming

* sometimes i just make stuff harder on myself :)
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: enhancement Issues requesting new features or pull requests implementing new features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants