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

Wonderitem Shuffle #2043

Merged
merged 4 commits into from
Dec 5, 2023
Merged

Wonderitem Shuffle #2043

merged 4 commits into from
Dec 5, 2023

Conversation

rrealmuto
Copy link

Figured why not PR this.

Adds a setting to shuffle Wonderitems into the location pool.

Wonderitems consist of:
Invisible rupees (like the ones you find in KF near the kokiri sword chest)
Invisible hit markers (things you hit with slingshot/hookshot/bow/sword) that drop rupees
Invisible "tags" (the stepping stones in KF)

Wonderitems are invisible so when this setting is enabled, a sparkle effect is used to show their location.

@fenhl fenhl added Type: Enhancement New feature or request Component: Logic Non-trivial changes to the JSON logic files Component: ASM/C Changes some internals of the ASM/C libraries Component: Setting specific to setting(s) labels Jul 15, 2023
@rrealmuto
Copy link
Author

FYI to those reviewing, this has been live on my branch for months now and has been rock solid. There is not a whole lot of logic that needs reviewing - most of the wonderitems are very self-explanatory.

@r0bd0g
Copy link

r0bd0g commented Jul 15, 2023

I think I see at least one likely mistake? I'd have to go in and see where all these are at...

Copy link
Collaborator

@cjohnson57 cjohnson57 left a comment

Choose a reason for hiding this comment

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

Amazing work, I'm real excited for this one. We're starting to gear up for a release, but once that's done this'll probably be the first large feature I merge.

ASM/c/en_wonderitem.c Outdated Show resolved Hide resolved
Patches.py Outdated Show resolved Hide resolved
LocationList.py Outdated Show resolved Hide resolved
data/World/Gerudo Training Ground MQ.json Show resolved Hide resolved
wonderitems.py Outdated Show resolved Hide resolved
wonderitems.py Outdated Show resolved Hide resolved
wonderitems.txt Outdated Show resolved Hide resolved
@r0bd0g
Copy link

r0bd0g commented Jul 16, 2023

I'm committing to review the logic on this in maybe a month or so.

@cjohnson57
Copy link
Collaborator

I'm committing to review the logic on this in maybe a month or so.

It shouldn't be too bad at all, it's a lot of new checks but 90% of them are either True or just checking for a single item, no changes to entrances etc.

@r0bd0g
Copy link

r0bd0g commented Aug 26, 2023

That was most of them. For the record, here some important things that I would say are still left.

  • The other thing I mentioned was that a lot of the location list's tags were wrong.
  • The naming inconsistency between the Vanilla and MQ Shadow versions of the "Shadow Temple 3 Spinning Pots Wonderitem" / "Shadow Temple MQ 3 Spinning Pots Arrow Wonderitem" still exists.
  • "Water Temple MQ Boss Hallway Hookshot Wonderitem"s needs to check for some additional items in the event that you were to come into this room from the boss door in mixed pools. It's not possible on main branch but we're supposed to have compatible logic anyway. It should be canuseHookshot and (Longshot or not_ohko or canuseNayrus or canuseIceArrows). But of course, using the Ice Arrows comes with the complication of having to make it so that Ice Arrows won't be foolish anymore. (So maybe it's not worth the trouble?) They would need to be not be foolish if dungeon wonder items are shuffled, and mixed_pools_bosses is True (in main branch this is always set to False).
  • This one applies to main branch but there's no reason not to fix it here? You should add a wall fairy to the boss hallway in MQ Water with the same logic as the previously mentioned wonderitems. I didn't know about it before but it seems to be logically relevant. (However, it's possible that I forgot to check whether the fairy is repeatable?)

@rrealmuto rrealmuto force-pushed the wonderitems branch 2 times, most recently from e5219d3 to 643af82 Compare September 2, 2023 14:59
Copy link
Collaborator

@cjohnson57 cjohnson57 left a comment

Choose a reason for hiding this comment

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

Alright! So this is good to go from my perspective. @r0bd0g Since it's been a while since you posted, were your concerns resolved?

@r0bd0g
Copy link

r0bd0g commented Nov 14, 2023

Nothing has changed with regards to the logic since I posted about my remaining concerns.

IIRC there were some wrong tags in location list.

There's a naming mismatch between the MQ and Vanilla versions of a new location in Shadow Temple.

If you enter MQ Water from the boss door, we need to check for some items to get down past the spike traps, relevant for any branches with mixed pools bosses. I previously posted some psuedocode of what the logic on the wonder items in the pre boss room could be changed to.

And somebody should check so I'm asking RealRob to do it, whether the wall fairies in the MQ Water preboss room are repeatable (leave the dungeon and come back when you check). If they are, then they need to be added as a "Wall Fairy", and technically this is a potential bug in main branch too, but I think this is a good place to correct it. It would have the same logic as the wonder items here, plus bottle.

Those last two points come with the added step of needing to add a foolish exception for Ice Arrows if it's mixed pools bosses (which is a property that is always false on main branch). (If it turns out the aforementioned wall fairy isn't repeatable, the exception would also only be needed when dungeon wonder items are shuffled.)

SettingsList.py Outdated Show resolved Hide resolved
SettingsList.py Outdated Show resolved Hide resolved
SettingsList.py Outdated Show resolved Hide resolved
@rrealmuto
Copy link
Author

So I think I addressed the rest of the comments. With the exception of I'm not touching that Ice Arrow thing someone else can do that if/when mixed pools bosses is merged. Let me know if we're good to go and I can rebase.

@r0bd0g
Copy link

r0bd0g commented Nov 21, 2023

I'll sign off on it yeah.

@cjohnson57
Copy link
Collaborator

Alright now that #2019 is merged this is my next priority to get merged

@rrealmuto
Copy link
Author

Ok I'll rebase it tonight

@rrealmuto
Copy link
Author

Finished rebasing. Just need to do a bit of testing

@rrealmuto
Copy link
Author

Ok seems to be working well after rebasing. Merge at will

@r0bd0g
Copy link

r0bd0g commented Dec 1, 2023

I specifically didn't include a Fairy here because what happens a lot of the time is you die and revive, and then the next trap hits you anyway.

@cjohnson57
Copy link
Collaborator

I'll merge most likely on Saturday

@rrealmuto
Copy link
Author

I specifically didn't include a Fairy here because what happens a lot of the time is you die and revive, and then the next trap hits you anyway.

Don't you get some iframes after the fairy heals you?

@cjohnson57 cjohnson57 merged commit 1827fbb into OoTRandomizer:Dev Dec 5, 2023
3 checks passed
@fenhl fenhl added this to the 8.1 milestone Mar 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: ASM/C Changes some internals of the ASM/C libraries Component: Logic Non-trivial changes to the JSON logic files Component: Setting specific to setting(s) Type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants