Improve mixed ER generation with multiple pools#33
Conversation
|
I ran another test on the second preset (Mixed Pools S4), where the shuffle would also go through the |
|
This looks like a huge improvement, thank you for looking into this!
Good catch, but looking at your changes, I'm not seeing where they're reconnected after shuffling. Is that done implicitly somewhere?
The numbers are close enough that I'll want to confirm with a larger sample size, I can set up an ootrstats run for that. If it ends up being an improvement, it should definitely be used for all cases to simplify the code. |
Looking at the existing code again, savewarps are reconnected but only if the source of the savewarp is the target of a shuffled entrance. So the remaining savewarps will have to be reconnected as well (probably easiest to just add a pass to reconnect them all vanilla before the existing code that updates them). |
|
Thanks for catching that with the boss room savewarps. I've added a reconnect pass as you suggested. I also found that blue warps, when shuffled, have the same issue, so I've made a change to disconnect (they get reconnected later when blue warp targets are determined). There were also a couple of bugs related to multiworld generation. With these changes, I got 20/20 successes on the first two settings, and 19/20 on the third. Since you have the tooling for a bigger test, I'd definitely recommend it, since my focus was really on just these three settings. I've update the PR with the new changes and results. |
|
While attempting to collect more data, I found a seed that takes much longer than the rest to generate (possible infinite loop?): commit f4ac36d, with the Mixed Pools S4 settings string ( |
|
Sorry, forgot to say, the following settings.json needs to be used in addition to the settings string to repro: {
"check_version": true,
"create_spoiler": true,
"create_cosmetics_log": true,
"create_patch_file": true,
"create_uncompressed_rom": true,
"create_compressed_rom": false,
"salt_seed": false
}It seems to be getting stuck in rom patching. |
|
This turned out to be a bug in |
In these cases, first un-assume all entrance pools, then assume and shuffle one pool at a time, and validate only at the very end.
10d24f8 to
9795d15
Compare
|
Here's my results. Metric is average CPU instructions until success (lower is better) with a sample size of 512.
detailed statsFormula used to calculate the metric: MW S5 Variantbaseline (ff2602f)success rate: 2/512 (0.39%) second fix (9795d15)success rate: 512/512 (100.00%) second fix, unconditional (30c65d8)success rate: 512/512 (100.00%) Mixed Pools S4baseline (ff2602f)success rate: 135/512 (26.37%) second fix (9795d15)success rate: 512/512 (100.00%) second fix, unconditional (30c65d8)success rate: 512/512 (100.00%) Mixed Pools S4 MWbaseline (ff2602f)No successful seeds, so average instruction count is infinite second fix (9795d15)success rate: 489/512 (95.51%) second fix, unconditional (30c65d8)success rate: 492/512 (96.09%) Based on these numbers, the sequential code path seems to be about 2% faster than the mixed code path for the settings where it's used. I'd say that makes the sequential code path worth keeping. |
When multiple entrance pools are shuffled together with mixed pools, seed generation frequently fails. The root cause is
assume_entrance_pool(). When all pools are assumed simultaneously,assume_entrance_pool()creates assumed exits on theRootregion for every entrance across all pools. During per-entrance validation,Searchexplores fromRootand sees all these assumed exits, leading it to consider every region reachable, regardless of its actual placement. This means bad placements get accepted, and the seed only fails later during the final fill, triggering a full retry.The fix in this PR is to un-assume all entrance pools, then assume and shuffle one pool at a time, so
Searchonly sees the assumed exits for the current pool. By skipping the per-poolvalidate_world()andconfirm_replacement()calls, we keep placements tentative. Only after all pools have been shuffled is this validation performed. If there's a failure, the shuffle is rolled back, entrance pools are un-assumed, and the process is repeated.Additionally, currently, boss room savewarps and blue warps when set to "Dungeon Entrance" remain connected to their vanilla targets during shuffling, providing false reachability paths. This PR also disconnects these connections before entrance shuffling, improving the generation success rate.
Finally, this PR fixes a couple of bugs that caused issues with multiworld generation. The
worldvariable was shadowed in a few places, leading to references to the wrong world. Similarly, theplaced_one_way_entrancesvariable was overwritten for each world, causing the final validation check across all worlds to use only the entrances of the last world shuffled.Testing
I looked into this because we wanted to run a multiworld seed with some crazier entrance randomization, but couldn't get a seed to generate successfully. We used the MW S5 preset as a base, then added Full Interior and Boss entrance shuffle to a single pool, and Dungeon + GC entrance to a separate pool. The following tests are run on v9.0.14-3. I've also added before/after results on the Mixed Pools S4 preset and this preset, but as a 3-player multiworld. Each test uses 20 seeds, with a max of 10 retries per seed.
MW S5 Variant:
WACSPSMAWCBDDSVLDNNACUXJ22A6AJACAAASABWEFAAEYGASHFSFAAJA2ETSAAFAEEALFSVCRLAASAJADSB4SHAE2BRCEH7KYCADJBLUK7NBAAAAAAANS8WBTPAHDACEGJNQYA6B43BCALEJGUBBMixed Pools S4:
EACSQSKAWCBDDSSJENNACWXJ22A6AWAEAAASABX9LXLMDA2DCAGXMAASASKCBBALAJJAWKBHF8WAAABSAGADWBRAJSVA3KRUXA2ALSUNLHMAAAAAAAADWHFJED2ZASSBCDARCSDAEAWJSNEDCMixed Pools S4 MW:
WACSQSKAWCBDDSSJENNACWXJ22A6AWAEAAASABX9LXLMDA2DCAGXMAASASKCBBALAJJAWKBHF8WAAABSAGADWBRAJSVA3KRUXA2ALSUNLHMAAAAAAAADWHFJED2ZASSBCDARCSDAEAWJSNEDC