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

Re-enable cg recolor #6911

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Re-enable cg recolor #6911

wants to merge 8 commits into from

Conversation

denaelc
Copy link
Contributor

@denaelc denaelc commented Oct 30, 2024

Fixed the bug, very minor changes. Somehow forgot a try-catch block which sometimes resulted in a nullpointer exception

Fixed the bug, very minor changes. Somehow forgot a try-catch block which sometimes resulted in a nullpointer exception
@runelite-github-app
Copy link

runelite-github-app bot commented Oct 30, 2024

@Adam-
Copy link
Member

Adam- commented Oct 30, 2024

Can you make this use the PreMapLoad event instead?
I'd prefer if this plugin wasn't readded as is; the way it works is by invalidating the gpu scene cache causing everything to be streamed each frame, which just gives very poor performance, and it isn't clear to users the reason why. Additionally, if you are getting npes from getModel(), it means you are holding onto and calling getModel() on despawned objects, which is just very wrong.

@denaelc
Copy link
Contributor Author

denaelc commented Oct 30, 2024

Yeah, that’s why I took it down in the first place instead of just pushing a quick fix. I may be tied up for the next couple of days, and since some users requested that this be uploaded again soon, I decided to re-upload it in a working state with only these minor changes. I’ll make the "bigger" changes in the near future if that's okay with you. If not, that's totally fine 👍

@LlemonDuck LlemonDuck added the waiting for author waiting for the pr author to make changes or respond to questions label Oct 31, 2024
@runelite-github-app runelite-github-app bot removed the waiting for author waiting for the pr author to make changes or respond to questions label Nov 15, 2024
@denaelc
Copy link
Contributor Author

denaelc commented Nov 15, 2024

I'm now using the new preMapLoad event and removed all instances of "setting the scene ID on objects". I'm also no longer trying to call getModel() on despawned Objects. The performance should now be improved substantially - thanks again for the new event!

@dan-drews
Copy link

Is this one ready to be reviewed? I've been missing this plugin, hope it'll be available soon :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants