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

Allow plandoing different settings per world #2055

Draft
wants to merge 3 commits into
base: Dev
Choose a base branch
from

Conversation

fenhl
Copy link
Collaborator

@fenhl fenhl commented Jul 30, 2023

This PR makes the settings entry of a plando file a per-world key, much like entrances and locations, allowing different settings to be specified for different worlds. Care has been taken to ensure that everything works as expected. For example:

  • If a chest in world 1 has an item for player 2, the chest appearance will be based on world 1's CAMC setting combined with world 2's win conditions.
  • Some items with different effects depending on settings, such as blue fire arrows/ice arrows or keyrings with/without boss keys, have been split into separate items internally so the correct text box is displayed to the player finding the item.
  • Defeat Ganon worlds ignore Triforce pieces received from the network.
  • Search.can_beat_game has been adjusted to allow a mix of different “Reachable Locations” settings (but see the note about goals below).

Therefore, per-world settings are supported for almost all settings, with the following exceptions:

  • world_count, player_num, distribution_file, create_spoiler, create_compressed_rom, etc. (since these apply to the randomizer in general, not specific worlds)
  • logic_rules (mixing glitchless and glitched works, but mixing no logic with glitchless and/or glitched does not work properly)
  • reachable_locations (mixing Required Only and All works, but mixing All Goals does not due to the issue with goals)
  • hint_dist (hint distributions with Goal hints don't work properly, see note about goals below)

This PR is a breaking change for plando authors, since the system where different starting items could be specified per world has been removed. Now, instead of writing:

{
    "settings": {
        "starting_items": {
            "World 1": {...},
            "World 2": {...}
        }
    }
}

Write it this way:

{
    "settings": {
        "World 1": {
            "starting_items": {...}
        },
        "World 2": {
            "starting_items": {...}
        }
    }
}

Special thanks to @Andols0 whose individual-settings branch served as the basis for this PR.

Issues keeping this in draft state

  • The code for goals has not been adjusted, so they currently base everything on world 1's settings. Hopefully @mracsys or someone else can help out with this.
  • Allowing a mix of No Logic and Glitchless/Glitched should be fairly easy to do in comparison.

@fenhl fenhl added Type: Enhancement New feature or request Component: Algorithm Search, Fill, Playthrough, etc Component: ASM/C Changes some internals of the ASM/C libraries Component: Plandomizer Plandomizer library and functionality labels Jul 30, 2023
@r0bd0g
Copy link

r0bd0g commented Jul 30, 2023

I can imagine a lot of ways this could go wrong, lol. This is some complex stuff I think.

I have a question about chest speeds. Is it allowed that the same item have different chest speeds within the same seed? I could see that being required to be possible, if the same item is major in some worlds and minor in others, but I'm not sure whether it's allowed for the same item to have different speeds from one chest to the next? I feel like I worried about ice traps for the same reason but can't remember what I was told about that.

@fenhl
Copy link
Collaborator Author

fenhl commented Jul 30, 2023

With this PR, “token (small chest)” and “token (big chest)” are internally treated as different items, with different IDs. I've tested it just to be sure and it works as expected.

@r0bd0g
Copy link

r0bd0g commented Jul 30, 2023

Just curious if you know whether ice traps still are or ever were bugged in the way I describe? I think I opened an issue about that a long time ago, but I forget how it ended up.

Just tokens or are there other items that that applies to? Like PoHs or HCs maybe? Any weird settings out there that might change whether some items are in big chests or not?

@fenhl
Copy link
Collaborator Author

fenhl commented Jul 31, 2023

Just curious if you know whether ice traps still are or ever were bugged in the way I describe? I think I opened an issue about that a long time ago, but I forget how it ended up.

I'm not aware of any bugs like that.

Just tokens or are there other items that that applies to? Like PoHs or HCs maybe? Any weird settings out there that might change whether some items are in big chests or not?

The same thing applies to heart containers and pieces, Deku and Hylian shields, and bombchus. There are no other items whose chest appearance changes conditionally. I'm certain about this because this PR deletes the previous code that handled this and replaces it with a mechanism that conditionally changes the IDs of these items.

@ETR-BTF
Copy link

ETR-BTF commented Jul 31, 2023

I'm not sure if I like the mixing of logic rules. If any of the items of a glitchless world is inside a glitch world, then the former is not really a glitchless world anymore. That is, that person's world is no longer guaranteed beatable without using glitches. It'd be even worse with a no logic seed mixed in, because then that glitchless world might not even be beatable at all.

@fenhl
Copy link
Collaborator Author

fenhl commented Jul 31, 2023

That's true, but this is a plando-only feature so I think it's fine to allow the possibility at the plando author's own risk. There could be a note about it in the wiki documentation.

@ETR-BTF
Copy link

ETR-BTF commented Jul 31, 2023

That's true, but this is a plando-only feature so I think it's fine to allow the possibility at the plando author's own risk. There could be a note about it in the wiki documentation.

For me it's not so much about protecting the users, but more about how glitchless logic will fail to do what it promised. It's more of a design choice objection. Allowing mixed logic rules is not as trivial as allowing mixed sanities and I think it needs to be considered very carefully if this is something we want.

@Andols0
Copy link

Andols0 commented Jul 31, 2023

That's true, but this is a plando-only feature so I think it's fine to allow the possibility at the plando author's own risk. There could be a note about it in the wiki documentation.

For me it's not so much about protecting the users, but more about how glitchless logic will fail to do what it promised. It's more of a design choice objection. Allowing mixed logic rules is not as trivial as allowing mixed sanities and I think it needs to be considered very carefully if this is something we want.

I'm not sure where the problem is?
The player with a glitchless world(1) will never need to do a glitch to do an item check.
An item might be in the glitched world (2) where player 2 need to do an glitch.
If its more of a philosophy thing I can't really see the difference in this compared to any other setting.

How to make it actually work with a no logic world, that's a thing I don't know how :/

@r0bd0g
Copy link

r0bd0g commented Jul 31, 2023

Ice traps could potentially appear as both major or minor items in the same seed. So I'm just curious how that's handled.

@fenhl
Copy link
Collaborator Author

fenhl commented Jul 31, 2023

There are separate cloak pools for each option of the ice trap appearances setting. Each ice trap pulls from the pool according to its world's setting (not necessarily the world it's placed in).

@fenhl fenhl added the Changes Item Table Adds/removes items from the item table. Allows coordination of multiple PRs which add to the table. label Dec 5, 2023

[0x0122] = ITEM_ROW(0x53, SILVER_CHEST, 0x41, -1, 0x9211, 0x0195, 0x77, no_upgrade, give_small_key_ring, FOREST_ID, true, NULL), // Forest Temple Key Ring (with boss key)
[0x0123] = ITEM_ROW(0x53, SILVER_CHEST, 0x41, -1, 0x9212, 0x0195, 0x77, no_upgrade, give_small_key_ring, FIRE_ID, true, NULL), // Fire Temple Key Ring (with boss key)
[0x0124] = ITEM_ROW(0x53, SILVER_CHEST, 0x41, -1, 0x9213, 0x0195, 0x77, no_upgrade, give_small_key_ring, WATER_ID, true, NULL), // Water Temple Key Ring (with boss key)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note to self: move these definitions to get item IDs that don't conflict with other open PRs.

@fenhl fenhl added Status: Help Wanted Extra attention is needed Status: Needs Review Someone should be looking at it Status: Needs Testing Probably should be tested Status: Under Consideration Developers are considering whether to accept or decline the feature described labels Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes Item Table Adds/removes items from the item table. Allows coordination of multiple PRs which add to the table. Component: Algorithm Search, Fill, Playthrough, etc Component: ASM/C Changes some internals of the ASM/C libraries Component: Plandomizer Plandomizer library and functionality Status: Help Wanted Extra attention is needed Status: Needs Review Someone should be looking at it Status: Needs Testing Probably should be tested Status: Under Consideration Developers are considering whether to accept or decline the feature described Type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants