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

Closed Forest with ER and adult start #1305

Closed
wants to merge 5 commits into from

Conversation

puddydoo
Copy link

Closed Forest is made to work with all forms of ER and with adult start.
Adult start requires either ER or open door of time.
Closed Forest is also changed to only gate the forest exit, and the requirement of defeating Gohma to leave the forest is moved to a separate logical setting.

This doesn't change behavior without mixed pools though
@Zannick Zannick added Component: Logic Non-trivial changes to the JSON logic files Component: Setting specific to setting(s) Status: Needs Review Someone should be looking at it Status: Under Consideration Developers are considering whether to accept or decline the feature described labels May 15, 2021
If Require Gohma is enabled then the 3 entrance pairs accessible at the beginning are not shuffled
@@ -100,6 +95,15 @@ def __init__(self, id, settings):

self.resolve_random_settings()

if self.open_forest == 'closed' and self.starting_age == 'adult' and self.logic_rules == 'glitchless' and \
not (self.open_door_of_time or ('Ocarina' and 'Song of Time') in self.distribution.starting_items or \
Copy link
Collaborator

Choose a reason for hiding this comment

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

'Ocarina' and 'Song of Time' just evaluates to 'Song of Time'.

if self.open_forest == 'closed' and self.starting_age == 'adult' and self.logic_rules == 'glitchless' and \
not (self.open_door_of_time or ('Ocarina' and 'Song of Time') in self.distribution.starting_items or \
self.shuffle_special_interior_entrances or self.shuffle_overworld_entrances or self.spawn_positions):
# adult is not compatible with glitchless closed forest without shuffled entrances or open door of time
Copy link
Collaborator

@fenhl fenhl Mar 3, 2022

Choose a reason for hiding this comment

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

A comment explaining why this is the case would be nice. It took us a while to figure out why the Door of Time setting affects this in #dev-glitchless-logic.

@@ -736,7 +745,9 @@
"ToT Light Arrows Cutscene": "is_adult and can_trigger_lacs"
},
"exits": {
"ToT Entrance": "True",
"ToT Entrance": "
(open_forest != 'closed' or starting_age == 'child' or here(is_child) or
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there should be a comment here explaining why these logical requirements exist.

@fenhl
Copy link
Collaborator

fenhl commented Mar 3, 2022

Judging from the amount of discussion that happened after I mentioned this in Discord, this seems to be a rather controversial setting. As such, it might make more sense to split this up into multiple smaller PRs that can be discussed and merged/rejected individually. I'm considering starting with splitting off “Closed Forest Requires Gohma” into a separate setting.

@cjohnson57
Copy link
Collaborator

This has been split off into more up to date PRs by fenhl.

@cjohnson57 cjohnson57 closed this Apr 11, 2022
@fenhl
Copy link
Collaborator

fenhl commented Apr 11, 2022

My current PRs (#1531 and #1536, for reference) don't cover all of the changes made here, but I plan to do more work in this area later (perhaps once decisions on the 2 open ones have been made).

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: Setting specific to setting(s) Status: Needs Review Someone should be looking at it Status: Under Consideration Developers are considering whether to accept or decline the feature described
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants