Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Splitted "Self muted/deafened" into seperate message events. #142

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

Marscho commented Jan 2, 2013

Self muted/deafened message seperated into self muted,
self unmuted, self deafened, self undeafened. Grants an audio feedback what mute/deaf-state is active. (global keys)
German translation added for these new messages.

Update 03.01.2012:

  • Moved the new message-events to the back of the message-list to prevent old config files from becoming imcompatible.
    • In my tests it worked both to switch from old to new and from new to old.
  • Added an array that allows to choose the order of the messages in the settingsscreen
  • Fixed English/German translation files
@Marscho Marscho Self muted/deafened message seperated into self muted, self unmuted, …
…self deafened, self undeafened. Grants an audio feedback what mute/deaf-state is active. (global keys)
2f7ace7

@hacst hacst and 1 other commented on an outdated diff Jan 2, 2013

src/mumble/Log.h
@@ -71,7 +71,7 @@ class Log : public QObject {
Q_OBJECT
Q_DISABLE_COPY(Log)
public:
- enum MsgType { DebugInfo, CriticalError, Warning, Information, ServerConnected, ServerDisconnected, UserJoin, UserLeave, Recording, YouKicked, UserKicked, SelfMute, OtherSelfMute, YouMuted, YouMutedOther, OtherMutedOther, ChannelJoin, ChannelLeave, PermissionDenied, TextMessage };
+ enum MsgType { DebugInfo, CriticalError, Warning, Information, ServerConnected, ServerDisconnected, UserJoin, UserLeave, Recording, YouKicked, UserKicked, SelfMute, SelfUnmute, SelfDeaf, SelfUndeaf, OtherSelfMute, YouMuted, YouMutedOther, OtherMutedOther, ChannelJoin, ChannelLeave, PermissionDenied, TextMessage };
@hacst

hacst Jan 2, 2013

Owner

The indexes in this enumeration are used for config loading/saving. If you add in the middle old configurations will no longer apply and will be loaded incorrectly.

I see three options:

  • Use uiUpdateCounter to update the format (be careful though, it'll be 0 on first run regardless of version).
  • Only append to the enum. You'll have to uncouple display from enum order though
  • Convert the index based saving to a real dict and only use the enum during runtime

I guess those all have their pro's and con's ;)

@Marscho

Marscho Jan 3, 2013

Contributor

I did go with option number 2.

It should also make it easier for future changes of the message-events.
Just append the new event to the enum and enter the position in the array where to display the message in the settingsscreen.

Thanks for your advice!

Owner

hacst commented Jan 2, 2013

General note: Be aware that as this contains new strings we won't be able to include this until 1.2.4 is released.

@Marscho Marscho -Moved the new message-events to the back of the message-list to prev…
…ent old config files becoming imcompatible.

-Added an array that allows to choose the order of the messages in the settingsscreen
f9242fd

@hacst hacst commented on an outdated diff Jan 4, 2013

src/mumble/Log.cpp
@@ -196,6 +200,33 @@ void LogConfig::browseForAudioFile() {
qdDate = QDate::currentDate();
}
+//- Displayorder in settingsscreen, allows to insert new events without breaking config-compatibility with older versions (Top = 0)
@hacst

hacst Jan 4, 2013

Owner

You should use the enum type instead of integer indices. With those you also don't need the comments and things become more readable. Here's a patch for that (untested):
http://d0t.dbclan.de/dump/mumble_patches/0001-Use-enum-type-and-constants-instead-of-integer-indic.patch

Tested your patch, works like a charm.
It also looks way better than my commented array.

Thanks for the help!

@ghost ghost assigned hacst Dec 7, 2013

Owner

hacst commented Sep 20, 2014

Sorry for taking so long to get back to this. Merged as:

570a1f2 Translation update
acd8174 Use enum type and constants instead of integer indices.
f0fc66b Self muted/deafened message seperated into self muted...

Code is very much equivalent to this PR. Did a rebase, minor whitespace and format changes and squashed your two commits into one as they weren't logically independent. I had to remove the translation file changes as with out transifex translation process we no longer edit them directly (see http://blog.mumble.info/changes-in-translation-workflow/ ).

Thanks a lot for your contribution and sorry again for taking so long to merge this back.

@hacst hacst closed this Sep 20, 2014

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