Core: An attempt at fixing the new connect_entrances bug

[NewSoupVi]

In #4420, we added a new step called connect_entrances.
The description of this step reads: "By the end of this step, all Entrances must be connected".

However, there are some worlds that are relying on other worlds not having any dangling entrances in set_rules (which is before connect_entrances).
The culprit here is get_all_state, which does region sweeps on all worlds.

This is my attempt at a fix. Only 1 line needs to be changed in Factorio and ALTTP each, but I also wanted to add protection against this happening to unsupported worlds.

This PR also serves as the "release blocker" PR, but this label does not mean this particular implementation is what we want to go with

Other options:

1. Don't do this with an assert, do it with a unit test (Counter argument: This would only catch default options)
2. Change the documentation of connect_entrances to say that Entrances must be connected somehow after create_items, but their final connection must be done by connect_entrances
3. Revert the connect_entrances PR for now? But it's very necessary for GER and plando rewrite

[NewSoupVi]

I'm a bit uncertain of the best approach, for now I wanted something open that passes unit tests, fixes the issue, and can carry the "release blocker" label. What I mostly hope to get from other core maintainers or ppl like alwaysintreble or Mysteryem is opinions on the approach.

We can also just revert #4420 for now. I just want to stress though that connect_entrances is GIGA important for upcoming GER / plando changes, to the point where I basically don't wanna merge any GER implementations until connect_entrances exists

[BadMagic100]

is an enforcement mechanism even needed here? If we fix it everywhere it's relevant then going forward the docs which state entrances must be connected only by connect_entrances should imply that worlds need to worry about it

Assuming there will be one anyway I don't have an opinion on what it should be

[BadMagic100]

This definitely feels super hacky, I would rather try to update the tests if relevant than do this and shoot ourselves in the foot in the future somehow

[BadMagic100]

(specifically referring to the generate_basic part of the assertion fwiw)

[Exempt-Medic]

I said a similar thing in DMs, but gonna state here that I fully agree with BadMagic's comment

[BadMagic100]

I also feel that option 2 (make dummy connections) also seems quite bad.

It would probably be helpful to know why they care about other worlds' region graph - do they actually use it or is it just the state sweep getting extra? If the latter, we may also consider looking into #2427 again

[NewSoupVi]

It's because they use get_all_state, which performs sweeps & which we don't have a per player version of.

[BadMagic100]

Yeah so then I think #2427 is the real long term solution here. Whether it's feasible to actually wrap that up idk, as it's been waiting on treble forever now. GER would also benefit from this for larger multiworlds

[NewSoupVi]

We could probably actually just steal get_singeplayer_all_state from that PR, it's all we need for now

[NewSoupVi]

Closing in favor of #4530