Archipelago icon indicating copy to clipboard operation
Archipelago copied to clipboard

[LTTP] Key Drop Shuffle

Open Alchav opened this issue 3 years ago • 9 comments

Very preliminary.

Currently does not allow for an option to enable/disable key drops. Unit tests are not updated. I see two separate routes to proper implementation:

  1. set up different sets of dungeon rules based on the key drop setting. Remove key drop locations if key drop shuffle is off. Create second set of unit tests to test with key drop shuffle on.

  2. use one logic rule set, and just pre-fill key drop locations with their original keys if key drop shuffle is off. Then remove the locations from the slot after main fill? Adapt unit tests to new logic

Option 1 would be more work (and would mean maintaining different sets of logic rules and unit tests) but option 2 would mean that if there are any problems with new logic, they could affect players that are playing with key drop shuffle off.

Other issues:

Currently the dungeon counters still show the original maximum dungeon items. If these are hardcoded numbers in the base patch I need the addresses for those to change them.

Need to adjust credits_total

Many inline comments do not accurately describe code as is.

Need to get this working with start_with dungeon item setting and universal keys.

I am still working on logic for skull woods and turtle rock.

Alchav avatar Feb 14 '22 20:02 Alchav

@espeon65536 I think you wanted to "have a look".

Berserker66 avatar Feb 14 '22 20:02 Berserker66

Option 2 is definitely the right way to go for the option. It should be the case that keydrop logic reduces identically to current logic with the keys locked into the pots. Dungeon counters are hardcoded to my knowledge, but it shouldn't be hard to tweak. Logic for TR will be hard but I should be able to create a working function for it like what we have right now.

espeon65536 avatar Feb 14 '22 21:02 espeon65536

the AP basepatch should have the dungeon counts overwritable at the same locations door rando does.

Berserker66 avatar Feb 14 '22 21:02 Berserker66

I didn't see where door rando was making any changes for that

Alchav avatar Feb 14 '22 21:02 Alchav

https://github.com/aerinon/z3randomizer/blob/DRMain/compasses.asm should provide some info on where to find the addresses

espeon65536 avatar Feb 14 '22 22:02 espeon65536

or this, I think: https://github.com/Berserker66/MultiWorld-Utilities/blob/1314544ecaa2b66188de23f5900c9e60f4cdd867/Rom.py#L2849

Berserker66 avatar Feb 14 '22 22:02 Berserker66

key_drop_shuffle is now an option. However, turning it off and having small keys set to original dungeon is causing generation failure as the game will appear as unbeatable during output and I haven't been able to figure out why yet

Alchav avatar Feb 15 '22 17:02 Alchav

This wouldn't happen to be able to reach testable stage this weekend? Also the merge conflict on OoT it curious.

Berserker66 avatar Mar 24 '22 00:03 Berserker66

This wouldn't happen to be able to reach testable stage this weekend? Also the merge conflict on OoT it curious.

@espeon65536 asked me not to touch the logic as he is supposedly working on it. The OoT code came from a PR he sent me.

Alchav avatar Mar 24 '22 00:03 Alchav

image

ThePhar avatar Jun 29 '23 20:06 ThePhar

@Alchav would you be willing to:

  1. fix the merge conflicts
  2. mark the option as experimental
  3. fix logic issues as they are found

It doesn't seem like people are actively invested in testing the logic in this PR, so these things may be required to move this forward.

Berserker66 avatar Sep 10 '23 07:09 Berserker66

@Alchav would you be willing to:

1. fix the merge conflicts

2. mark the option as experimental

3. fix logic issues as they are found

It doesn't seem like people are actively invested in testing the logic in this PR, so these things may be required to move this forward.

  1. I attempted to fix merge conflicts. I've since run into a few issues, mainly with world/multiworld variable names, but also with the conflicts in Rules.py with many functions having been renames. I've fixed issues where found, but I wouldn't be surprised if more crop up.
  2. The changes to logic are in effect with or without Key Drop Shuffle turned on. When KDS is turned off, keys are preplaced into the KDS locations and set to events, and are used in the logic.
  3. I would absolutely try to fix any logic issues that come up.

Alchav avatar Sep 10 '23 19:09 Alchav

After generating some large multiworlds with equal weights to all logically relevant option choices, I've run into generation failures that appear to be related to "self locking item" rules and have set this to draft until I figure all that out

Alchav avatar Sep 10 '23 23:09 Alchav

@Alchav would you be willing to:

1. fix the merge conflicts

2. mark the option as experimental

3. fix logic issues as they are found

It doesn't seem like people are actively invested in testing the logic in this PR, so these things may be required to move this forward.

  1. I attempted to fix merge conflicts. I've since run into a few issues, mainly with world/multiworld variable names, but also with the conflicts in Rules.py with many functions having been renames. I've fixed issues where found, but I wouldn't be surprised if more crop up.
  2. The changes to logic are in effect with or without Key Drop Shuffle turned on. When KDS is turned off, keys are preplaced into the KDS locations and set to events, and are used in the logic.
  3. I would absolutely try to fix any logic issues that come up.

Good, and fair enough on 2. Unfortunately LttP is still not apworld compatible. So I'm now in the process of making a whole AP build with this PR for ease of use for testing for users, so they can test along until APs next feature release.

Berserker66 avatar Sep 11 '23 15:09 Berserker66

Just a question: would it be possible to add the key_drop_shuffle option into the fill_slot_data-function in /worlds/alttp/__init__.py into the slot_options-list without me opening another PR when this is ready/released/merged? (for the poptracker pack to autofill this setting too)

If not i'm gonna wait

StripesOO7 avatar Sep 24 '23 19:09 StripesOO7