Skip to content

Option to randomize the Frogs 2 melody #2064

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

Merged
merged 5 commits into from
Jun 1, 2025
Merged

Conversation

fenhl
Copy link
Collaborator

@fenhl fenhl commented Aug 8, 2023

This PR turns the “Randomize Ocarina Melodies” setting into a multiselect and adds a 3rd option “Frogs Ocarina Game” which randomizes the song that needs to be played for the “ZR Frogs Ocarina Game” check. The notes appear in the spoiler log and can be plando'd like the other melodies, but the melody is locked to a length of exactly 14 notes. Rather than using the existing algorithm for generating random songs, a fully random sequence of notes with even weights is generated.

Special thanks to @rrealmuto whose random_frog_song branch served as the basis for this PR.

I've also included a drive-by fix where the misc. location hint for frogs 2 was incorrectly assumed to be reachable as adult.

@fenhl fenhl added Type: Enhancement New feature or request Component: Logic Non-trivial changes to the JSON logic files Component: Setting specific to setting(s) Component: Patching Affects the patching of the ROM labels Aug 8, 2023
@cjohnson57 cjohnson57 added Status: Needs Review Someone should be looking at it Status: Under Consideration Developers are considering whether to accept or decline the feature described Status: Waiting for Release This PR is ready for merge, but we're holding off on it until after the next release and removed Status: Waiting for Release This PR is ready for merge, but we're holding off on it until after the next release labels Nov 11, 2023
@@ -386,34 +387,40 @@ def get_random_song() -> Song:


# create a list of 12 songs, none of which are sub-strings of any other song
def generate_song_list(world: World, frog: bool, warp: bool) -> dict[str, Song]:

Choose a reason for hiding this comment

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

Is there a technical reason that frogs2 is using this same function? Like will the mini-game break if the frog-song has another song as a sub-string?

If not, I think it would be cleaner to have it have its own function for generating randomly and probably its own dictionary entry for plando. If more enough code is similar perhaps it could be refactored into a few more functions shared between them.

If so, perhaps this could also fix that issue so that the code can be separated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just tested this and it looks like the minigame doesn't break, so I will remove the subsong check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, looking at the code, the subsong check is already skipped for frogs 2. I don't think moving it to a separate function makes sense, as it would have to duplicate a lot of the existing behavior, namely selecting between using a plando'd melody, the vanilla melody, or a random one.

@cjohnson57 cjohnson57 added Status: Waiting for Author Changes or response requested and removed Status: Under Consideration Developers are considering whether to accept or decline the feature described labels Jan 13, 2025
@fenhl fenhl removed the Status: Waiting for Author Changes or response requested label Jan 17, 2025
@fenhl fenhl requested a review from cjohnson57 January 17, 2025 11:12
@fenhl
Copy link
Collaborator Author

fenhl commented Apr 20, 2025

Reviewed by @GSKirox and @rrealmuto in voice.

@fenhl fenhl added Status: Waiting for Release This PR is ready for merge, but we're holding off on it until after the next release and removed Status: Needs Review Someone should be looking at it labels Apr 20, 2025
@fenhl fenhl removed the Status: Waiting for Release This PR is ready for merge, but we're holding off on it until after the next release label Jun 1, 2025
@fenhl fenhl added this to the next milestone Jun 1, 2025
@fenhl fenhl merged commit 1b27340 into OoTRandomizer:Dev Jun 1, 2025
3 checks passed
@fenhl fenhl deleted the frogs2-melody branch June 1, 2025 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Logic Non-trivial changes to the JSON logic files Component: Patching Affects the patching of the ROM 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.

3 participants