classicdb / database

Classic DB is a content database for CMaNGOS Classic: world, NPCs, objects, quests and so on.
https://github.com/cmangos/mangos-classic
Other
87 stars 56 forks source link

Eye of Sulfuras not usable #848

Closed Xentr0 closed 8 years ago

Xentr0 commented 8 years ago

Eye of Sulfuras is not usable, it will always give the error that it is on cooldown.

This is fixed by changing: (17204,7,0,'Eye of Sulfuras',29175,5,1024,1,800000,200000,0,-1,-1,60,0,0,0,0,0,0,0,0,0,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,21160,0,-1,0,-1,0,-1,0,0,0,0,-1,0,-1,0,0,0,0,-1,0,-1,0,0,0,0,-1,0,-1,0,0,0,0,0,0,0,1,'',0,0,0,0,0,2,0,0,0,0,0,0,0,0,'',0,0,0,0,0,0), to (17204,7,0,'Eye of Sulfuras',29175,5,1024,1,800000,200000,0,-1,-1,60,0,0,0,0,0,0,0,0,0,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,21160,0,0,0,-1,0,-1,0,0,0,0,-1,0,-1,0,0,0,0,-1,0,-1,0,0,0,0,-1,0,-1,0,0,0,0,0,0,0,1,'',0,0,0,0,0,2,0,0,0,0,0,0,0,0,'',0,0,0,0,0,0),

Inside of the SQL file.

I don't know if this is the right way to mention this but it's my first time suggesting an improvement, it's a small little bug that I found while trying to run my own server.

Xentro

Muehe commented 8 years ago

This is definitely the right place to mention it, thanks for doing so. However the big sql file is only changed every now and again when the DB version is upgraded. Inbetween versions you use the small sql files in the updates directory. Your patch would look like:

UPDATE item_template SET spellcharges_1 = 0 WHERE entry = 17204;

Btw. you can make diffs here on Github by writing `````followed by diff instead of a language name like sql, then the code with - in front of old line and + in front of new line and three` again to close the code. It really helps spotting what changed:

-(17204,7,0,'Eye of Sulfuras',29175,5,1024,1,800000,200000,0,-1,-1,60,0,0,0,0,0,0,0,0,0,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,21160,0,-1,0,-1,0,-1,0,0,0,0,-1,0,-1,0,0,0,0,-1,0,-1,0,0,0,0,-1,0,-1,0,0,0,0,0,0,0,1,'',0,0,0,0,0,2,0,0,0,0,0,0,0,0,'',0,0,0,0,0,0),
+(17204,7,0,'Eye of Sulfuras',29175,5,1024,1,800000,200000,0,-1,-1,60,0,0,0,0,0,0,0,0,0,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,21160,0,0,0,-1,0,-1,0,0,0,0,-1,0,-1,0,0,0,0,-1,0,-1,0,0,0,0,-1,0,-1,0,0,0,0,0,0,0,1,'',0,0,0,0,0,2,0,0,0,0,0,0,0,0,'',0,0,0,0,0,0),

Also you can put results from "git diff" in there.

Muehe commented 8 years ago

So, I looked up spellcharges and it turns out -1 is the correct value, as the item should be deleted after using (right?). Maybe this is a core issue afterall, in which case cmangos issues would be the place to report it. Could you check if the item gets deleted with your patch?

Xentr0 commented 8 years ago

it does, it works as intended

Muehe commented 8 years ago

While your patch might solves the issue I am not sure it is the right way to go here because -1 should be working and in fact is working on many other items. I think this might point to core issue. Maybe wait for @cala's opinion on this but according to the (maybe outdated) documentation the DB value is correct.

Xentr0 commented 8 years ago

Well you are right and the "fix" is a bit weird. The tooltip on the item changes to say that you have 0 charges left. So I went back on my server and tested it again on my warrior and I couldn't use the spell, it would start casting and then stop the cast and nothing would happen. However when I try this on a night elf druid it does work. I don't know how or why this is, but as you pointed out it does seem to be a deeper issue here. (The default values don't on either my druid or my warrior) Will do some more testing!

Xentr0 commented 8 years ago

Also just a little note. I didn't know about the documentation on the website so I found the value by comparing an item that works in a similar way (in this case the items that form the portal deck). I don't think the amount of charges themselves don't do much because it's the spell itself that deletes the item. For example, for some portal cards the amount of charges was 1, for some it was 0 but it worked no matter what card you used. I think the hammer just bugs out with -1 charges for some reason. Still testing the classes btw to see if that's the issue.

UPDATE: So I changed the data entry to 1 charge (because it makes more sense) and went back online to test a bunch of different classes. The issue I was having doesn't seem to be class related, however I was unable to use the Eye of Sulfuras with Corrupted Ashbringer Equipped (I think this is because of the buff it gives.) So, so far I guess it's a pretty solid fix.

Neotmiren commented 8 years ago

The suggested fix is correct. Just look into SQL version of .dbc files: the item casts spell which uses the item itself as reagent. So it should not be "consumable on use"... or the player must have 2 Eyes of Sulfuras in the inventory. Server code correctly understands that -1 spellcharges item will be consumed on cast and cannot be valid reagent.

cala commented 8 years ago

I agree with @Neotmiren

@Xentr0 could you please do a pull request for this so I could set correct authorship to the commit?

Xentr0 commented 8 years ago

Well I'd love to but I have no clue how. Really new to Github, sorry.

Muehe commented 8 years ago

Hope I got this right...

Note: The reason for the branching thing is you keep your master branch clean. If your change gets merged you can later delete your change branch, pull your (and other) changes to your master branch and then apply other changes. Easier than doing the whole routine above again.

Xentr0 commented 8 years ago

Thanks a lot Muehe, I got it :)