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

Revamp avatar system, remove MC avatars from core #3313

Open
wants to merge 27 commits into
base: develop
Choose a base branch
from

Conversation

tadhgboyle
Copy link
Member

@tadhgboyle tadhgboyle commented Apr 11, 2023

No description provided.

@tadhgboyle tadhgboyle added this to the 2.2.0 milestone Apr 11, 2023
@tadhgboyle tadhgboyle force-pushed the feat/avatar-revamp branch 2 times, most recently from 9b88f5e to c62eb56 Compare April 15, 2023 00:08
@tadhgboyle tadhgboyle marked this pull request as ready for review May 7, 2023 15:57
@tadhgboyle tadhgboyle requested review from partydragen and a team and removed request for partydragen May 8, 2023 04:33
<table class="table table-borderless table-striped">
<thead>
<tr>
<th>Name</th>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded text

MinecraftAvatarSource::registerSource(new VisageMinecraftAvatarSource());
}

// EventHandler::registerListener(UserIntegrationUnlinkedEvent::class, ResetAvatarCacheHook::class);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those should be uncommented?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or wait, this is listeners

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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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}")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@partydragen partydragen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And that Minecraft Avatar Source should be default priority, Well below Uploading Image source type

So new sites still using Minecraft avatars by default as its still a CMS designed for Minecraft

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.

None yet

2 participants