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 <beauxq@yahoo.com>
Co-authored-by: black-sliver <59490463+black-sliver@users.noreply.github.com>

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
 

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
+        def verify(self, world: typing.Type[World], player_name: str, plando_options: PlandoOptions) -> None:
-        from worlds.AutoWorld import World
-
-        def verify(self, world: 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 super().current_key
-            return self.name_lookup[self.value]
 
     @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()

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)

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)