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

mana conversion mod tweaks #2491

Merged
merged 4 commits into from
Dec 5, 2019
Merged

Conversation

gmriggs
Copy link
Collaborator

@gmriggs gmriggs commented Dec 3, 2019

The main purpose of this patch was to fix a bug where appraisal was showing Mana Conversion Bonus: +0% for a many casting items

At first I thought this was a bug in the loot generator, but after discussing it with @harliq, it turns out there are actually many casting items with base weenies that have PropertyFloat.ManaConversionMod=0 in the data

So to account for this, Appraisal is now checking if ManaConversionMod exists, but is 0, it removes it.

I also noticed some possible bugs with Hermetic Void, which sounds like it is cast on the wand, similar to the other item debuffs. The logic for ManaConversionMod enchantments should be taking both auras and spells cast directly on the item into account for this.

Update: it has been discovered from retail pcaps that retail also had this bug.

Retail also didn't normalize ManaConversionMod on the items -- some of the base weenies have 0, some of them have null. The retail pcaps had ManaConversionMod=0 in many instances (even while unbuffed), but there were also a few rare instances of ManaConversionMod not being sent.

The 'show_mana_conv_bonus_0' server option has been added, which defaults to true to match retail behavior.

@Warloxx01
Copy link

Hi, I added this and the (Null mana c) to a clean server build and this happened when I was playing in Metos...
Error.txt

This didnt crash my server.

@gmriggs
Copy link
Collaborator Author

gmriggs commented Dec 4, 2019

Hey @Warloxx01,

Thanks for the bug report!

I have checked in an update that should hopefully fix this error. Please let me know if you find anything else

@ziang4891
Copy link
Contributor

ziang4891 commented Dec 5, 2019

I can confirm that the null checks fix the exception, on appraisal

@gmriggs gmriggs merged commit b7062b2 into ACEmulator:master Dec 5, 2019
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

4 participants