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

Changes bicon() to use the builtin 515 feature #33764

Open
wants to merge 1 commit into
base: Bleeding-Edge
Choose a base branch
from

Conversation

Exxion
Copy link
Member

@Exxion Exxion commented Dec 2, 2022

Obviously DNM until we're on 515.
515 makes this Just Work. It's clientside and even includes overlays (though not vis_contents, which may or may not be a bug). Includes a couple hacks to ensure it works in edge cases. We could finally add examine icons back to humans with this. They never actually worked properly before, but they do with this.
I should probably also excise all other uses of icon2base64() as well but I haven't yet

Just opening because I want to remember it and also clean up my local copy

@Exxion Exxion added the ✋ Do Not Merge ✋ Don't you do it. label Dec 2, 2022
@d3athrow
Copy link
Collaborator

we are on 515, should this be merged or cleaned up?

@d3athrow d3athrow marked this pull request as ready for review June 23, 2023 17:22
@Exxion
Copy link
Member Author

Exxion commented Jun 23, 2023

It would require us to require 515 clients, and there seem to be some issues with 515 clientside. I'm told it has crashing issues (though haven't seen it myself) and also the admin Special panel just doesn't seem to show up at all. The other skin issues seem to be fixed, though.

@Eneocho
Copy link
Collaborator

Eneocho commented Jan 30, 2024

Is this any closer to being mergeable?

@SECBATON-GRIFFON
Copy link
Contributor

apparently so, the chicken has decreed 515 switch will be mandatory soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✋ Do Not Merge ✋ Don't you do it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants