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

[ALTTP] Add second Power Star to differentiate non-native progression items #2866

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

Kappatechy
Copy link
Contributor

What is this fixing or adding?

This change updates the ALTTP base patch to add item ID 0x69 for advancement items from other worlds, and adds the corresponding item and sprite handling to allow displaying gold power stars for advancement/progression items from other worlds and silver power stars for other items. A new option, "Star Scams," is added to allow players to opt out of this adjustment and have all non-native items appear as gold power stars, whether they're progression items or not.

New base patch was generated from ArchipelagoMW/z3randomizer#9 after updating it to include recent "Bombless" functionality.

How was this tested?

Multiworlds were generated using ALTTP and several copies of Bumper Stickers, with Star Scams set On for some, Off for others. A number of locations were checked to ensure that the Power Star graphics matched expectations, and the items behaved appropriately.

If this makes graphical changes, please attach screenshots.

AP_16894030208794895037_P1_StarTest1002

krelbel and others added 8 commits August 31, 2023 01:19
This change updates the alttp basepatch to add item ID 0x69 for advancement items from other worlds, and adds the corresponding item and sprite handling to allow displaying gold power stars for advancement/progression items from other worlds and silver power stars for other items.

The basepatch was generated from krelbel/z3randomizer@91c3dd6

Example image: https://i.imgur.com/B2LUdFS.png
This change updates the gold/silver power star item names to be consistent with the final planned behavior of progression items being gold and all other items being silver, and imports a new basepatch based on ArchipelagoMW/z3randomizer@b787b83 which fixes a bug in the previous commit where the silver power star item in newitems.asm:.properties still used the gold power star palette, causing the "item get" animation to incorrectly show a gold power star upon picking up a silver power star.

Tested with a 2-player lttp/hollowknight multiworld verifying standing items look correct in and out of shops and picking up items animates correctly for both gold and silver power stars.

The previous commit listed an outdated screenshot (with the colors reversed); the updated screenshot showing this feature is: https://i.imgur.com/iMPxzgw.png
Fix outdated comment
Update the LTTP basepatch to include recent additions for "Bomb Bag" setting, alongside our Silver Power Star adjustments.
Allow players to opt out of seeing non-progression items as Silver Stars, through a new YAML option.
The wording of the comments in the get_nonnative_item_sprite function was a little confusing, making it sound like the vote mentioned there was for Gold vs. Silver Power Stars, when it's actually for using Power Stars as a whole. This text has been clarified.
@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Feb 26, 2024
@Kappatechy
Copy link
Contributor Author

This is an update/extension of #2139 created with permission from the original developer.

@ScipioWright ScipioWright added the is: enhancement Issues requesting new features or pull requests implementing new features. label Feb 26, 2024
@krelbel
Copy link

krelbel commented Feb 26, 2024

Just confirming here, as the original developer, I fully approve of this change. Thanks for taking care of this Kappa! I'm really excited to see this finally committed. (If everyone's okay with this, then I'll happily close my original PR that I reopened to make my intentions clear.)

Copy link

@shananas shananas left a comment

Choose a reason for hiding this comment

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

Ran 5 seeds with this over the past week or so with no issues. this last seed I did with the shop fix merged in #2890 to confirm it all interacted fine and had no issues.

Copy link
Collaborator

@Exempt-Medic Exempt-Medic left a comment

Choose a reason for hiding this comment

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

Tested two games with these changes and everything appeared to work correctly.

@Exempt-Medic Exempt-Medic added waiting-on: world-maintainer Issue/PR is waiting for feedback or approval by the maintainer of a world. waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. and removed waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Jul 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. waiting-on: world-maintainer Issue/PR is waiting for feedback or approval by the maintainer of a world.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants