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

Support playing ads to avoid Pandora account problems #686

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

GrantMoyer
Copy link

@GrantMoyer GrantMoyer commented Jul 9, 2019

Related issue: #415
This is a rebase of fbfc5f1 onto the current master. The rebase seemed pretty straightforward, only a few things needed to be manually merged, and it was pretty trivial to merge them, but I'm not familiar with the code base, so it might not be entirely correct. Currently, it compiles and runs fine, but doesn't seem to play any ads.

@PromyLOPh
Copy link
Owner

PromyLOPh commented Jul 10, 2019

Looks like ad “songs” are currently skipped. With this patch https://gist.github.com/PromyLOPh/1d16c520643114910c69eafed2859842 it works for me and audio ads are played as well.

Just needs a lot of polishing, because most of the UI can’t handle these “ad tracks” yet.

@GrantMoyer
Copy link
Author

GrantMoyer commented Jul 10, 2019

I get ads with the patch, too. I'll look into the rest of the UI problems when I get the chance. Could you give me a rough list of places I'll need to look to get the UI working correctly with ads?

I suppose we should also try to get an account to trigger the ads disguised as songs state, then see if this fixes that.

@PromyLOPh
Copy link
Owner

Essentially everything that accepts a PianoSong_t. The most obvious is BarUiListSongs, which is used in a lot of places and right now will just printf the plain format string (due to lack of information). I guess BarUiSelectSong needs to ignore ad songs and so does BarUiHistoryPrepend, because you can’t interact with them (i.e. rate them, move them, …). BarUiDispatch might be used to prevent interaction with currently playing ads (except for skipping?).

As for verifying this fix works: This was very difficult, if I remember correctly. Firstly, because it’s not clear how to trigger that particular server behavior. And I also got different feedback from different people – it worked for some, but did not for others. So it might be worth checking how the current Android client handles audio ads.

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

Successfully merging this pull request may close these issues.

None yet

2 participants