Skip to content
This repository was archived by the owner on Sep 18, 2024. It is now read-only.

#168 Added restart track setting#169

Open
DawidIzydor wants to merge 3 commits intodeath-save:masterfrom
DawidIzydor:master
Open

#168 Added restart track setting#169
DawidIzydor wants to merge 3 commits intodeath-save:masterfrom
DawidIzydor:master

Conversation

@DawidIzydor
Copy link

Added restart track setting #168

Copy link
Member

@eclarke12 eclarke12 left a comment

Choose a reason for hiding this comment

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

I will approve this PR once the noted changes are made, thanks!


"SETTINGS.HypeTrackEnableN": "Enable Hype Track",
"SETTINGS.HypeTrackEnableH": "Enable the ability to set a Track for an Actor",
"SETTINGS.restartHypeTracksN": "Restart Hype Track",
Copy link
Member

Choose a reason for hiding this comment

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

Please captialise the first letter of each translation key-part.
This should be "SETTINGS.RestartHypeTracksN"

// Stop the playlist if it is playing
if (this.playlist.playing) {
await this.playlist.stopAll();
await Playback.pauseAllTracks();
Copy link
Member

Choose a reason for hiding this comment

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

This call doesn't pass the playlist to pauseAllTracks, is that correct?

@@ -1,5 +1,9 @@
import * as MAESTRO from "./config.js";

class PlaybackSettings{
Copy link
Member

Choose a reason for hiding this comment

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

Instead of a static inner class, I'd prefer adding a property to the game.maestro object. Eg. game.maestro.pausedTracks or similar.



const restartHypeTracks = game.settings.get(MAESTRO.MODULE_NAME, MAESTRO.SETTINGS_KEYS.HypeTrack.restartHypeTracks);
var resumeTime = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Please use let instead of var for consistency sake

const restartHypeTracks = game.settings.get(MAESTRO.MODULE_NAME, MAESTRO.SETTINGS_KEYS.HypeTrack.restartHypeTracks);
if(restartHypeTracks && playlist.data && playlist.data.sounds)
{
var sounds = playlist.data.sounds;
Copy link
Member

Choose a reason for hiding this comment

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

Please use let instead of var for consistency sake

config: true,
onChange: async s => {}
}),
})
Copy link
Member

Choose a reason for hiding this comment

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

Something weird going on with the indentation here. Please ensure it follows the same format as other settings.

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.

2 participants