From 414166f6a27d92cf9104b8fff900e180a8b3586c Mon Sep 17 00:00:00 2001 From: alwaysintreble Date: Tue, 7 Mar 2023 01:44:20 -0600 Subject: [PATCH] Core: Minor Options cleanup (#1182) * Options.py cleanup * TextChoice cleanup * make Option.current_option_name a property * title the TextChoce option names * satisfy the linter * a little more options cleanup * move the typing import * typing should be PlandoSettings * fix incorrect conflict merging * make imports local * the tests seem to want me to import these twice though i hate it. * changes from review. Make the various Location verifying Options `LocationSet` * remove unnecessary fluff * begrudgingly support get_current_option_name. Leave a comment that worlds shouldn't be touching this * log a deprecation warning and return the property for `get_current_option_name()` --------- Co-authored-by: beauxq Co-authored-by: black-sliver <59490463+black-sliver@users.noreply.github.com> --- BaseClasses.py | 2 +- Options.py | 66 ++++++++++--------- worlds/alttp/test/options/TestPlandoBosses.py | 10 +-- worlds/zillion/__init__.py | 2 +- 4 files changed, 43 insertions(+), 37 deletions(-) diff --git a/BaseClasses.py b/BaseClasses.py index 16a06005..11eb1f87 100644 --- a/BaseClasses.py +++ b/BaseClasses.py @@ -1261,7 +1261,7 @@ class Spoiler(): res = getattr(self.multiworld, option_key)[player] display_name = getattr(option_obj, "display_name", option_key) try: - outfile.write(f'{display_name + ":":33}{res.get_current_option_name()}\n') + outfile.write(f'{display_name + ":":33}{res.current_option_name}\n') except: raise Exception diff --git a/Options.py b/Options.py index 10add11b..5ce0901a 100644 --- a/Options.py +++ b/Options.py @@ -1,5 +1,6 @@ from __future__ import annotations import abc +import logging from copy import deepcopy import math import numbers @@ -9,6 +10,10 @@ import random from schema import Schema, And, Or, Optional from Utils import get_fuzzy_results +if typing.TYPE_CHECKING: + from BaseClasses import PlandoOptions + from worlds.AutoWorld import World + class AssembleOptions(abc.ABCMeta): def __new__(mcs, name, bases, attrs): @@ -95,11 +100,11 @@ class Option(typing.Generic[T], metaclass=AssembleOptions): supports_weighting = True # filled by AssembleOptions: - name_lookup: typing.Dict[int, str] + name_lookup: typing.Dict[T, str] options: typing.Dict[str, int] def __repr__(self) -> str: - return f"{self.__class__.__name__}({self.get_current_option_name()})" + return f"{self.__class__.__name__}({self.current_option_name})" def __hash__(self) -> int: return hash(self.value) @@ -109,7 +114,14 @@ class Option(typing.Generic[T], metaclass=AssembleOptions): return self.name_lookup[self.value] def get_current_option_name(self) -> str: - """For display purposes.""" + """Deprecated. use current_option_name instead. TODO remove around 0.4""" + logging.warning(DeprecationWarning(f"get_current_option_name for {self.__class__.__name__} is deprecated." + f" use current_option_name instead. Worlds should use {self}.current_key")) + return self.current_option_name + + @property + def current_option_name(self) -> str: + """For display purposes. Worlds should be using current_key.""" return self.get_option_name(self.value) @classmethod @@ -131,17 +143,14 @@ class Option(typing.Generic[T], metaclass=AssembleOptions): ... if typing.TYPE_CHECKING: - from Generate import PlandoOptions - from worlds.AutoWorld import World - - def verify(self, world: World, player_name: str, plando_options: PlandoOptions) -> None: + def verify(self, world: typing.Type[World], player_name: str, plando_options: PlandoOptions) -> None: pass else: def verify(self, *args, **kwargs) -> None: pass -class FreeText(Option): +class FreeText(Option[str]): """Text option that allows users to enter strings. Needs to be validated by the world or option definition.""" @@ -162,7 +171,7 @@ class FreeText(Option): return cls.from_text(str(data)) @classmethod - def get_option_name(cls, value: T) -> str: + def get_option_name(cls, value: str) -> str: return value @@ -424,6 +433,7 @@ class Choice(NumericOption): class TextChoice(Choice): """Allows custom string input and offers choices. Choices will resolve to int and text will resolve to string""" + value: typing.Union[str, int] def __init__(self, value: typing.Union[str, int]): assert isinstance(value, str) or isinstance(value, int), \ @@ -434,8 +444,7 @@ class TextChoice(Choice): def current_key(self) -> str: if isinstance(self.value, str): return self.value - else: - return self.name_lookup[self.value] + return super().current_key @classmethod def from_text(cls, text: str) -> TextChoice: @@ -450,7 +459,7 @@ class TextChoice(Choice): def get_option_name(cls, value: T) -> str: if isinstance(value, str): return value - return cls.name_lookup[value] + return super().get_option_name(value) def __eq__(self, other: typing.Any): if isinstance(other, self.__class__): @@ -573,12 +582,11 @@ class PlandoBosses(TextChoice, metaclass=BossMeta): def valid_location_name(cls, value: str) -> bool: return value in cls.locations - def verify(self, world, player_name: str, plando_options) -> None: + def verify(self, world: typing.Type[World], player_name: str, plando_options: "PlandoOptions") -> None: if isinstance(self.value, int): return - from Generate import PlandoOptions + from BaseClasses import PlandoOptions if not(PlandoOptions.bosses & plando_options): - import logging # plando is disabled but plando options were given so pull the option and change it to an int option = self.value.split(";")[-1] self.value = self.options[option] @@ -716,7 +724,7 @@ class VerifyKeys: value: typing.Any @classmethod - def verify_keys(cls, data): + def verify_keys(cls, data: typing.List[str]): if cls.valid_keys: data = set(data) dataset = set(word.casefold() for word in data) if cls.valid_keys_casefold else set(data) @@ -725,7 +733,7 @@ class VerifyKeys: raise Exception(f"Found unexpected key {', '.join(extra)} in {cls}. " f"Allowed keys: {cls.valid_keys}.") - def verify(self, world, player_name: str, plando_options) -> None: + def verify(self, world: typing.Type[World], player_name: str, plando_options: "PlandoOptions") -> None: if self.convert_name_groups and self.verify_item_name: new_value = type(self.value)() # empty container of whatever value is for item_name in self.value: @@ -830,7 +838,9 @@ class OptionSet(Option[typing.Set[str]], VerifyKeys): return item in self.value -local_objective = Toggle # local triforce pieces, local dungeon prizes etc. +class ItemSet(OptionSet): + verify_item_name = True + convert_name_groups = True class Accessibility(Choice): @@ -866,11 +876,6 @@ common_options = { } -class ItemSet(OptionSet): - verify_item_name = True - convert_name_groups = True - - class LocalItems(ItemSet): """Forces these items to be in their native world.""" display_name = "Local Items" @@ -892,22 +897,23 @@ class StartHints(ItemSet): display_name = "Start Hints" -class StartLocationHints(OptionSet): +class LocationSet(OptionSet): + verify_location_name = True + + +class StartLocationHints(LocationSet): """Start with these locations and their item prefilled into the !hint command""" display_name = "Start Location Hints" - verify_location_name = True -class ExcludeLocations(OptionSet): +class ExcludeLocations(LocationSet): """Prevent these locations from having an important item""" display_name = "Excluded Locations" - verify_location_name = True -class PriorityLocations(OptionSet): +class PriorityLocations(LocationSet): """Prevent these locations from having an unimportant item""" display_name = "Priority Locations" - verify_location_name = True class DeathLink(Toggle): @@ -948,7 +954,7 @@ class ItemLinks(OptionList): pool |= {item_name} return pool - def verify(self, world, player_name: str, plando_options) -> None: + def verify(self, world: typing.Type[World], player_name: str, plando_options: "PlandoOptions") -> None: link: dict super(ItemLinks, self).verify(world, player_name, plando_options) existing_links = set() diff --git a/worlds/alttp/test/options/TestPlandoBosses.py b/worlds/alttp/test/options/TestPlandoBosses.py index a6c3485f..83c1510a 100644 --- a/worlds/alttp/test/options/TestPlandoBosses.py +++ b/worlds/alttp/test/options/TestPlandoBosses.py @@ -1,5 +1,5 @@ import unittest -import Generate +from BaseClasses import PlandoOptions from Options import PlandoBosses @@ -123,14 +123,14 @@ class TestPlandoBosses(unittest.TestCase): regular = MultiBosses.from_any(regular_string) # plando should work with boss plando - plandoed.verify(None, "Player", Generate.PlandoOptions.bosses) + plandoed.verify(None, "Player", PlandoOptions.bosses) self.assertTrue(plandoed.value.startswith(plandoed_string)) # plando should fall back to default without boss plando - plandoed.verify(None, "Player", Generate.PlandoOptions.items) + plandoed.verify(None, "Player", PlandoOptions.items) self.assertEqual(plandoed, MultiBosses.option_vanilla) # mixed should fall back to mode - mixed.verify(None, "Player", Generate.PlandoOptions.items) # should produce a warning and still work + mixed.verify(None, "Player", PlandoOptions.items) # should produce a warning and still work self.assertEqual(mixed, MultiBosses.option_shuffle) # mode stuff should just work - regular.verify(None, "Player", Generate.PlandoOptions.items) + regular.verify(None, "Player", PlandoOptions.items) self.assertEqual(regular, MultiBosses.option_shuffle) diff --git a/worlds/zillion/__init__.py b/worlds/zillion/__init__.py index 78e20ce7..241cb452 100644 --- a/worlds/zillion/__init__.py +++ b/worlds/zillion/__init__.py @@ -255,7 +255,7 @@ class ZillionWorld(World): group_players = group["players"] start_chars = cast(Dict[int, ZillionStartChar], getattr(multiworld, "start_char")) players_start_chars = [ - (player, start_chars[player].get_current_option_name()) + (player, start_chars[player].current_option_name) for player in group_players ] start_char_counts = Counter(sc for _, sc in players_start_chars)