Revamp avatar system, remove MC avatars from core #3313
Conversation
tadhgboyle
force-pushed
the
feat/avatar-revamp
branch
2 times, most recently
from
9b88f5e
to
c62eb56
Compare
tadhgboyle
force-pushed
the
feat/avatar-revamp
branch
from
e1a4285
to
eaa1312
Compare
tadhgboyle
requested review from
partydragen and
a team
and removed request for
partydragen
| <table class="table table-borderless table-striped"> | ||
| <thead> | ||
| <tr> | ||
| <th>Name</th> |
| MinecraftAvatarSource::registerSource(new VisageMinecraftAvatarSource()); | ||
| } | ||
|
|
||
| // EventHandler::registerListener(UserIntegrationUnlinkedEvent::class, ResetAvatarCacheHook::class); |
There was a problem hiding this comment.
Those should be uncommented?
| public function get(User $user): ?string { | ||
| $base_url = ($this->_full_url ? rtrim(URL::getSelfURL(), '/') : '') . ((defined('CONFIG_PATH')) ? CONFIG_PATH . '/' : '/') . 'uploads/avatars'; | ||
|
|
||
| if (Settings::get('custom_user_avatars') && $user->data()->has_avatar) { |
There was a problem hiding this comment.
Allow custom user avatars? setting on custom avatar source does not save on enabled state
| } | ||
|
|
||
| foreach ($exts as $ext) { | ||
| if (file_exists(ROOT_PATH . "/uploads/avatars/{$user->data()->id}.{$ext}")) { |
There was a problem hiding this comment.
Need to be a way to remove or unselect default custom avatar
People most likey want Custom avatar to be priority one, and minecraft next but if they have uploaded a custom default image then they are not able to fallback to minecraft avatar
| $cache_key = $user_id . '_' . $source->getSafeName() . '_' . $size . '_' . (int) $full_url; | ||
| if ($this->_cache->isCached($cache_key)) { | ||
| $avatar = $this->_cache->retrieve($cache_key); | ||
| if ($avatar) { |
There was a problem hiding this comment.
Isn't this check a bit redundant? Seeing as you already checked if it's in there?
| } | ||
| // Fallback to initials avatar | ||
| if (!isset($url)) { | ||
| $url = $this->_sources[InitialsAvatarSource::class]->getAvatar($user, $size, $full_url); |
There was a problem hiding this comment.
You never store this in cache if it fails, which would mean we attempt to re-fetch it from the respective avatar source every page load until it works. Is this intentional?
| protected function get(User $user): ?string { | ||
| $minecraft_integration = $user->getIntegration('Minecraft'); | ||
| if ($minecraft_integration !== null) { | ||
| $identifier = $minecraft_integration->data()->identifier; | ||
| } else { | ||
| $identifier = $user->data()->username; | ||
| // Fallback to steve avatar if they have an invalid username | ||
| // TODO: what is this regex? | ||
| if (preg_match('#[^][_A-Za-z0-9]#', $identifier)) { | ||
| $identifier = 'Steve'; | ||
| } | ||
| } | ||
|
|
||
| return self::getAvatarFromIdentifier($identifier, $this->_size); | ||
| } |
There was a problem hiding this comment.
We should probably check if the uuid is an offline one and use usernames instead if this is the case. This should fix the fact that when force premium accounts is disabled, people only receive steve skins. Nameless generates an offline uuid when force premium accounts is enabled which would still make this use that offline uuid for retrieving accounts
No description provided.