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

Tests: minor: update tests base for Options API #2516

Merged
merged 5 commits into from
Jun 18, 2024
Merged

Tests: minor: update tests base for Options API #2516

merged 5 commits into from
Jun 18, 2024

Conversation

chesslogic
Copy link
Contributor

What is this fixing or adding?

Currently, self.multiworld.accessibility[1] is being compared to a string. I believe this property is defined as a member of Options.Accessibility, so I changed it to reference the enumeration.

How was this tested?

I ran the test suite.

If this makes graphical changes, please attach screenshots.

No graphical changes.

@alwaysintreble
Copy link
Collaborator

  1. String comparisons to options are completely valid
  2. What you're comparing it to is not an enum; it's the option class's attribute
  3. The actual "bug" here is that it's using the old options system instead of the new one. Should be self.multiworld.worlds[1].options.accessibility

@ThePhar ThePhar added is: refactor/cleanup Improvements to code/output readability or organizization. affects: core Issues/PRs that touch core and may need additional validation. labels Nov 26, 2023
@black-sliver black-sliver mentioned this pull request Jan 31, 2024
Merged
@PoryGone PoryGone added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Feb 11, 2024
qwint
qwint reviewed Mar 7, 2024
View reviewed changes
test/bases.py Outdated Show resolved Hide resolved
@Exempt-Medic Exempt-Medic added the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label Mar 28, 2024
Exempt-Medic
Exempt-Medic approved these changes Jun 16, 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.

LGTM

@Exempt-Medic Exempt-Medic 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 Jun 16, 2024
@NewSoupVi NewSoupVi merged commit 67a0a04 into ArchipelagoMW:main Jun 18, 2024
3 checks passed
sflavelle pushed a commit to sflavelle/Archipelago-tgc that referenced this pull request Jun 20, 2024
@chesslogic @sflavelle
Tests: minor: update tests base for Options API (ArchipelagoMW#2516)
602b0de 
* update tests for Options API

* The actual "bug"

* resolve qwint's comment from 3 months ago
qwint pushed a commit to qwint/Archipelago that referenced this pull request Jun 24, 2024
@chesslogic @qwint
Tests: minor: update tests base for Options API (ArchipelagoMW#2516)
2bd581b 
* update tests for Options API

* The actual "bug"

* resolve qwint's comment from 3 months ago
GameWyrm pushed a commit to GameWyrm/Archipelago-GW that referenced this pull request Jul 4, 2024
@chesslogic @GameWyrm
Tests: minor: update tests base for Options API (ArchipelagoMW#2516)
90809f9 
* update tests for Options API

* The actual "bug"

* resolve qwint's comment from 3 months ago
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qwint qwint qwint left review comments

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

@NewSoupVi NewSoupVi NewSoupVi approved these changes

@alwaysintreble alwaysintreble alwaysintreble approved these changes

Assignees
No one assigned
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: refactor/cleanup Improvements to code/output readability or organizization. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@chesslogic @alwaysintreble @qwint @NewSoupVi @Exempt-Medic @ThePhar @PoryGone