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

Fix(Core/Color): ClassColor #18911

Closed
wants to merge 4 commits into from
Closed

Conversation

PkllonG
Copy link
Contributor

@PkllonG PkllonG commented May 18, 2024

Changes Proposed:

This PR proposes changes to:

  • Core (units, players, creatures, game systems).
  • Scripts (bosses, spell scripts, creature scripts).
  • Database (SAI, creatures, etc).

Issues Addressed:

  • Closes

SOURCE:

It was extracted from the client and calculated to arrive at it

RAID_CLASS_COLORS = {
	["HUNTER"] = { r = 0.67, g = 0.83, b = 0.45 },
	["WARLOCK"] = { r = 0.58, g = 0.51, b = 0.79 },
	["PRIEST"] = { r = 1.0, g = 1.0, b = 1.0 },
	["PALADIN"] = { r = 0.96, g = 0.55, b = 0.73 },
	["MAGE"] = { r = 0.41, g = 0.8, b = 0.94 },
	["ROGUE"] = { r = 1.0, g = 0.96, b = 0.41 },
	["DRUID"] = { r = 1.0, g = 0.49, b = 0.04 },
	["SHAMAN"] = { r = 0.0, g = 0.44, b = 0.87 },
	["WARRIOR"] = { r = 0.78, g = 0.61, b = 0.43 },
	["DEATHKNIGHT"] = { r = 0.77, g = 0.12 , b = 0.23 },
};
	local classColor = classFilename and RAID_CLASS_COLORS[classFilename] or NORMAL_FONT_COLOR;
	nameLabel:SetFormattedText("|cff%.2x%.2x%.2x%s|r", classColor.r * 255, classColor.g * 255, classColor.b * 255, UnitName("party"..i));
        struct cs
        {
            double r;
            double g;
            double b;
        };

        static cs _cs[10] =
        {
            { 0.67, 0.83, 0.45 },
            { 0.58, 0.51,  0.79 },
            { 1.0, 1.0,  1.0 },
            { 0.96,  0.55,  0.73 },
            { 0.41,  0.8,  0.94 },
            { 1.0,  0.96,  0.41 },
            { 1.0,  0.49,  0.04 },
            { 0.0,  0.44,  0.87 },
            { 0.78,  0.61,  0.43 },
            { 0.77,  0.12,  0.23 }
        };

        for (uint8 i = 0; i < 10; ++i)
        {
            printf("%.2x%.2x%.2x \n", unsigned int(_cs[i].r * 255), unsigned int(_cs[i].g * 255), unsigned int(_cs[i].b * 255));
        }
aad372
9382c9
ffffff
f48cba
68ccef
fff468
ff7c0a
0070dd
c69b6d
c41e3a

The changes have been validated through:

  • Live research (checked on live servers, e.g Classic WotLK, Retail, etc.)
  • Sniffs (remember to share them with the open source community!)
  • Video evidence, knowledge databases or other public sources (e.g forums, Wowhead, etc.)
  • The changes promoted by this pull request come partially or entirely from another project (cherry-pick). Cherry-picks must be committed using the proper --author tag in order to be accepted, thus crediting the original authors, unless otherwise unable to be found

Tests Performed:

This PR has been:

  • Tested in-game by the author.
  • Tested in-game by other community members/someone else other than the author/has been live on production servers.
  • This pull request requires further testing and may have edge cases to be tested.

How to Test the Changes:

  • This pull request can be tested by following the reproduction steps provided in the linked issue
  • This pull request requires further testing. Provide steps to test your changes. If it requires any specific setup e.g multiple players please specify it as well.

Known Issues and TODO List:

  • [ ]
  • [ ]

How to Test AzerothCore PRs

When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].

You can help by testing PRs and writing your feedback here on the PR's page on GitHub. Follow the instructions here:

http://www.azerothcore.org/wiki/How-to-test-a-PR

REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).

For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.

@github-actions github-actions bot added CORE Related to the core file-cpp Used to trigger the matrix build labels May 18, 2024
@PkllonG PkllonG changed the title Fix(Core/Color): ClassColor Fix(Core/Color): ClassColor & QuestColor May 18, 2024
@PkllonG PkllonG changed the title Fix(Core/Color): ClassColor & QuestColor Fix(Core/Color): ClassColor May 18, 2024
@sudlud
Copy link
Member

sudlud commented May 21, 2024

Can you please provide information on how to test these changes?

Thanks!

@PkllonG
Copy link
Contributor Author

PkllonG commented May 21, 2024

I've tested it, and there's no code referencing this

@sudlud
Copy link
Member

sudlud commented May 21, 2024

.gear stats seems to use the function

@sudlud
Copy link
Member

sudlud commented May 29, 2024

what about another PR to use these colors within Chat.h -> playerLink()?

@sudlud
Copy link
Member

sudlud commented May 29, 2024

can you please rerun your conversion script and round the result instead of casting it to "int" directly?

I have the feeling that this would lead to the same values that are already present in AC.

e.g. the first line of your converted data:
{ 0.67, 0.83, 0.45 },
times 255 =
{ 170.85, 211.65, 114.75 },
rounded:
{ 171, 212, 115 },
in HEX:
AB D4 73

which is the existing value for CLASS_HUNTER, whereas you propose AA D3 72 which you would get due to casting the results to ìnt instead of rounding.

@sudlud
Copy link
Member

sudlud commented Jun 4, 2024

I'll close this PR for now, for the reason in my comment above.

IMO when executed correctly this PR would not propose any changes at all as the colors are identical to the client values.

@sudlud sudlud closed this Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CORE Related to the core file-cpp Used to trigger the matrix build Ready to be Reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants