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

feat: add padding for album name one have 1 character #6

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

aokblast
Copy link
Contributor

@aokblast aokblast commented Sep 21, 2023

Some albums' name only have 1 character. However, discord's status have to be at least two characters.
Therefore, we add 1 space padding for this case.

@jtpox
Copy link
Owner

jtpox commented Sep 21, 2023

image
There seems to be a problem when I play a certain file your version. On the left is the instance from the main branch, while on the right, your fork. Of course, the two aren't ran at the same time.

The file played was a DFF with no cover and almost no metadata.
image

Also to add, a default image does exist if you are using the default Discord Client ID, but not if you are using your own API keys.

You can add a default image by uploading one to the Discord Developer Portal.
Applications > Select App > Rich Presence > Art Assets
Upload a default image there and name it "roon_labs_logo".

@aokblast
Copy link
Contributor Author

For the first problem, I just want to fix the album with only a single word. So maybe I can change the function from < 2 to == 1 that will be compatible with your case.
For the second problem, I think we have to add some comment in the README to notify other suffer from this case instead of changing the source code?
Therefore, I will rebase and adjust this PR properly.

@aokblast aokblast changed the title feat: add default png used when the album has no image feat: add padding for album name one have 1 character Sep 21, 2023
@aokblast
Copy link
Contributor Author

Besides, In RPC, only type watching and playing are allowed as described in link.

@jtpox
Copy link
Owner

jtpox commented Sep 22, 2023

Besides, In RPC, only type watching and playing are allowed as described in link.

YES! That's what I've been trying to explain on the other issue haha.

Apologies for the slow reply. As for your new pull request, the issue persists. I have done some tests and have concluded that an empty string of just spaces will not be accepted by Discord RPC.

May I suggest changing the display value of state to something like this:

/**
 * WORKS:
 * undefined
 * 
 * NOT WORKING
 * ""
 * false
 */
state: data.now_playing.three_line.line3.substr(0, 128) || undefined,

We can choose to display something if line3 exists and keep it unset if it doesn't.

@aokblast
Copy link
Contributor Author

Oh, sorry. I understand your problem now.
What I want to solve is for the case that the album has only 1 word as the following picture shows.

CleanShot 2023-09-22 at 22 05 42

Because the state item in the discord should be at least two word, your change will still be failed on my case.

However, I don't have the album in your case. I want to ask the data.now_playing.three_line.line3 in the album in your case return undefined or empty string?

If we know the answer, we can further solve this issue.

@aokblast
Copy link
Contributor Author

Update: I make a little change and it still works on my case.
Can you help me test on your case?
Thanks!

Because Discord needs state string's length at least two, we add padding for the album's name is only one character
@jtpox
Copy link
Owner

jtpox commented Sep 22, 2023

Yup it works right now.

The file I played was displayed like this in data.now_playing
image

@aokblast
Copy link
Contributor Author

Ok, I think if both of us cases work, you can merge it if there is no problem anymore. XD

@jtpox jtpox merged commit 954e6c1 into jtpox:main Sep 22, 2023
@jtpox
Copy link
Owner

jtpox commented Sep 22, 2023

Ok, I think if both of us cases work, you can merge it if there is no problem anymore. XD

Merged, thanks for the pull request!

@aokblast aokblast deleted the feat/default_png branch September 22, 2023 15:21
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