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 currency display in Wrath Classic #715

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Fiffers
Copy link

@Fiffers Fiffers commented Oct 9, 2022

Addresses #707

Refactored a bit of the currency frame module to allow it to work with the currency API in Wrath Classic. With the old version, the GetCurrency local variables that were just making copies of the globals in Retail were overwriting those functions of the same name in Wrath Classic. Therefore, they needed to be renamed.

Otherwise, everything else is pretty straightforward. I wrote some functions to handle the differences between the two in an attempt to minimize the changes to the pre-existing codebase (159-181). Please have a look at these and let me know if you have a better solution in mind.

Lastly, I needed to take the GetCurrencyListInfo() from Wrath Classic and convert the resultant data to fit the mold of its retail counterpart. After all of this, it works great on Wrath Classic!

Sadly, I do not have a retail character, so I cannot test this PR on there. There should be no adverse effects, but it'd be helpful if someone could test these changes out and provide feedback on their results.

Here's a picture! =)
image

I haven't tested this in Retail, but it should work.
@Beldin31
Copy link

Beldin31 commented Oct 11, 2022

So i've tried this and it does show the currencies in the bag, which is great.
My only problem is it shows all the currencies regardless of wether the "show on backpack" box is checked or not.
Is there a way to do it or is the functionnality blocked somehow?

Thanks anyways ;)

@Mentalscars
Copy link

Mentalscars commented Oct 21, 2022

Appreciate you truing to fix it but I have the same issue as the person above, it shows every currency no matter what, also live and classic use different versions of the addon so I don't think you need to worry about the live stuff you put in

@PunkROC01
Copy link

Addresses #707

Refactored a bit of the currency frame module to allow it to work with the currency API in Wrath Classic. With the old version, the GetCurrency local variables that were just making copies of the globals in Retail were overwriting those functions of the same name in Wrath Classic. Therefore, they needed to be renamed.

Otherwise, everything else is pretty straightforward. I wrote some functions to handle the differences between the two in an attempt to minimize the changes to the pre-existing codebase (159-181). Please have a look at these and let me know if you have a better solution in mind.

Lastly, I needed to take the GetCurrencyListInfo() from Wrath Classic and convert the resultant data to fit the mold of its retail counterpart. After all of this, it works great on Wrath Classic!

Sadly, I do not have a retail character, so I cannot test this PR on there. There should be no adverse effects, but it'd be helpful if someone could test these changes out and provide feedback on their results.

Here's a picture! =) image

Hey! I was wondering if you still play and are able to update this at all? It broke since the Ulduar 3.4.1 patch

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.

4 participants