Core: move PlandoConnections and PlandoTexts to the options system

[Silvris]

What is this fixing or adding?

Moves plando connections and texts to the options system. In order to use plando connections and texts, worlds will need to define the option in their options dataclass/definitions, and supported games will have plando connections and texts appear within the template yaml. Additionally, rolled plando connections/texts will appear in the spoiler log.

This includes changes to LttP, OoT, Minecraft, and TUNIC as they use plando connections currently (or plan to in the near future).

How was this tested?

Manually, by generating games using plando connections and confirming that the resulting connection was proper.

Texts was tested by generating a game using the mercenary mode trigger block, and confirming that the text was correctly placed.

No automated testing has been added.

If this makes graphical changes, please attach screenshots.

Excerpt from LttP template:

  plando_connections:
    # Generic connections plando. Format is:
    # - entrance: "Entrance Name"
    #   exit: "Exit Name"
    #   direction: "Direction"
    #   percentage: 100
    # Direction must be one of 'entrance', 'exit', or 'both', and defaults to 'both' if omitted.
    # Percentage is an integer from 1 to 100, and defaults to 100 when omitted.
    []

  plando_texts:
    # Text plando. Format is:
    # - text: 'This is your text'
    #   at: text_key
    #   percentage: 100
    # Percentage is an integer from 1 to 100, and defaults to 100 when omitted.
    []

Excerpt from spoiler log:

Plando Connections:              Eastern Palace => Tower of Hera (Bottom)
Plando Texts:                    {'kakariko_tavern_fisherman': 'hey @\nYour mark schemes within a den of evil.', 'sign_ganon': "A hit has been put out on one of Ganon's generals."}

[Silvris]

@espeon65536 @silent-destroyer

[ScipioWright]

I'm the one doing the ER and connection plando stuff for Tunic, so I'm approving this in Silent's stead.
I currently have this PR up for implementing connection plando: #2864
If the PR I have up gets merged first, I'll PR to your branch to fix any merge conflicts/errors. And if your PR gets merged first, I'll update mine accordingly.

Well, approving the Tunic change anyway. I'll try to remember to actually review this tonight -- it looked fine already just skimming the code, but I want to do test gens and such for an actual review.

[Silvris]

Updated for KDL3.

[alwaysintreble]

code looks fine other than some style gripes. i'm not really sure some of these typing hints are useful since they have Any in them and it might be better to force/assert them as the correct type at some point instead? and then i think two implementation detail questions i had.

[alwaysintreble]

resubmitting review from earlier because my comment lines got fucked up for some reason

[alwaysintreble]

are we sure this should be the default behavior? it seems like enough of the worlds that use this would be fine with it (and my world is too) but it almost feels backwards.

[Silvris]

It primarily does two things.

As you've said, it's sufficient for worlds that don't have wild constraints on possible connections. It also simplifies backward compatibility with the currently implemented games, as they already have to check for whether or not the plando is a valid connection, so we can skip the option-based check (the exception being Minecraft, as it's can_connect was simple enough to implement on the option itself).

[alwaysintreble]

if this is going to be a scoped class like this it should probably be in the PlandoConnection one instead of here so that the direction can be correctly typed?

[beauxq]

I agree with this.

But also, maybe an Enum?

(The downside to making it an Enum is the long definition... PlandoConnection.Direction.both compared to "both")

So another alternative is no class, just a type alias:

PlandoConnectionDirection = typing.Literal["entrance", "exit", "both"]


class PlandoConnection(typing.NamedTuple):
    entrance: str
    exit: str
    direction: PlandoConnectionDirection
    percentage: int = 100

(I think a type alias has to be gloabal scope.)

[Silvris]

Once 3.8 is dropped, StrEnum would probably be a good option. Not sure what the best option is for right now.

[beauxq]

I don't see what value StrEnum would have over Enum for this.
The values in a normal Enum can be str.
StrEnum is for when you want to pass it to a function that only takes str, or use str methods like lower, but I don't see you doing any of that here.

[beauxq]

It almost seems better to make the values:

class Direction(Enum):
    ENTRANCE = "=>"
    EXIT = "<="
    BOTH = "<=>"

so that you could use those values in get_option_name (and simplify the code there)

[alwaysintreble]

the issue with that is that the existing implementations specifically compare the string when resolving the connections, so it'd need to be an alias of some sort.

[beauxq]

Ok, I didn't realize this was connecting with past implementations.

[Berserker66]

Scrolled over it, looks fine. Didn't test anything.

[Silvris]

With this current system, I can't do self.options.plando_connections.remove(connection), nor can I do self.options.plando_connections.exit = "Shop Portal", nor can I do self.options.plando_connections[index] = replacement, so as far as I can tell there's no way for me to modify the player's plando connections like I used to.

self.options.plando_connections.value.remove(cxn). That being said, you will need to explicitly define "Shop" as a valid entrance/exit.

[alwaysintreble]

With this current system, I can't do self.options.plando_connections.remove(connection), nor can I do self.options.plando_connections.exit = "Shop Portal", nor can I do self.options.plando_connections[index] = replacement, so as far as I can tell there's no way for me to modify the player's plando connections like I used to.

self.options.plando_connections.value.remove(cxn). That being said, you will need to explicitly define "Shop" as a valid entrance/exit.

is there a reason they don't just inherit from OptionList? it has the necessary overrides to do exactly as requested here

[Silvris]

is there a reason they don't just inherit from OptionList? it has the necessary overrides to do exactly as requested here

This code would also fail on an OptionList, as it does not override explicit "insert" and "remove", nor does it inherit it from Option or VerifyKeys.