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

Draft
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

@SonixApache
Copy link
Contributor

are we on 515 yet

@Exxion
Copy link
Member Author

Exxion commented Aug 21, 2024

Before merging this, we should probably actually set the min client version to 515 so that people don't play on 514 and ask why examine icons are broken. I should also test it again just to make sure I didn't forget anything, but as far as I remember, this is ready

@Exxion Exxion removed the ✋ Do Not Merge ✋ Don't you do it. label Aug 21, 2024
@SECBATON-GRIFFON
Copy link
Contributor

will this close all the t-thanks BYOND issues with overlays looking bad on right click icons or not

@Eneocho
Copy link
Collaborator

Eneocho commented Aug 21, 2024

2 years man

@Exxion
Copy link
Member Author

Exxion commented Aug 21, 2024

will this close all the t-thanks BYOND issues with overlays looking bad on right click icons or not

It won't have any effect on right click icons. Not directly, at least.

2 years man

Part of that delay is our fault, but also Lummox took goddamn forever to release 515 into stable

@Exxion Exxion marked this pull request as draft September 8, 2024 07:05
@Exxion
Copy link
Member Author

Exxion commented Sep 8, 2024

Testing found some significant bugs, which will probably be easy to fix when it's not 3 AM

@SECBATON-GRIFFON
Copy link
Contributor

oh boy 3AM!!!

@SonixApache
Copy link
Contributor

it's always 3 AM somewhere in the world

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.

5 participants