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

Added eye wheel. Render only on ddrace/race/ictf+/dm+/gctf+ server. #317

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@Chairn
Contributor

Chairn commented Aug 30, 2015

Eye wheel, use current tee skin for rendering. Works only on ddrace/race servers, and servers with a '+' in their gametype.

@heinrich5991

This comment has been minimized.

Contributor

heinrich5991 commented Aug 30, 2015

Perhaps we should use a strategy for finding out whether the server supports this other than using its user agent string ("game type"). :)

@Chairn

This comment has been minimized.

Contributor

Chairn commented Aug 30, 2015

We can try /emote when joining server and see the server's answer.

For now, ive just tried several server, which is indeed, not the best.

@heinrich5991

This comment has been minimized.

Contributor

heinrich5991 commented Sep 10, 2015

So. I guess this would be fine to merge now, but we should try to enhance the feature detection once #326 is merged. Will test the pull request.

@heinrich5991

This comment has been minimized.

Contributor

heinrich5991 commented Sep 10, 2015

Mh. My impressions after testing it:

  1. Why can you select the normal face expression of a tee? It doesn't seem to do anything.
  2. I'm not sure the eye wheel is all to useful – after all you can modify the eyes using the normal speech-bubble emoticons as well, which is a lot more noticable.
@Chairn

This comment has been minimized.

Contributor

Chairn commented Sep 10, 2015

  1. It should reset emote if there is already one in progress.
  2. I agree, that's why i added cl_eye_wheel. But you can also just want to show that your happy or angry without the emoticon which last only few secs.
  3. Maybe i should add cl_eye_duration to set the duration of the emote?
@heinrich5991

This comment has been minimized.

Contributor

heinrich5991 commented Sep 11, 2015

I personally don't think this is useful enough to be a feature. I'm not the one who can reject ideas, so I guess you'll have to wait for feedback from someone else.

@nzyuzin

This comment has been minimized.

Contributor

nzyuzin commented Sep 11, 2015

The feature seems cool to me, I would love to be able to change facial expression of my tee without evoking emotions. :^)

Though I haven't seen the implementation, maybe you could attach a screenshot to PR so it would be easy for others to see how you implemented eye selection from user perspective?

@heinrich5991

This comment has been minimized.

Contributor

heinrich5991 commented Sep 11, 2015

@nzyuzin

This comment has been minimized.

Contributor

nzyuzin commented Sep 11, 2015

Woah, that's absolutely fantastic!
Great job! 👍

@MyNewBie

This comment has been minimized.

MyNewBie commented Sep 11, 2015

HClient already have it !

@H-M-H

This comment has been minimized.

Member

H-M-H commented Sep 17, 2015

In my opinion this is only useful if this eyeemotes are permanent (last for a long time) because as heinrich5991 already pointed out normal emotes are sufficient in case you just want to change your eyes for a short time. Thus I suggest to set a high value as default.

@H-M-H

This comment has been minimized.

Member

H-M-H commented Oct 21, 2015

@Chairn
So I just tested this locally and found some bugs:

  1. on non ddrace or +... servers the selector is hidden but the chat message can be generated anyway
  2. if you release between emote normal and blink some random text is spammed

Something else I would like to be improved:
int m_SelectedEmote;
int m_SelectedEmoticon;

// this is just confusing, no-one will get the difference at first glance thus something like m_SelectedEyeEmote would be way better (same for functions btw)

@heinrich5991

This comment has been minimized.

Contributor

heinrich5991 commented Oct 21, 2015

Do we actually want this? I mean the idea is nice and all, but does it actually have proper usability? Or is it just a feature that everyone thinks looks nice but wouldn't actually use themselves.

@H-M-H

This comment has been minimized.

Member

H-M-H commented Oct 21, 2015

Well I put it into my personal client some time ago and used it regulary, though with a long duration.
Edit: For some more opinions see responses to http://forum.ddnet.tw/viewtopic.php?f=5&t=559#p4867

@H-M-H

This comment has been minimized.

Member

H-M-H commented Nov 15, 2015

see #373

@H-M-H H-M-H closed this Nov 15, 2015

@Chairn Chairn deleted the Chairn:eye_wheel branch Jul 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment