Skip to content
This repository has been archived by the owner on Feb 12, 2023. It is now read-only.

Added feature: ability to set custom audio files for qtox events #5406

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Monsterovich
Copy link
Contributor

@Monsterovich Monsterovich commented Oct 23, 2018

Sometimes you want to change notification sound but they were stored inside qtox binary. Audio files must be put in %APPDATA%/qtox/audio otherwise the default sound is used.


This change is Reviewable

Copy link
Member

@sudden6 sudden6 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r1.
Reviewable status: 0 of 1 LGTMs obtained


src/audio/audio.cpp, line 203 at r1 (raw file):

}

QString Audio::getSoundPath(QString fn)

can you align this code for the path structure with the one from https://github.com/qTox/qTox/blob/master/src/widget/style.cpp

so that we have the possibility to have different sound themes. I think <AppDataPath/audio/default/<...> would be a good structure.

@Chiitoo
Copy link
Contributor

Chiitoo commented Oct 24, 2018

Just wanted to say thanks for this. It's one of those things I've been wanting to do for years... but never got to. :]

Definitely agree with the path. Currently on Linux, it would be "$HOME"/.config/tox/audio/, which is certainly not optimal, and this would definitely go well with what the themes will be/are already doing.

@Inari-Whitebear
Copy link

So this is pretty old now, any idea if this will still go anywhere? Would be a nice feature.

If not, what are the rules for picking up dead PRs? Do I use the existing PR and modify it? Do I make my own code that does the same change?

@anthonybilinski
Copy link
Member

Making changes on top of the existing branch and creating a new PR would be good. It'll need to be rebased up to tip since there are some conflicts.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants