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

Override hideout save warp in hideout shuffle #7

Closed
wants to merge 5 commits into from

Conversation

mracsys
Copy link

@mracsys mracsys commented Aug 20, 2022

In vanilla, the Thieves' Hideout save warp leads to the 1 torch carpenter jail cell instead of the default overworld spawn point. If hideout shuffle is on, this has a couple down sides:

  1. The save warp is a new one-way entrance to wherever the 1 torch cell is located.
  2. The behavior may be unexpected given how niche it is.

Having access to an area that you cannot reach with any available items or movement does not feel in theme for ER. This change overrides the Thieves' Hideout save warp target to the default overworld spawn for the current age.

@fenhl
Copy link
Owner

fenhl commented Aug 21, 2022

What about going from the back of a dungeon to the front by savewarping in mixed pools bosses? Do you think that should be changed as well?

@r0bd0g
Copy link

r0bd0g commented Aug 21, 2022

If this change applies even when hideout shuffle is off, that's a mistake. Saving in hideout is useful.

But regardless I think hideout save should continue to warp to hideout. Going from one hideout to the other might be a little weird but savewarping to reach somewhere you couldn't already get to has precedent already with the OW spawn of the age you don't start as. It's true a lot of people won't know about this behaviour since a lot of people don't even know you can save inside hideout, we're going to get questions about what to do, but it's a pretty easy thing to explain.

At worst though I would suggest that the hideouts just savewarp to the same hideout that you're currently in, if that savewarping from one to the other is really so much of a problem. In some cases even that could be used to go somewhere new, so you'd also have to savewarp to the entrance that you came in from, not just any one entrance in that part of hideout. (I think the whole hideout is one map though so I don't think that's an extra complication?) And again I don't really agree that we need this and this would definitely only apply when hideout is shuffled.

@mracsys
Copy link
Author

mracsys commented Oct 9, 2022

What about going from the back of a dungeon to the front by savewarping in mixed pools bosses? Do you think that should be changed as well?

Yes. Using a savewarp to bypass small keys or normally impassable paths (reverse Shadow boat ride) feels janky to me. Overworld savewarps always have a path back to spawn, either the way you came as starting age or itemless as the alternate age.

I would be OK with hideout savewarps going back to the entrance for that segment as r0b suggests. Grottos/interiors would also savewarp to that exit. This needs more romhacking to work if we go this route.

This change would only apply with hideout shuffle on. I'll double check in a bit if I overlooked that. Vanilla behavior is preserved with hideout shuffle disabled.

fenhl pushed a commit that referenced this pull request Mar 6, 2023
Update preset with dungeon shortcuts and boss keysy
@fenhl
Copy link
Owner

fenhl commented May 6, 2023

@cjohnson57 and I discussed this in private. I gave the following list of pros and cons:

Reasons to have the hideout savewarp go to the 1-torch jail:

  • It's the vanilla behavior. If it were changed, it would have to be conditional on hideout ER being on (since people have requested keeping the vanilla behavior when it's off, as it would impact routing), generating another settings-dependent behavior you have to be aware of.
  • It gives the hideout a more interesting topology rather than just being a set of connectors.
  • It makes the world faster to navigate by adding more connections between certain regions.
  • It's more consistent with the handling of dungeon savewarps in mixed pools bosses. You could make it consistent the other way by also removing dungeon savewarps, but I feel like this would be too harsh of a change, and @r0bd0g and I agree that it would require too many compromises (e.g. starting keys, incompatible settings, forcing access from the boss door, etc) to make the logic work.

Reasons to have the hideout savewarp go to overworld spawn:

  • Having the world be more challenging to navigate could also be seen as positive.
  • People may not be aware that the vanilla behavior exists which could lead to them getting stuck.
  • We currently don't have a way to override savewarp behavior of arbitrary scenes or grottos, which means that interiors and grottos connected to hideout exits would continue to use the overworld savewarp despite being part of the hideout hint area. However, this is consistent with how mixed pools bosses works, and is currently only possible in decoupled due to entrance parity.
  • The work has already been done. All I need to do to make the change is merge @mracsys's PR.

I also brought up the possibility of making this a setting, with the options being “Off”, “On (Savewarp to Overworld)”, and “On (Savewarp to 1-Torch Jail)”. It's a bit heavy-handed but that would make it really obvious because you have to explicitly select it, and it would allow people to select the one they want based on how easy or difficult they want navigation to be. @cjohnson57 has decided to merge hideout ER as-is for now but will consider adding it as a setting. As such, I'm closing this PR, but I've merged it into dev-fenhl as a setting.

@fenhl fenhl closed this May 6, 2023
@mracsys
Copy link
Author

mracsys commented May 6, 2023

Thanks for the detailed pros/cons! You didn’t have to go that far. I’ll also keep it on my branch as it’s how I prefer to play.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants