XF 2.0 How to correctly override fnAvatar function?

[CMTV]

Hi!

I am trying to add support for my custom avatar type "minecraft". Avatar type is used in switch construction in the middle of a huge function called fnAvatar (XF\Template\Templater):

/* ... a lot of code ... */

switch ($avatarType)
{
    case 'gravatar':
    case 'custom':
    case 'minecraft': // <------------ I only need to add this line
        $src = $user->getAvatarUrl($size, $forceType, $canonical);
        break;
    case 'default':
    default:
        $src = null;
        break;
}

/* ... a lot of code ... */

So I only need to put one line in switch block. How can I do so without copying the whole function code in overriding function?

 

[Chris D]

In this example, how is $avatarType being set to minecraft? Are you passing it in as the forcetype attribute or is it worked out elsewhere?

 

[CMTV]

No. I overrided getAvatarType() function:

public function getAvatarType()
{
    $avatarType = parent::getAvatarType();

    if($this->isUsingMinecraftSkinHeadAsAvatar())
    {
        $avatarType = 'minecraft';
    }

    return $avatarType;
}

But it also can be set as forcetype attribute.

Here is how fnAvatar gets avatar type:

$avatarType = $forceType ?: $user->getAvatarType();

 

[Chris D]

Fair enough.

In that case, probably undo the change to getAvatarType() and then:

public function fnAvatar($templater, &$escape, $user, $size, $canonical = false, $attributes = [])
{
    if ($user instanceof \XF\Entity\User)
    {
        $forceType = $this->processAttributeToRaw($attributes, 'forcetype', '', true);
        $avatarType = $forceType ?: $user->getAvatarType();

        if ($user->isUsingMinecraftSkinHeadAsAvatar())
        {
            $avatarType = 'minecraft';
        }
     
        if ($avatarType == 'minecraft')
        {
            $attributes['forcetype'] = 'minecraft';
        }
    }
    return parent::fnAvatar($templater, $escape, $user, $size, $canonical, $attributes);
}

Because you're not explicitly overriding the user's default avatar type, it will still be listed as custom / gravatar, which means the code will get to getAvatarUrl() and have a $forceType of minecraft.

You'd then just extend getAvatarUrl() and look out for the $forceType being minecraft and return the appropriate URL.

I think that will work but obviously haven't really tested that. There might be some edge cases, for example if we have set forcetype anywhere, you may not want to override that.

 

[CMTV]

Unfortunately it is not working.

This code in fnAvatar:

$avatarType = $forceType ?: $user->getAvatarType();

is executed before switch block so it falls to default case...

Ofcourse I can treat Minecraft avatars as custom avatars but it is not correct conceptually because there is a gravatar avatar type.

 

[Chris D]

This line in the extended code:

$attributes['forcetype'] = 'minecraft';

Should make this line in the original code:

$avatarType = $forceType ?: $user->getAvatarType();

Evaluate to minecraft.

The only other way would be to mostly override the entire function.

You could of course just forego extending the fnAvatar() method at all, and check your isUsingMinecraftSkinHeadAsAvatar() method (and any other conditions) inside getAvatarUrl().

 

[CMTV]

The only other way would be to mostly override the entire function.

Yes. It seems to be the only way...

You could of course just forego extending the fnAvatar() method at all, and check your isUsingMinecraftSkinHeadAsAvatar() method (and any other conditions) inside getAvatarUrl().

But it will only work if user is using custom avatar/gravatar and will not work if user has no avatar yet. So the idea of creating truly custom avatar types (when many addons adds their own avatar types) seems to be broken for me.

 

[CMTV]

Ok. I guess I found the correct solution.

I do override getAvatarType() function because I think it is important for other systems to know that current avatar type is not custom,gravatar or default:

public function getAvatarType()
{
    $avatarType = parent::getAvatarType();

    if($this->isUsingMinecraftSkinHeadAsAvatar())
    {
        $avatarType = 'minecraft';
    }

    return $avatarType;
}

fnAvatar function is not used when defining avatar type so it is completely not important which avatar type I do pass to it. So I just pass custom avatar type to it as I need it to call getAvatarUrl() function. Moreover, it will correctly handle forcetypes:

public function fnAvatar($templater, &$escape, $user, $size, $canonical = false, $attributes = [])
{
    if ($user instanceof User)
    {
        $forceType = $this->processAttributeToRaw($attributes, 'forcetype', '', true);
        $avatarType = $user->getAvatarType();

        if((!$forceType && $avatarType == 'minecraft') || $forceType == 'minecraft')
        {
            $attributes['forcetype'] = 'custom';
            $avatarOutput = parent::fnAvatar($templater, $escape, $user, $size, $canonical, $attributes);

            $avatarOutput = preg_replace(
                '/src\s*=\s*"(.+?)"/',
                'src="' . $user->getAvatarUrl($size, 'minecraft', $canonical) . '"',
                $avatarOutput
            );

            return $avatarOutput;
        }
    }

    return parent::fnAvatar($templater, $escape, $user, $size, $canonical, $attributes);
}

Finally I override getAvatarUrl() function:

public function getAvatarUrl($sizeCode, $forceType = null, $canonical = false)
{
    if($forceType == 'minecraft' || $this->isUsingMinecraftSkinHeadAsAvatar())
    {
        return $this->getMinecraftSkinHeadUrl($sizeCode);
    }

    return parent::getAvatarUrl($sizeCode, $forceType, $canonical);
}

It is working. Is it conceptually correct?

 

[Chris D]

Yeah that was an approach I considered. Generally regex is an ugly way to solve these sorts of things, but if it works and works well, then I don't see an issue with it.

An alternative approach is to use a DOM parser but for something which is called so frequently that's probably a bit heavy.

I think longer term, some sort of system to manage different avatar types would be useful. I'll make a note of that.