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

LTTP/SM/SMZ3: Show correct item icon for cross-game items #1112

Merged
merged 21 commits into from
Jun 29, 2023

Conversation

Alchav
Copy link
Contributor

@Alchav Alchav commented Oct 17, 2022

What is this fixing or adding?

Adds functionality to LTTP, Super Metroid, and SMZ3 to show the correct, corresponding item icon for items from the other game worlds, instead of stars or AP icons.

How was this tested?

Generating multiworlds containing all three games, playtesting and seeing the correct item graphics. Further testing should be done to ensure everything looks correct.

If this makes graphical changes, please attach screenshots.

You'll have to take my word for it that this is a Super Metroid item being found in SMZ3
AP_14089154938208861744_P1_YourName1000

@@ -1730,7 +1732,7 @@ def write_custom_shops(rom, world, player):
replacement_price_data = get_price_data(item['replacement_price'], item['replacement_price_type'])
slot = 0 if shop.type == ShopType.TakeAny else index
if item['player'] and world.game[item['player']] != "A Link to the Past": # item not native to ALTTP
item_code = get_nonnative_item_sprite(item['item'])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems janky but I don't know how else to get an item code from just the name. But also I think a lot of the shop code is janky and should be rewritten at some point.

Copy link
Member

Choose a reason for hiding this comment

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

world.item_name_to_id: dict[str, int]

@Alchav
Copy link
Contributor Author

Alchav commented Oct 21, 2022

LTTP items that aren't valid items in SMZ3 need to be filtered out. Triforce piece has the correct icon if you're in the LTTP side of SMZ3, but will identify it as "a junk item" and in SM side it will have a non-progression AP icon and show no item name when collected. Not sure the easiest way to do this since item names do not match and the item codes are in properties of a class. I could do a big converstion dict like with SM items but it would be a lot of items

@ThePhar
Copy link
Member

ThePhar commented Nov 1, 2022

I have concerns with world implementations hard coding information that exists in other world implementations. In my opinion, world implementations should not need to know anything about other worlds and also, if another world maintainer changes a name or id, this would break. Also, thinking forward, I don't want to set the precedent, "Hey it's okay to code info from another world implementation because their interaction would be neat."

It's a nice to have, for sure, but I think the cons outweigh the pros.

@Alchav Alchav closed this Nov 1, 2022
@Berserker66
Copy link
Member

@lordlou is this something conceptually you'd like?

@lordlou
Copy link
Contributor

lordlou commented Nov 5, 2022

I personally like the idea but that opinion might be biased because I know AP well. Since it changes a behavior that was uniform across all games (I think?), it might confuse some players. That being said, maybe a vote could help us decide (see https://discord.com/channels/731205301247803413/731214280439103580/934522580864471061).

If the vote is in favor, maybe a core feature could be added to supply a matching list of ItemID that games can query. Also, maybe it would be worth making it an option since it is relevant only to each local player.

@Alchav Alchav reopened this Mar 22, 2023
@Alchav Alchav marked this pull request as ready for review March 22, 2023 00:56
@Alchav Alchav marked this pull request as draft March 25, 2023 20:19
@Alchav Alchav marked this pull request as ready for review March 26, 2023 07:52
@ThePhar ThePhar added the is: enhancement Issues requesting new features or pull requests implementing new features. label May 31, 2023
worlds/sm/__init__.py Outdated Show resolved Hide resolved
Alchav and others added 2 commits June 29, 2023 09:33
# Conflicts:
#	worlds/sm/__init__.py
Copy link
Member

@Berserker66 Berserker66 left a comment

Choose a reason for hiding this comment

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

I haven't actually tested this, nor do I want to. However, given that these are some of the most played games, issues should be found during testing phase fingers crossed.

@Berserker66 Berserker66 merged commit 776b5fa into ArchipelagoMW:main Jun 29, 2023
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants