Paths

Table of Contentst

Differential D4500

Don't add default tracks, if civ.json overrides them.
ClosedPublic
Actions

Authored by Stan on Feb 22 2022, 12:25 AM.

Details

Reviewers

asterix

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP26542: Only add default music tracks if civ.json doesn't override them.

Summary

Currently the following tracks are played no matter the civ

	"An_old_Warhorse_goes_to_Pasture.ogg",
	"Calm_Before_the_Storm.ogg",
	"Juno_Protect_You.ogg"

This can break the experience if you expect only asian themed songs to be played;

Test Plan

Agree, test that for the han, only han musics are played

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

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

Event Timeline

Stan created this revision.Feb 22 2022, 12:25 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/6796/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/5704/display/redirect

Harbormaster completed remote builds in B19602: Diff 19731.Feb 22 2022, 12:40 AM

Stan requested review of this revision.Feb 22 2022, 12:40 AM

I agree. @marder @Freagarach @wowgetoffyourcellphone can you test this as well?

This revision is now accepted and ready to land.Feb 23 2022, 9:04 AM

Increased ease of modding seems good.

binaries/data/mods/public/gui/common/music.js
84 ↗	(On Diff #19731)	So if there is one track, it will be played over and over again?
85 ↗	(On Diff #19731)	Is this called often? (I assume not.)

Stan added inline comments.Mar 4 2022, 7:58 AM

binaries/data/mods/public/gui/common/music.js
84 ↗	(On Diff #19731)	Yep. Another option would be to have an option in the civ.json
85 ↗	(On Diff #19731)	Once at the beggining of the match I think

Freagarach added inline comments.Mar 4 2022, 8:01 AM

binaries/data/mods/public/gui/common/music.js
84 ↗	(On Diff #19731)	As long as it is a known and deliberate side-effect. :) (Maybe check for the absense of `this.tracks[musicType]`?)

wowgetoffyourcellphone awarded a token.Mar 4 2022, 8:04 AM

This revision was landed with ongoing or failed builds.Mar 4 2022, 3:09 PM

Closed by commit rP26542: Only add default music tracks if civ.json doesn't override them. (authored by Stan). · Explain Why

This revision was automatically updated to reflect the committed changes.

Stan added a commit: rP26542: Only add default music tracks if civ.json doesn't override them..

Owners added a subscriber: Restricted Owners Package.Mar 4 2022, 3:09 PM

Revision Contents
Changeset List

Path

Size

ps/

trunk/

binaries/

data/

mods/

public/

gui/

common/

music.js

59 lines

Diff 19816

View Options

ps/trunk/binaries/data/mods/public/gui/common/music.js

Don't add default tracks, if civ.json overrides them.ClosedPublicActions