Differential D21477

Use the theme file defined sounds if they are declared.
ClosedPublic
Actions

Authored by jjorge on May 29 2019, 8:25 AM.

Details

Reviewers

aacid

Group Reviewers

KDE Games

Commits

R391:35ae7d63e71d: Use the theme file defined sounds if they are declared.

Summary

The theme files define themed sounds, but the code does not use them.
This make it work, tested with Europe theme downloaded through KnewStuff.

Test Plan

Does this need any other action from me to be reviewed?

Thanks.

Diff Detail

Repository

R391 KBlocks

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

jjorge created this revision.May 29 2019, 8:25 AM

Restricted Application added a reviewer: KDE Games. · View Herald TranscriptMay 29 2019, 8:25 AM

Restricted Application added a subscriber: kde-games-devel. · View Herald Transcript

jjorge requested review of this revision.May 29 2019, 8:25 AM

Harbormaster completed remote builds in B12242: Diff 58822.May 29 2019, 8:25 AM

Add .arcconfig file to the project to make contribution easier.

Harbormaster completed remote builds in B12243: Diff 58823.May 29 2019, 8:27 AM

ther'es some small things as not compating against "" but using isEmpty, but i'll fix that myself when commititng

This revision is now accepted and ready to land.May 29 2019, 10:30 PM

Actually no, this is not all good.

The paths only get calculated on app startup not when the theme is changed while the app running.

Can you please fix that?

This revision now requires changes to proceed.May 29 2019, 10:32 PM

Added a loadtheme function to allow sound theme switching on config change.

Harbormaster completed remote builds in B12273: Diff 58901.May 30 2019, 11:39 AM

jjorge edited the test plan for this revision. (Show Details)Jun 3 2019, 5:05 PM

aspotashev added a subscriber: aspotashev.Jun 8 2019, 9:49 PM

You have a memory leak and some other things like the warnings, but i'll fix them myself and commit.

Thanks for the contribution :)

KBlocksSound.cpp
68	This is by far not a warning

aacid accepted this revision.Jun 8 2019, 9:58 PM

This revision is now accepted and ready to land.Jun 8 2019, 9:58 PM

Closed by commit R391:35ae7d63e71d: Use the theme file defined sounds if they are declared. (authored by jjorge, committed by aacid). · Explain WhyJun 8 2019, 10:06 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

		Path
A	M	.arcconfig (3 lines)
M		CMakeLists.txt (2 lines)
M		KBlocksScene.cpp (4 lines)
M		KBlocksSound.h (11 lines)
M		KBlocksSound.cpp (50 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	58822	5340020		May 29 2019, 8:25 AM	★	★
Diff 2	58823	5340020	- Add .arcconfig file to the project to make contribution easier.	May 29 2019, 8:27 AM	★	★
Diff 3	58901	5340020	- Added a loadtheme function to allow sound theme switching on config change.	May 30 2019, 11:39 AM	★	★
Diff 4	59424	f5d8b1c	R391:35ae7d63e71db4e2d824081d7e05c1a484755be8	Jun 8 2019, 10:06 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
1d460225c31d	d812e7e91020	4b23138a4a8a	José JORGE [zezinho]	Added a loadtheme function to allow sound theme switching on config change. (Show More…)	May 30 2019, 11:38 AM
4b23138a4a8a	b5286ce3037e	96747464a883	José JORGE [zezinho]	Add .arcconfig file to the project to make contribution easier.	May 29 2019, 8:25 AM
96747464a883	5b473539296a	5340020cc39e	José JORGE [zezinho]	Use the theme file defined sounds if they are declared. (Show More…)	May 29 2019, 8:22 AM