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

fix some issues #4

Merged
merged 1 commit into from
Mar 6, 2024
Merged

Conversation

beauxq
Copy link

@beauxq beauxq commented Mar 5, 2024

What is this fixing or adding?

The from_any type annotation is usually Any, but this type alias is for better narrowing of the generics.

The default typing has been explored and discussed ArchipelagoMW#2173 and ArchipelagoMW#2899
and it looks like no type annotation is the best we can do for it. (It will inherit the type from the base class. Some type checkers will infer the type for better checking.)

The tuple instead of list protects against global mutability bugs we've seen before.
__init__ can turn the tuple into a list

I don't know if you want to do error checking instead of the text.get("text", "") default empty string.

get_option_name is a classmethod in the superclass. I'm not sure whether you were trying to override that or make a new instance method. I assumed override.

In general, you should prefer isinstance(x, ...) over type(x) (especially when this code is designed for inheritance).

How was this tested?

just unit tests

Silvris pushed a commit that referenced this pull request Mar 5, 2024
* Cleaning up (#4)

Cleanup

* Adressed change about spaces no longer being replaced to underscores.

Added a "that" to remove an ambiguity

* Update data/options.yaml

Combined the two sentences into one, per suggestion

Co-authored-by: Nicholas Saylor <[email protected]>

---------

Co-authored-by: Nicholas Saylor <[email protected]>
Silvris pushed a commit that referenced this pull request Mar 5, 2024
* Cleaning up (#4)

Cleanup

* Update landstalker_setup_en.md

Fixed Redirect
@Silvris Silvris merged commit 19b9c03 into Silvris:plando_options Mar 6, 2024
9 checks passed
Silvris pushed a commit that referenced this pull request Sep 6, 2024
* Cleaning up (#4)

Cleanup

* Changed channel name

* Changed channel name

* Update docs/world maintainer.md

* Update docs/world maintainer.md
Silvris added a commit that referenced this pull request Dec 31, 2024
* first pass

* move to a metaclass approach

* another concept for rabbits goals

* ffxvi and pizza tower
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants