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

LADX: upstream logic updates #3963

Merged

Conversation

threeandthreee
Copy link
Contributor

What is this fixing or adding?

Updates logic from upstream. Also had to correct a logic test.

Changelog

Upstream has changed their logic files to indicate the technique rather than the items, most changes are simply that, listed below are the actual changes to logic:

D1

  • The Three of a Kind with Bomb is moved from Normal to Hard Logic

D3

  • Adds killing the enemies in the North Room with a Pot in Hard Logic

D4

  • Hard logic: Removes Feather requirement from grabbing the Pit Key
  • Hell logic: new hookshot clip (line 64)
  • Hell logic: hookshot spam over the first pit of crossroads, then buffer down (line 69)
  • Hell logic: push block left of keyblock up, then shaq jump off the left wall and pause buffer to land on keyblock.
  • Hell logic: split zol for more entities, and clip through the block left of keyblock by hookshot spam

D5

  • Hell logic: use zoomerang dashing left to get an unclipped boots superjump off the right wall over the block. reverse is push block (line 69)

D6

  • Hard logic: allow damage boosting past the mini thwomps
  • Glitched logic: bomb triggering elephants in two cases

D7

  • Hard logic: Three of a King with bombs only

D8

  • Hard logic: allows to drop the Gibdos into holes as a way to kill them
  • Glitched logic: underground section with fire balls jumping up out of lava. Use boots superjump off left wall to jump over the pot blocking the way

D0

  • Normal logic: Karakoros now need power bracelet to put them into their holes
  • Hard logic: Karakoros without power bracelet but with weapon
  • Hell logic: Karakoros with only bombs

Overworld

  • Normal logic: requires hookshot or shield to traverse Armos Cave
  • Hard logic: Traverse Armos Cave with nothing (formerly normal logic)
  • Hard logic: get the animal village bomb cave check with jump and boomerang
  • Hard logic: use rooster to go to D7
  • Glitched logic: Bomb triggering Yoshi Trade
  • Lots of Jesus Rooster Jumps

How was this tested?

Generating, light playtesting

palex00 and others added 16 commits September 16, 2024 10:10
No logic changes or bugfix are in this file. It is only code cleanup.
- The Three of a Kind with Bomb is moved from Normal to Hard Logic

The rest is code cleanup.

lines 22-25 | 22-26 & 33 | 34 remain different in AP | Upstream with no effective difference
Logic Changes:
- Hard mode now considers killing the enemies in the top room with pot

Everything else is cleanup.
Logic Changes:
- Hard Logic: Removes Feather requirement from grabbing the Pit Key
- Hell logic: new hookshot clip (line 64)
- Hell logic: hookshot spam over the first pit of crossroads, then buffer down (line 69)
- Hell logic: push block left of keyblock up, then shaq jump off the left wall and pause buffer to land on keyblock.
- Hell logic: split zol for more entities, and clip through the block left of keyblock by hookshot spam

The rest is code cleanup
Logic Changes:
- Hell logic: use zoomerang dashing left to get an unclipped boots superjump off the right wall over the block. reverse is push block (line 69)

The rest is cleanup.

The upstream splits the post_gohma region into pre_gohma, gohma and post_gohma. I did not implement this yet as I do not know the implications. To port this the following lines need to be changed (AP | LADXR):
18 | 18-20;
55 | 58;
65 | 68-69
Logic Changes:
- Hard logic: allow damage boosting past the mini thwomps
- Glitched logic: bomb triggering elephants in two cases

Everything else is cleanup
Logic Changes:
- Hard logic: Three of a Kind is now possible with bombs only

Everything else is code cleanup
Logic change:
- Hard logic: allows to drop the Gibdos into holes as a way to kill them
- Glitched logic: underground section with fire balls jumping up out of lava. Use boots superjump off left wall to jump over the pot blocking the way


The rest is code cleanup
Logic changes:
- Normal logic: Karakoros now need power bracelet to put them into their holes
- Hard logic: Karakoros without power bracelet but with weapon
- Hell logic: Karakoros with only bombs

Everything else is code cleanup
* Updating overworld.py

This tries to update all logic of the Overworld.

Logic changes include:
- Normal logic: requires hookshot or shield to traverse Armos Cave
- Hard logic: Traverse Armos Cave with nothing (formerly normal logic)
- Hard logic: get the animal village bomb cave check with jump and boomerang
- Hard logic: use rooster to go to D7
- Lots of Jesus Rooster Jumps

I stopped counting and need to go over this again.

Also, please investigate line 474 AP because it's removed in LADXR-Upstream and I don't know why.

* remove featherless fisher under bridge from hard

it was moved to hell upstream and its already present in our code

---------

Co-authored-by: Alex Nordstrom <[email protected]>
@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Sep 19, 2024
@ScipioWright ScipioWright added is: enhancement Issues requesting new features or pull requests implementing new features. is: refactor/cleanup Improvements to code/output readability or organizization. labels Sep 19, 2024
Copy link

@kbranch kbranch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not an AP dev, but I've done some work upstream and am pretty familiar with its logic. All of this looks pretty independent of the AP internals, so I think I'm reasonably qualified to review it.

I carefully combed through a diff of this vs. upstream - I found a couple spots that I'd expect to throw runtime errors with higher logic levels, but otherwise it looks pretty good.

Anywhere I say "upstream does ..." is just a suggestion. I think they're all useful suggestions, but leaving them out shouldn't cause problems.

I couldn't get GitHub to let me add comments to the exact lines for everything below, in overworld.py. I'm guessing they happened after the AP fork, but before the time frame where PRs were looked at for this update.

upstream includes this below line 135:
graveyard.connect(forest_heartpiece, OR(BOOMERANG, HOOKSHOT), one_way=True) # grab the heart piece surrounded by pits from the north

below line 243:
self._addEntranceRequirementExit("castle_secret_entrance", None) # leaving doesn't require pit_bush

line 251 has the r.bush requirement removed:
self._addEntrance("castle_main_entrance", castle_frontdoor, castle_inside, None)

below line 280:
self._addEntranceRequirementExit("prairie_to_animal_connector", None) # leaving doesn't require pit_bush

below line 394:
multichest_cave.connect(multichest_cave_secret, BOMB, one_way=True)

below line 472 (doctored on the fly a bit to fit the AP version, take with a grain of salt):
self._addEntranceRequirementExit("d2", r.wall_clip) # Clip out at d2 entrance door

worlds/ladx/LADXR/logic/dungeon5.py Outdated Show resolved Hide resolved
worlds/ladx/LADXR/logic/dungeon8.py Outdated Show resolved Hide resolved
worlds/ladx/LADXR/logic/overworld.py Show resolved Hide resolved
worlds/ladx/LADXR/logic/overworld.py Show resolved Hide resolved
@threeandthreee
Copy link
Contributor Author

Thanks for going over this, and yeah I failed to test higher logic levels, palex ended up having to fix some of this on the spot to generate the async this morning.

Copy link

@kbranch kbranch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me with the new changes

Copy link

@SushiKishi SushiKishi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeds with these changes have generated as expected. The proposed changes seem reasonable for the difficulty settings.

Copy link
Contributor

@palex00 palex00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR was tested as part of a Beta apworld that was used in three dedicated Test Asyncs, as well as dozen of normal Syncs. I have also personally run 200 test generations which succeeded.

We have also done polling on what users want, etc. which can be found here

The new feature has worked as expected and other stuff does not seem impacted.

Copy link
Member

@NewSoupVi NewSoupVi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a bunch of logic changes. As always with logic: I'm gonna trust the previous reviewers that the changes are correct.

@NewSoupVi
Copy link
Member

NewSoupVi commented Dec 5, 2024

There are conflicts now though, and these aren't simple enough where I could figure out how to resolve them myself. @threeandthreee

@NewSoupVi NewSoupVi added waiting-on: author Issue/PR is waiting for feedback or changes from its author. and removed waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Dec 5, 2024
@NewSoupVi NewSoupVi merged commit 203d89d into ArchipelagoMW:main Dec 5, 2024
16 checks passed
@threeandthreee threeandthreee deleted the ladx/upstream-logic-updates branch December 10, 2024 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: enhancement Issues requesting new features or pull requests implementing new features. is: refactor/cleanup Improvements to code/output readability or organizization. waiting-on: author Issue/PR is waiting for feedback or changes from its author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants