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: newgrounds support #639

Closed
wants to merge 21 commits into from

Conversation

hyperdefined
Copy link
Contributor

@hyperdefined hyperdefined commented Jul 18, 2024

A WIP pull request for adding Newgrounds support, from #620.
Would love some help with this, never worked with cobalt source before or Javascript.

Links supported:

Here is what's missing:

  • Video downloads don't have the correct file name, extension is undefined.
  • Audio links work, but the filename is incorrect (downloads as newgrounds_id.undefined)
  • Audio files do not have metadata attached, not sure how to do this currently.

@hyperdefined hyperdefined marked this pull request as ready for review July 18, 2024 14:49
@ihatespawn
Copy link
Contributor

ihatespawn commented Jul 24, 2024

https://www.newgrounds.com/audio/listen/1343831 doesn't work

{
	"status": "error",
	"text": "newgrounds is supported, but something is wrong with your link. maybe you didn't copy it fully?"
}

@hyperdefined hyperdefined requested a review from ihatespawn July 25, 2024 01:14
@hyperdefined
Copy link
Contributor Author

everything seems to be good, if anyone wants to take a look

Copy link
Contributor

@ihatespawn ihatespawn left a comment

Choose a reason for hiding this comment

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

lgtm

@ihatespawn
Copy link
Contributor

Copy link
Contributor

@ihatespawn ihatespawn left a comment

Choose a reason for hiding this comment

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

deepsource fixes

src/modules/processing/services/newgrounds.js Outdated Show resolved Hide resolved
src/modules/processing/services/newgrounds.js Outdated Show resolved Hide resolved
src/modules/processing/services/newgrounds.js Outdated Show resolved Hide resolved
@hyperdefined
Copy link
Contributor Author

Was not aware about rebasing, I'll fix that for the future.

To fix the ugly messages, I can redo this PR if needed.

@ihatespawn
Copy link
Contributor

i don't think you need to redo the pr, i didn't see any prs be rejected because of that

@hyperdefined hyperdefined requested a review from dumbmoron July 27, 2024 21:33
@hyperdefined
Copy link
Contributor Author

ok that did not work, thanks github for the merge 👍

@hyperdefined hyperdefined marked this pull request as draft August 8, 2024 16:58
@wukko
Copy link
Member

wukko commented Aug 8, 2024

but why

@hyperdefined
Copy link
Contributor Author

going to redo this, the branch is out of date and I don't want all the ugly testing commits in it

@hyperdefined hyperdefined deleted the newgrounds-support branch August 8, 2024 17:15
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.

4 participants