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

Add “Vanilla Locations” option for “Shuffle Songs” setting #1882

Open
wants to merge 2 commits into
base: Dev
Choose a base branch
from

Conversation

fenhl
Copy link
Collaborator

@fenhl fenhl commented Feb 14, 2023

This new option is fairly straightforward. It behaves like songs on songs except songs are not even shuffled among each other. The usual considerations for unshuffled items such as being considered “already hinted” are taken care of by the item pool code.

This PR is part of my work on a “Shuffle Items” setting, which can be turned off to make all item locations vanilla, e.g. if you only want to shuffle entrances.

@r0bd0g
Copy link

r0bd0g commented Feb 14, 2023

If we haven't added logic consideration with stones/medallion placement yet then we'd certainly need it before this setting could be added. Obvious examples include Shadow's med not being one of the three that unlocks Nocturne, or any Stones adult-only when it's child start and closed door.

But even with that logic, how do you handle ALR on, adult start, with closed door?

@fenhl
Copy link
Collaborator Author

fenhl commented Feb 14, 2023

If we haven't added logic consideration with stones/medallion placement yet then we'd certainly need it before this setting could be added. Obvious examples include Shadow's med not being one of the three that unlocks Nocturne, or any Stones adult-only when it's child start and closed door.

Dungeon rewards have always been considered by logic as far as I'm aware.

But even with that logic, how do you handle ALR on, adult start, with closed door?

This is something I'd like to handle with an enable relation (#1827). I might make an attempt at implementing those. For now I've added a more descriptive error message for this case.

@r0bd0g
Copy link

r0bd0g commented Feb 14, 2023

"Dungeon rewards have always been considered by logic as far as I'm aware."
They were chosen completely randomly with no logic considerations for a long time. If you're not aware of that having changed, you should check that it's actually been changed. (I wonder if it would need to retry a few times to get a valid layout, similar to how songs has to retry.) (I wouldn't be surprised to learn it was changed at some point since plando'd items could affect what rewards are possible in what locations. It was a headache for plando makers, having to reroll seeds until it landed on a compatible layout.)

Does the code you just added handle random age start choosing adult, or only manually selecting for adult start. (You probably should just ban random age also?)

@fenhl
Copy link
Collaborator Author

fenhl commented Feb 14, 2023

They were chosen completely randomly with no logic considerations for a long time.

This is still the case. The optimization you're suggesting doesn't work since dungeon rewards are placed before entrances, and entrance randomizer assumes a vanilla entrance layout before they're shuffled. So this is another one of these things that have to wait for entrance rando rework.

Does the code you just added handle random age start choosing adult, or only manually selecting for adult start.

It handles both. Manually selecting adult start generates an error message, and random age forces child like it does with closed forest.

@r0bd0g
Copy link

r0bd0g commented Feb 15, 2023

So then if the dungeon reward layout doesn't roll perfectly it's going to have to global retry over and over? You can imagine like child start closed door it might not be so easy to get a valid one.

@cjohnson57 cjohnson57 added Type: Enhancement New feature or request Component: Setting specific to setting(s) labels Feb 18, 2023
@cjohnson57
Copy link
Collaborator

These kinds of issues are why in 3DR I had "Vanilla" be a logic setting that overrode all other logic, and if you wanted to make situations like starting as adult with closed DoT it was on you to glitch your way out of them, lol. I can see why that's not the goal here, though.

I think we can live with the error message for SoT, it's already what'll happen if you plando this situation. For dungeon rewards I agree that some smarter logic considerations would be good to have.

@cjohnson57 cjohnson57 added the Status: Waiting for Author Changes or response requested label Feb 18, 2023
@fenhl fenhl mentioned this pull request Feb 16, 2024
@fenhl fenhl added Status: Blocked Waiting for something else to happen first and removed Status: Waiting for Author Changes or response requested labels Feb 16, 2024
@fenhl
Copy link
Collaborator Author

fenhl commented Feb 16, 2024

Blocked on parts of #2181.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Setting specific to setting(s) Status: Blocked Waiting for something else to happen first Type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants