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

mapcolors: rewrite module implementation #1255

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Artur91425
Copy link
Contributor

@Artur91425 Artur91425 commented Feb 9, 2024

Completely rewrote the implementation of the module.

  • Fixed a bug with incorrect class colors determination.
  • Added coloring of nicknames when mouseover on frames. This feature can be disabled in the settings.
  • The new implementation does not use OnUpdate all the time.

90% of the time the game is played, there is no need to update this data... Therefore, in the new implementation, the data is updated only when it is really necessary.
Data updating also works on OnUpdate, but now only when the WorldMapFrame and/or BattlefieldMinimap are displayed.

I tested and debugged everything for several days. There shouldn't be any problems

@shikulja
Copy link
Contributor

shikulja commented May 7, 2024

This makes sense; in the current state, the class colors on the map are often mixed up
https://youtu.be/kOfugzU3NMw

@shagu
Copy link
Owner

shagu commented Jun 3, 2024

Sorry for the late reply. I have been busy and testing this PR requires me to queue to battle grounds and actually play.
Which is why I have postponed it all the time till I forgot about it.

First of all: I do like the the code and it's great to see a fix for the broken colors here.

However in my tests the average execution time per update went from (old) 0.0072s to 0.026s.

Old:
Bildschirmfoto vom 2024-06-03 10-30-13

New:
Bildschirmfoto vom 2024-06-03 10-29-10

Of course, my tests aren't perfect and it might behave different on different machines. But having a factor 10 less performance is in general a sign that soon people will get performance issues with that.

So I don't want to enforce it to everyone as it is right now. Maybe caching the last applied color per icon to not overwrite it each frame, would already be enough? I didn't looked into solutions to it yet.

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

3 participants