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

Add dummy-catalyst-mapping.json for mapping dummy catalyst items to actual catalyst items #603

Merged
merged 4 commits into from
May 8, 2024

Conversation

FlaminSarge
Copy link
Contributor

@FlaminSarge FlaminSarge commented Apr 29, 2024

This affects any of the older exotics that used to have "Upgrade Masterwork" and now have dummy items for the catalyst sockets.
Bungie uses reinitializationPossiblePlugHashes to autoapply the actual catalyst item once it's completed, but the values available to be socketed on the exotic itself do not include the actual catalyst item, so this mapping is needed for D2 tools to be able to automatically grab the real catalyst item based on the dummy item present in the socket options for a given exotic.

Ideally, this would allow D2 tools to show/use the correct catalyst plug for 'compare' tools e.g.
light.gg, this section (replacing the dummy item that doesn't provide the correct stats):
Screenshot 2024-04-29 at 5 33 25 AM
d2foundry, this section:
Screenshot 2024-04-29 at 5 34 21 AM

Since I already have the catalyst equipped/completed for all of my weapons, I was not able to grab an example for use in DIM, but I believe the following window currently shows no entries if the catalyst is not unlocked for one of the legacy exotics yet, and it may be useful to show a greyed-out entry of the actual catalyst with its proper effects instead:
image

@FlaminSarge
Copy link
Contributor Author

The other way I had tried implementing this was:

const legacyDummyCatalysts = getAllDefs('InventoryItem').filter(
  (i) => i.itemType === 20 && i.plug?.uiPlugLabel === 'masterwork_interactable',
).map((i) => [i.plug?.plugCategoryHash, i.hash]);
const modernCatalysts = new Map(getAllDefs('InventoryItem').filter(
  (i) => i.itemType === 19 && i.plug?.uiPlugLabel === 'masterwork',
).map((i) => [i.plug?.plugCategoryHash, i.hash]));
const legacyCatalystMapping = Object.fromEntries(legacyDummyCatalysts
  .map(([plugCategoryHash, legacyHash]) => [legacyHash, modernCatalysts.get(plugCategoryHash)])
  .filter(([legacyHash, modernHash]) => legacyHash && modernHash)
  .map(([legacyHash, modernHash]) => [legacyHash!, modernHash!]
  ));

There didn't seem to be a significant perf difference between the two but this seems less... "correct" than matching based on Socket category and pulling socket.reinitializationPossiblePlugHashes.

@FlaminSarge
Copy link
Contributor Author

FlaminSarge commented Apr 30, 2024

...Thoughts on renaming 'legacy' to 'dummy'?

Separately, I plan to create a mapping for craftable catalysts, unsure how I want to go about that quite yet though (whether it should be like this from dummy hash -> real hash or if it should be from weapon -> [item hashes])

@delphiactual
Copy link
Collaborator

re: legacy vs dummy either is fine, let me know when you are ready for merge

Copy link
Contributor

@bhollis bhollis left a comment

Choose a reason for hiding this comment

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

I think this makes sense. I do agree that "dummy" is probably better than "legacy". It'd be great if you could run pnpm pretty on this too (it should've done it automatically during commit, not sure why it didn't.

src/generate-catalyst-data.ts Outdated Show resolved Hide resolved
…ctual catalyst items

This affects any of the older exotics that used to have "Upgrade Masterwork" and now have dummy items for the catalyst sockets.
Bungie uses reinitializationPossiblePlugHashes to autoapply the actual catalyst item once it's completed, but the values available to be socketed in do not include the actual catalyst item, so this mapping is needed for D2 tools to be able to automatically grab the real catalyst item based on the dummy item present in the socket options for a given exotic.
@FlaminSarge
Copy link
Contributor Author

FlaminSarge commented May 4, 2024

I think this makes sense. I do agree that "dummy" is probably better than "legacy". It'd be great if you could run pnpm pretty on this too (it should've done it automatically during commit, not sure why it didn't.

✗ pnpm pretty
 ERR_PNPM_BAD_PM_VERSION  This project is configured to use v8.8.0 of pnpm. Your current pnpm is v9.0.4

If you want to bypass this version check, you can set the "package-manager-strict" configuration to "false" or set the "COREPACK_ENABLE_STRICT" environment variable to "0"

Disabled the strict check and it went through (but also touched every file in the repo in the process; also touched the spacing on other methods in this file, so recommend looking at the diff with 'hide whitespace changes' now).

@delphiactual this should be ready to go

@FlaminSarge FlaminSarge changed the title Add legacy-catalyst-mapping.json for mapping dummy catalyst items to actual catalyst items Add dummy-catalyst-mapping.json for mapping dummy catalyst items to actual catalyst items May 4, 2024
@delphiactual delphiactual merged commit 093195f into DestinyItemManager:master May 8, 2024
5 checks passed
@FlaminSarge FlaminSarge deleted the catalysts branch May 8, 2024 23:34
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.

None yet

3 participants