Core: Add connect_entrances world step/stage

[NewSoupVi]

There is a lot of history for this, but basically, for Item Plando (using the upcoming rewrite) and Generic Entrance Randomizer to work well together, there is a specific timing window that has to be satisfied. The easiest way to do this is to add a new connect_entrances world stage.

When entrance source/target regions have to be finalized is also currently undefined, which has lead to some minor debate. This should prevent this from being up for discussion in the future.

[NewSoupVi]

I was gonna add this to The Witness as part of this PR, but unfortunately, my entrances currently need to be connected before this step because I add extra locations in create_regions based on the outcome of a state sweep.

[BadMagic100]

Code and docs look good, did not do any real testing

[NewSoupVi]

Got permission from @gaithern to change KH1, the simplest apworld that can easily use this, over to this system.
(An approval here would still be nice ofc :3)

[BadMagic100]

Came back to this pr in passing and was thinking it would be nice if this one can be expedited, I think it is highly beneficial to have the docs updated before more people start putting time into ger implementations

In particular I take mild issue with the inclusion of the world change here given that we essentially have the requisite peer reviews (qwint could be poked again since it looks like feedback is addressed) but now there an additional non-critical world change requiring an additional review

[NewSoupVi]

Came back to this pr in passing and was thinking it would be nice if this one can be expedited, I think it is highly beneficial to have the docs updated before more people start putting time into ger implementations

In particular I take mild issue with the inclusion of the world change here given that we essentially have the requisite peer reviews (qwint could be poked again since it looks like feedback is addressed) but now there an additional non-critical world change requiring an additional review

I figured getting permission from gaithern to make this change is enough, which is why I didn't add the "waiting on: world maintainer" label. https://discord.com/channels/731205301247803413/731214280439103580/1324181076087804005

Although yes I was hoping that they'd also see the ping here and approve haha

[gaithern]

Seems simple enough to me, I'm guessing there's no changes to functionality just written slightly differently right?

[NewSoupVi]

(Re: BadMagic) Wdym about qwint tho? They didn't put anything on this PR. You mean on Discord?

[NewSoupVi]

Seems simple enough to me, I'm guessing there's no changes to functionality just written slightly differently right?

Yeah. :) Just moving the entrance connection to the new connect_entrances step.

[NewSoupVi]

Nice, thank you!
Re BadMagic: Problem solved, doesn't count as "second review" though, since they were only asked to review the very minimal changes to KH1

[qwint]

changes look reasonable and seeeem applied in all the places they need to be,
ran 3.11 and 3.12 tests locally and everything passed even with the hk1 changes

also not a full test but I moved my minit ER to connect_entrances and I at least didn't fail any basic tests in my ER test, but I've been a bit behind on GER updates (and I didn't actually play any seeds with that upate) so it's not that big of a datapoint

[BadMagic100]

(Re: BadMagic) Wdym about qwint tho? They didn't put anything on this PR. You mean on Discord?

I thought qwint reviewed but it seems that it was only comments on my comments lol (but now that also appears resolved)

[qwint]

iirc I did have a comment in discord about the step order which may be what you were also thinking of lol

[NewSoupVi]

Took me 4 commits but I got there

[Berserker66]

Pretty sure once a world uses this it breaks Universal Tracker again 👍