-
-
Notifications
You must be signed in to change notification settings - Fork 117
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
base: master
Are you sure you want to change the base?
Conversation
This makes sense; in the current state, the class colors on the map are often mixed up |
Sorry for the late reply. I have been busy and testing this PR requires me to queue to battle grounds and actually play. 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. 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. |
No. This won't work. The problem is that the buttons |
When checking, did you try to limit the number of updates to 1 second or less? It seems to me that even if you make a delay of 1-2 seconds, this will not become a problem. It will still update very quickly and almost imperceptibly, but performance should improve greatly. |
I'm not speaking about caching the class color per unit, but saving the last-set-color-value from SetVertexColor to avoid the SetVertexColor call while r, g, b would be the same as last time. I have noticed on other similar calls that those functions can be hard on performance even if they set the same color as it already had before. So the idea was to only perform SetVertexColor if |
No, I haven't. I just used the PR as it was. |
Added caching. It should get better. P.S. I forgot to remove the debug output before committing... |
Completely rewrote the implementation of the module.
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