search

wowdev / WoWDBDefs

Client database definitions for World of Warcraft
Other
246 stars 95 forks source link

defs: Fix 4.3.4.15595 incorrect Difficulty DBC ref #105

Closed HelloKitty closed 2 years ago

HelloKitty commented 2 years ago

Difficulty DBC doesn't exist in Cata.

HelloKitty commented 2 years ago

Sorry for the mediocre commit name.

bloerwald commented 2 years ago

Hey, thanks. Before I merge this, to be sure:

Does the table not exist or are the values also different? If the values are identical we usually assume an existing but unshipped table, and reference that virtual table: it still is a reference to that table semantically, and them shipping or not shipping it is not a change to the format of this table.

HelloKitty commented 2 years ago

@bloerwald I cannot comment on the values themselves but during this version the table/DBC didn't exist. Maybe it would be what would be expected in a hypothetical Difficulty DBC if it existed during these versions. On wow.tools these foreign keys don't properly link to anything on these versions since the DBC doesn't exist again until some MoP version.

It's up to you if you want to merge it because even if you don't merge it there is still an inconsistency with the WOTLK/3.x defintions then which refer to it as Difficulty and not DifficultyID. So if you want the defs to refer to tables that existed or were shipped in future versions then maybe the 3.x defs should be updated.

bloerwald commented 2 years ago

Your change is correct: In 4.3.4, Sinestra has Difficulty = 2/3. In 5.0.3 it has DifficultyID = 5/6, which is clearly not the same value, and also Difficulty.OldEnumValue = 2 for Difficulty.ID = 5 and Difficulty.OldEnumValue = 3 for Difficulty.ID = 6. It was indeed a bad name in the definitions and MoP was the version to introduce that table, also server side.

Thanks for the contribution!

HelloKitty commented 2 years ago

Your change is correct: In 4.3.4, Sinestra has Difficulty = 2/3. In 5.0.3 it has DifficultyID = 5/6, which is clearly not the same value, and also Difficulty.OldEnumValue = 2 for Difficulty.ID = 5 and Difficulty.OldEnumValue = 3 for Difficulty.ID = 6. It was indeed a bad name in the definitions and MoP was the version to introduce that table, also server side.

Thanks for the contribution!

Oh, you dug deep into it! Well, glad it was a helpful PR in the end.