Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix inconsistent auto-muting #159

Merged
merged 3 commits into from
Aug 23, 2022

Conversation

vincens2005
Copy link
Contributor

Automatic muting sometimes failed, as did unmuting.
This is a fix for both those issues

src/main.js Outdated
@@ -142,6 +142,7 @@ app.on("ready", async () => {
store.get(settings.trayIcon) && addTray(mainWindow, { icon }) && refreshTray();
store.get(settings.api) && expressModule.run(mainWindow);
store.get(settings.enableDiscord) && discordModule.initRPC();
console.log(store.get(settings.mutedArtists).find((artist) => artist === "Yes"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is there just a new random console.log here that would end up having no use for end-users nor people that use the app in general since devtools don't work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that was a mistake and I meant to remove it sorry. i'll fix that

src/preload.js Outdated
Comment on lines 94 to 103
const playing_title = window.document.querySelector(this.playing_title);

if (playing_title) {
const row = playing_title.closest(this.tracklist_row);
if (row) {
const album_name_cell = row.querySelector(this.album_name_cell);
if (album_name_cell) {
return album_name_cell.textContent;
}
}
Copy link
Contributor

@Mar0xy Mar0xy Aug 20, 2022

Choose a reason for hiding this comment

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

This seems like unnecessary overhead for a function that mostly already works fine the occasional glitch out when going on another album which cause the album name to change happens but it is so minor since when you switch to another album you either anyways will start playing it or immediately switch back to another playlist which will fix the album name with the next song anyways.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was occasionally causing the mute to fail, but I rearranged the other bit some so maybe i can revert that change

@vincens2005
Copy link
Contributor Author

OK fixed whoopsie

Copy link
Owner

@Mastermindzh Mastermindzh left a comment

Choose a reason for hiding this comment

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

Thanks @vincens2005,
Seems fine now! I'll check it out later today and make a merge along with some other work I still had open.

Thanks for reviewing it as well @Mar0xy!

@Mastermindzh
Copy link
Owner

image

Life had other plans yesterday (main sewage line backed up).
Will try and get to it tonight (I should have the time)

@vincens2005
Copy link
Contributor Author

damn gl

@Mastermindzh Mastermindzh changed the base branch from master to bugfix/4.1.1 August 23, 2022 19:19
@Mastermindzh Mastermindzh merged commit 0f20bfa into Mastermindzh:bugfix/4.1.1 Aug 23, 2022
Mastermindzh added a commit that referenced this pull request Aug 23, 2022
* - Fixed `cannot read property of undefined` error because of not passing mainWindow around.
- vincens2005, fixed inconsistent auto muting

* Fix inconsistent auto-muting (#159)

* fix muting sometimes not working

* fix inconsistent unmuting

* fix bad code in inconsistent muting fig

Co-authored-by: Cukmekerb <[email protected]>
@Mastermindzh
Copy link
Owner

@vincens2005 , thanks man.

I made it work tonight though, 4.1.1 is out!

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

Successfully merging this pull request may close these issues.

3 participants