BAndysc / WoWDatabaseEditor

Integrated development environment (IDE), an editor for Smart Scripts (SAI/smart_scripts) for TrinityCore based servers. Cmangos support work in progress. Featuring a 3D view built with OpenGL and custom ECS framework
MIT License
445 stars 152 forks source link

Conditions using CONDITION_REPUTATION_RANK #219

Closed Odyssey closed 1 week ago

Odyssey commented 6 months ago

Describe the bug When working with conditions using CONDITION_REPUTATION_RANK, the editor uses the wrong field type and values for ConditionValue2 (Reputation Rank).

When generating sql, it writes the index of the element in the selectabable field instead of the corresponding bitmask value. It is also not treated as a bitmask, so it's not possible to create a mask containing more than one reputation rank.

Because of that, reputation based conditions created with the editor may often be wrong (see the reproducing section).

Version (commit hash) d83de4265ddfc05e3dacfd0a915aee50a010db71

To Reproduce Steps to reproduce the behavior:

  1. As an example let's use gossip_menu_id 8558 and 8560, corresponding to The Aldor and The Scryers reputation flightmasters from TBC SELECT * FROM conditions WHERE SourceTypeOrReferenceId=15 AND SourceGroup IN (8558,8560);
  2. Those two characters are supposed to be flightmasters for reputation ranks >= friendly with factionIds 932 and 934 respectively, and are currently only set to friendly difficulty, so when someone reaches honored, revered or exalted they cannot use the flightmaster point anymore (as the field needs to be conditionValue2=16|32|64|128 bitmask value for that to happen).
  3. In WowDatabaseEditor, search for gossip_menu_option for MenuId 8558 or 8560
  4. Notice you cannot switch conditionValue2 (faction rank) to a mask with multiple ranks at the same time, even though the field is a bitmask.
  5. Switch reputation rank to any value, accept, and check the generated sql.
  6. Notice the generated value corresponds to the element index instead of the bitmask value for said rank.

Expected behavior Should be able to stack multiple reputation ranks as a bitmask and should output the right values to generated sql. Should not be described as "minimum rank" but rather "rank mask".

Screenshots TC Wiki, conditions table Condition editor for gossip_menu_option with the example above SQL output generated with rank=friendly (4 instead of 16)

fangshun2004 commented 4 months ago

Fixed in https://github.com/BAndysc/WoWDatabaseEditor/pull/224. Shows that it has been merged into the main branch. But for some unknown reason, the json in the main branch hasn't changed

BAndysc commented 4 months ago

Fixed in #224. Shows that it has been merged into the main branch. But for some unknown reason, the json in the main branch hasn't changed

The json is changed:

https://github.com/BAndysc/WoWDatabaseEditor/blob/master/WDE.Conditions/SmartData/conditions.json#L593

Is there any problem with it?

fangshun2004 commented 4 months ago

https://github.com/BAndysc/WoWDatabaseEditor/blob/ad03280fc13e3544e46d0b1ace550973c72ef4f6/WDE.Conditions/SmartData/conditions.json#L104 It's still "ReputationRankParameter https://github.com/BAndysc/WoWDatabaseEditor/pull/224/files It has been changed to “ReputationRankMaskParameter” and merged

Odyssey commented 1 week ago

All good nowadays.