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 bank with item swaps #5953

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

nwjgit
Copy link
Contributor

@nwjgit nwjgit commented Jul 14, 2024

Description:

Add a method to swap items with the wrong ids with the proper id & properly update bank/gear/sac/cl data.
This fixes the dt2 beta id issue and any future similar issues.

Currently this is only under /tools user fixbank I would recommend maybe having this auto run under some command such as when the user runs /bank or /cl

Changes:

  • Add /tools user fixbank to OSB
  • Add code inside fixbank that handles swapping item ids for bank/gear/sac/cl data
  • Use the real id for the ring creation to prevent future beta id rings from entering the game
  • Set the rings to use itemAliases
  • Make /sacrifice /sell /trade use item aliases to always select the correct item.

Other checks:

@nwjgit nwjgit added the Type: Bug Something isn't working label Jul 14, 2024
Copy link
Collaborator

@gc gc left a comment

Choose a reason for hiding this comment

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

I'm not 100% sure on this yet, we would benefit from repairItems function in osb (although I just redid that function entirely on bso), but repairItems is only used for removing items, not swapping. I believe it may be more beneficial to do the item swapping entirely in SQL scripts, so that its fixed for everyone on the bot all at once, and people don't have to run a command or get errors. There's also the G.E that has the items in it. Is there a more practical issue this is fixing or is the goal just to have the IDs correct?

@@ -52,7 +52,7 @@ const magusRing: Createable[] = [
{
name: 'Magus ring',
inputItems: new Bank().add('Magus icon').add('Chromium ingot', 3),
outputItems: new Bank().add('Magus ring')
outputItems: new Bank().add(28313) //use id to prevent beta id usage
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use EItem.MAGUS_RING. They have the proper IDs I believe.

@nwjgit
Copy link
Contributor Author

nwjgit commented Jul 18, 2024

I'm not 100% sure on this yet, we would benefit from repairItems function in osb (although I just redid that function entirely on bso), but repairItems is only used for removing items, not swapping. I believe it may be more beneficial to do the item swapping entirely in SQL scripts, so that its fixed for everyone on the bot all at once, and people don't have to run a command or get errors. There's also the G.E that has the items in it. Is there a more practical issue this is fixing or is the goal just to have the IDs correct?

I believe the SQL script would be optimal, but without access to the real db this is the best I could do. The G.E. shouldn't have any of the beta id rings in it since it isn't tradable (Not 100% sure on BSO tho).

The fix in 6b02a88 had its issues:

  • items still worth 0 gp
  • we can still create the old beta id rings and they wont be "switched" until the bot restarts
  • items can't be traded/sacrificed/sell by name since these commands check for the old id still
    • This PR had a fix for that by checking the additions to itemAliases.ts I had setup
  • the true soulreaper axe ID is missing from OSJS so we are stuck regardless with beta id for it (which has no value or tradable)

The goal would be to migrate all five of the ids to their real id, and then remove the beta ids permanently from OSJS. Those five ids are constantly being used in beta worlds so this issue, even if fixed today, will become an issue in the future when those ids are renamed to something else. With that all correct the rings & axe will have their proper icon sell/sac value and ability to be traded.

@nwjgit nwjgit added the Type: Suggestion New feature or request label Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working Type: Suggestion New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DT2 item incorrect data DT2 Rings are using the beta version
2 participants