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

YouLikeHits Youtube Subscriber is functional #9

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

YouLikeHits Youtube Subscriber is functional #9

wants to merge 9 commits into from

Conversation

ashp0
Copy link

@ashp0 ashp0 commented Feb 16, 2021

Some errors but works perfectly

Copy link
Owner

@gekkedev gekkedev left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I've marked a few places that require changes. Also, I noticed it is possible to gain subscribe points without actually subscribing. Any idea how that works? AFAIK YLH has no access to your YT account.

setInterval(() => {
if (J("*:contains('503 Service Unavailable')").length) {
console.log("Server Error! reloading...");
location.reload();
location.reload(true);
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think we have to explicitly pass a parameter here.

}
});
}
}
};
Copy link
Owner

@gekkedev gekkedev Mar 7, 2021

Choose a reason for hiding this comment

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

Please don't add extra semicolons, especially if your main change is for a different line.


alertOnce = (message, identifier) => {
localIdentifier = (identifier != undefined) ? identifier : message;
if (shownWarnings.indexOf(localIdentifier) == -1) {
shownWarnings.push(localIdentifier);
alert(message)
Copy link
Owner

Choose a reason for hiding this comment

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

Why are you removing this line? If you don't want an alert to show up, either

  1. do not use alertOnce OR
  2. remove this function

but I suggest keeping it because it's used to show the user some extra information (once!) he might not have known

}
} else {

console.log(document.location.pathname);
Copy link
Owner

Choose a reason for hiding this comment

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

please remove all logging that you used to debug while implementing. We should reduce all logging to a minimum.

if (J("*:contains('Something is wrong with the account you\'re trying to subscribe to.')").length) location.reload();
if (J("*:contains('You took longer than 90 seconds to finish Subscribing to the account and confirming it.')").length) location.reload();

// Check for alerts/errors without JQuery
Copy link
Owner

Choose a reason for hiding this comment

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

What kind of error are you checking for here? I don't understand this part, please remove it for now.

if (J("*:contains('We could not verify the action. Please make sure you are Subscribed and try waiting at least 10 seconds before closing the PopUp.')").length) location.reload();
if (J("*:contains('This YouTube account no longer exists.')").length) location.reload();
if (J("*:contains('Something is wrong with the account you\'re trying to subscribe to.')").length) location.reload();
if (J("*:contains('You took longer than 90 seconds to finish Subscribing to the account and confirming it.')").length) location.reload();
Copy link
Owner

Choose a reason for hiding this comment

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

Always good to check for errors, but I have 2 questions:

  1. Is it always right to call location.reload()? Sometimes it sounds like too much. A normal user might just click on "[Skip this Account]". Maybe YLH will discover that many page reloads to flag us 😜
  2. Could you chain the conditionals into one statement so that we only have one call to location.reload()?

solveCaptcha(captcha[0], J("input[name='answer']"), "ylh_yt_traffic_captchasolving", () => J("input[value='Submit']").first().click());
}
break;
}
Copy link
Owner

Choose a reason for hiding this comment

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

There's a lot of indentation going on in these lines, but that makes it hard to see the actual changes. Please undo the indentation (we can address this issue separately).

@@ -139,8 +271,7 @@
console.log("Visiting a new page...");
buttons[0].onclick();
});
} else {
}
} else {}
Copy link
Owner

Choose a reason for hiding this comment

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

A good idea to shorten this part. Could you remove the whole else part? That would be even better.

@@ -157,6 +288,7 @@
GM.setValue('ylh_traffic_tab_open', false).then(() => { //free the way for a new tab
/*window.close(); //might not always work in FF
setTimeout (window.close, 1000);*/
setTimeout(window.close, 1000);
Copy link
Owner

Choose a reason for hiding this comment

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

This interferes with the website viewing part. Is it an accident? The new tabs are supposed to be self-closing.

// @version 0.1
// @description Click the subscribe button
// @author Developers
// @match https://www.youtube.com/channel/*
Copy link
Owner

Choose a reason for hiding this comment

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

  1. Could you add this into the main script? It seems more logical to me to support both sites within the same script (yes, the URL checking part requires some changes because it is currently only focused on detecting PHP URLs on YHL's server).
  2. It does the job; nothing against that. However, that will make youtube impossible to use because whenever you browse a channel, it will subscribe and close the window 😂 Maybe we can instead make sure that we really want to subscribe. Look at how the website viewing section works:
    GM.setValue('ylh_traffic_tab_open', false).then(() => { //free the way for a new tab

    It will share a boolean across both windows/tabs to communicate. You could copy the boolean part and instead use it to check if automatic subscribing should be active.

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.

2 participants