Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LTTP: Key Drop Shuffle #282

Merged
merged 53 commits into from
Sep 27, 2023
Merged

Conversation

Alchav
Copy link
Contributor

@Alchav Alchav commented Feb 14, 2022

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.

@Berserker66
Copy link
Member

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

@espeon65536
Copy link
Collaborator

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.

@Berserker66
Copy link
Member

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

@Alchav
Copy link
Contributor Author

Alchav commented Feb 14, 2022

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

@espeon65536
Copy link
Collaborator

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

@Berserker66
Copy link
Member

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

@Alchav
Copy link
Contributor Author

Alchav commented Feb 15, 2022

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 and others added 5 commits February 16, 2022 13:39
set address on key drop locations to None
remove plando key drop location ban
add location unit tests for dungeons that are not currently broken
second last two key drops in Desert Palace now have can_kill_most_things as requirement due to room where you must kill several enemies. A test for DP boss with only ice rod for a weapon had to be removed
* Document id range for items and locations

* Update Text.py (ArchipelagoMW#274)

Changed the Houlihan hint tile to list the winner of the SGLive 2021 tournament in similar style to alttp tournament winners.

* [SM] Fix "No Energy" bugs

* OoT: certain ER options convert closed forest into closed deku + child start

* OoT: ER fixes
Don't allow beatable only to influence priority placements
Shuffle spawns after warp songs to prevent spawn points going to Desert Colossus
Prevent child spawn from priority placing at Colossus if overworld ER is off

* OoT: ice traps have the trap attribute

* OoT: make Farore's Wind a never-exclude item if the relevant trick is off

* OoT: invert logic of previous commit

* OoT: regions are not barren if they contain never-exclude items

* ALttP: rework some keydrop rules
TR function is unchanged and probably too conservative, need to rework that
GT and TR tests not passing yet

Co-authored-by: black-sliver <59490463+black-sliver@users.noreply.github.com>
Co-authored-by: Bondo <38083232+BadmoonzZ@users.noreply.github.com>
Co-authored-by: Alchav <59858495+Alchav@users.noreply.github.com>
@Berserker66
Copy link
Member

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

@Alchav
Copy link
Contributor Author

Alchav commented Mar 24, 2022

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.

@Berserker66
Copy link
Member

@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.

Alchav and others added 3 commits September 10, 2023 14:15
@Alchav
Copy link
Contributor Author

Alchav commented Sep 10, 2023

@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 Alchav marked this pull request as draft September 10, 2023 19:51
@Alchav
Copy link
Contributor Author

Alchav commented Sep 10, 2023

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 Alchav marked this pull request as ready for review September 11, 2023 05:25
@Berserker66
Copy link
Member

@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.

@StripesOO7
Copy link
Contributor

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

@Berserker66 Berserker66 merged commit 812dc41 into ArchipelagoMW:main Sep 27, 2023
12 checks passed
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
Co-authored-by: espeon65536 <81029175+espeon65536@users.noreply.github.com>
Co-authored-by: black-sliver <59490463+black-sliver@users.noreply.github.com>
Co-authored-by: Bondo <38083232+BadmoonzZ@users.noreply.github.com>
Co-authored-by: Fabian Dill <Berserker66@users.noreply.github.com>
t3hf1gm3nt added a commit to t3hf1gm3nt/Archipelago that referenced this pull request Jan 19, 2024
@Alchav had changed the required version in the playerSettings.yaml to 0.4.3 as a part of ArchipelagoMW#282, but the PR didn't end up getting into a release until 0.4.4. Updating yaml to make sure people have the proper required version if they ever use this template to play with KDS
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
Co-authored-by: espeon65536 <81029175+espeon65536@users.noreply.github.com>
Co-authored-by: black-sliver <59490463+black-sliver@users.noreply.github.com>
Co-authored-by: Bondo <38083232+BadmoonzZ@users.noreply.github.com>
Co-authored-by: Fabian Dill <Berserker66@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants