server icon indicating copy to clipboard operation
server copied to clipboard

Remove avatar related commands from protocol and tables from model

Open Brutus5000 opened this issue 4 years ago • 10 comments

Avatars are managed by the API. There is no more need to have this in the protocol except for python client.

Why now? We recently removed the url column from the database and replaced it with a filename column. The main reason behind this change is that we want to decouple the filename from the url (we want to be able to move file around if we need to). Then the prefix to the content server should be configuration only (which it is now in the API).

For now the url column is kept for backward compatibility by adding the prefix hardcoded, but we want to remove this. Therefore we either put the url prefix as configuration in the server or we just remove the protocol and the tables from the server which reduces complextiy which is unneeded.

This will break the avatars in the python client, but due to url encoding issues we solved now in the api they are partially broken already. Alternative for the python client: Use the avatar endpoints available since Q1 2018.

Brutus5000 avatar Jun 25 '20 20:06 Brutus5000

Only thing about using the API for avatars is that the server won't be able to send updates when people change their avatar. So basically the only way to change your avatar is to log out and back in (which I think causes inconsistent UI state since the client shows your new avatar as being selected immediately)

Askaholic avatar Jul 01 '20 18:07 Askaholic

Changing the avatar is triggered by the client, isn't it? So the client should be able to manage it's own state when making changes.

Brutus5000 avatar Jul 01 '20 20:07 Brutus5000

Yea but other players won't be notified that you changed your avatar. Somehow you need your client change to trigger a client change in other people's clients. Which you could do by logging out and back in, or if you are already changing your avatar through the server protocol, then the server can just notify everyone straight away that your avatar changed (that's what happens right now).

Askaholic avatar Jul 01 '20 20:07 Askaholic

I am pretty sure the user details are updated regularly (probably if you jump back into the chat tab), otherwise we wouldn't have that bug that the most active clans got queried 70k times per day.

Alternatively: send that it has changed, but the details are to be queried via api.

Brutus5000 avatar Jul 01 '20 20:07 Brutus5000

Now that the chat list bugs have been fixed and the avatars are properly cached in the client it would require a message that the avatar has changed.

Sheikah45 avatar Jun 09 '21 11:06 Sheikah45

I'm kindof leaning towards closing this issue. I'm not sure we really want to do this.

Askaholic avatar Jun 11 '21 02:06 Askaholic

i can work on this. Do i understand right that i only have to remove the tables, all functions that use them and the unittests of those functions ? or do i have to change the functions to use the api instead ?

KaukaHan avatar Nov 11 '21 13:11 KaukaHan

This evolved a little bit beyond a good first issue. As right now the python server sends out the changes to the avatar made by the player to all connected players. If this were to be removed then players would need to reconnect in order to see changes to avatars unless the api also broadcasts those changes. For this reason it is unclear if we want to remove the avatar commands.

Sheikah45 avatar Nov 11 '21 13:11 Sheikah45

would such a addition to the api be something achievable for a newbie ? if there is some broadcarst for another purpose i can try to use that as a blueprint.

KaukaHan avatar Nov 14 '21 18:11 KaukaHan

I would say yes, but I'd recommend you focus on the change in this application while someone else (e.g. I or maybe kubko) make sure the API is actually sending these messages via RabbitMQ.

You'd need to setup some logic to listen to RabbitMQ queues (including creating them if missing). I think there is no case in the server to copy and paste similar stuff. But in general attaching to RabbitMQ and sending out a message via the lobby connections shouldn't be uber complicated :D

Brutus5000 avatar Nov 15 '21 17:11 Brutus5000