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

Show subclass super #10513

Closed
wants to merge 3 commits into from
Closed

Show subclass super #10513

wants to merge 3 commits into from

Conversation

ryan-rushton
Copy link
Contributor

@ryan-rushton ryan-rushton commented Jun 7, 2024

Overview

This changes the loadout code so that we show the super at all times. This change largely accommodates prismatic, which doesn't replace the icon with the super icon. I think this behaviour is desirable for prismatic as its not really the same as whatever super you are running due to transcendence.

TODO

Before

Screenshot 2024-06-07 at 10 02 17 PM

After

Screenshot 2024-06-07 at 10 02 02 PM

@@ -101,6 +101,7 @@ export default function LoadoutEditSubclass({
// plugs in the loadout and they may be different to the popup
onClick={plugs.length ? undefined : onClick}
item={subclass.item}
hideSelectedSuper
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we just remove this prop altogether from inventory item and do this by default?

Comment on lines +23 to +26
export const prismaticSubclassHashes = [
1616346845, // titan
3893112950, // warlock
4282591831, // hunter
Copy link
Contributor Author

@ryan-rushton ryan-rushton Jun 7, 2024

Choose a reason for hiding this comment

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

The only other way I can see to do this atm is trait id's ("item.subclass.prism") and from memory we avoid them? See https://data.destinysets.com/i/InventoryItem:3893112950

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just found we generate the trait hash, but we don't have the information available for the inventory item. Let me know how you want to solve this as I don't love the hardcoded hashes here.

@bhollis
Copy link
Contributor

bhollis commented Jun 8, 2024

I think I'd like to see if we can try and preserve the current behavior of highlighting the current super. The way we do it for the other subclasses is we have a manually edited background and then we overlay the super icon - if we do the same with prismatic, we'd have a diamond with the proper super inside a pink circle, and I think it'd still be clear that it's prismatic. My worry with putting the super in with the other subclass plugs is that it's kind of hard to pick out.

@ryan-rushton
Copy link
Contributor Author

ryan-rushton commented Jun 8, 2024

Happy to give that a go. I also had that thought but personally prefer this way conceptually. I'll put something together when I get a chance if someone doesn't beat me.

@delphiactual
Copy link
Contributor

image

@bhollis
Copy link
Contributor

bhollis commented Jun 11, 2024

I think I'd be OK with this if the super were in a diamond.

@bhollis
Copy link
Contributor

bhollis commented Jun 28, 2024

Closing since the other approach merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants