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

Fix effective starting trade items #2217

Open
wants to merge 1 commit into
base: Dev
Choose a base branch
from

Conversation

fenhl
Copy link
Collaborator

@fenhl fenhl commented May 12, 2024

This PR fixes a bug where in full trade shuffle, if you start with a trade quest item from Impa and then find another one, the one you started with is lost. To reproduce, start this plando and open one of the chests:

{
    "settings": {
        "open_forest": "open",
        "spawn_positions": ["child"],
        "adult_trade_shuffle": true,
        "shuffle_song_items": "any",
        "starting_items": {
            "Zeldas Letter": 1
        }
    },
    "entrances": {
        "Child Spawn -> KF Links House": "KF Midos House"
    },
    "locations": {
        "Song from Impa": "Poachers Saw",
        "KF Midos Top Left Chest": "Pocket Egg",
        "KF Midos Top Right Chest": "Cojiro",
        "KF Midos Bottom Left Chest": "Odd Mushroom",
        "KF Midos Bottom Right Chest": "Odd Potion"
    }
}

On Dev, you will see that the poacher's saw has been replaced with the item you got from the chest, and using the D-pad to go back to the saw doesn't work. After opening a second chest, you can toggle between the two items you got from chests, but still can't select the saw. With this PR, the D-pad toggle works as soon as the first chest is opened, and the saw is included.

Details:

  • The code from Patches.py that would set the “owned” flags for starting trade items is removed and instead these flags are handled by the SaveContext.py system, which also handles effective starting items such as the song from Impa in Skip Child Zelda.
  • The logic in configure_effective_starting_items for selecting the last trade item in the sequence is removed. This logic was redundant since SaveContext.py already handles an address being written to multiple times by keeping the larger value.
  • Minor code cleanup in Patches.py removing a redundant check (world.skip_child_zelda implies 'Zeldas Letter' in world.distribution.starting_items).
  • Minor code cleanup in SaveContext.py for the ocarina buttons.

@fenhl fenhl added Type: Bug Something isn't working Status: Needs Review Someone should be looking at it Component: Patching Affects the patching of the ROM labels May 12, 2024
@cjohnson57
Copy link
Collaborator

Good work. So the bug was caused by Patches.py not setting the "owned" flag correctly?

@fenhl
Copy link
Collaborator Author

fenhl commented May 13, 2024

Yes. The code in Patches.py to set “owned” flags was only running for explicit starting items, not for effective starting items like the song from Impa. I could have simply changed the iterator in that code, but I think this fix is cleaner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Patching Affects the patching of the ROM Status: Needs Review Someone should be looking at it Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants