ripple/hanayo
5
0
Fork 0
Code Issues 2 Pull requests Projects Releases Packages Wiki Activity Actions

Add support for /s/ and /beatmapsets/ urls #79

Merged
Lithium merged 9 commits from feature/beatmapsets-url into master 2023-05-09 16:08:52 +00:00
Conversation 39 Commits 9 Files changed 9 +35 -12
Lithium commented 2023-05-07 09:39:01 +00:00 (Migrated from zxq.co)
Copy link

This was needed for the -devserver url redirects to work again. When -devserver was introduced, peppy also changed the url of beatmap links, both for chat (/np) and the "open website" button in the song selection.

Tested to be working on a local instance, doublecheck appreciated. CI Seems to be failing for whatever reason

This was needed for the -devserver url redirects to work again. When -devserver was introduced, peppy also changed the url of beatmap links, both for chat (/np) and the "open website" button in the song selection. Tested to be working on a local instance, doublecheck appreciated. CI Seems to be failing for whatever reason
Lithium commented 2023-05-07 09:39:02 +00:00 (Migrated from zxq.co)
Copy link

requested review from @Nyo

requested review from @Nyo
Lithium commented 2023-05-07 09:39:02 +00:00 (Migrated from zxq.co)
Copy link

assigned to @Lithium

assigned to @Lithium
Lithium commented 2023-05-07 09:41:03 +00:00 (Migrated from zxq.co)
Copy link

added 1 commit

Compare with previous version

added 1 commit <ul><li>7ba12881 - remove unneeded log</li></ul> [Compare with previous version](/ripple/hanayo/-/merge_requests/3/diffs?diff_id=49&start_sha=b1cf22ee3f7e58989e4f3e15039a7a748780cf2d)
Lithium commented 2023-05-07 09:42:00 +00:00 (Migrated from zxq.co)
Copy link

added 1 commit

Compare with previous version

added 1 commit <ul><li>ee1645e5 - restore agpl warning</li></ul> [Compare with previous version](/ripple/hanayo/-/merge_requests/3/diffs?diff_id=51&start_sha=7ba12881e966e1a2ada95035bb7714cc0927b4a1)
Lithium commented 2023-05-07 11:13:21 +00:00 (Migrated from zxq.co)
Copy link

update: not 100% working as of now, evening out small bugs

update: not 100% working as of now, evening out small bugs
Lithium commented 2023-05-07 11:30:45 +00:00 (Migrated from zxq.co)
Copy link

added 1 commit

  • 7cd97102 - Re-route ranking requests to make space for /beatmaps

Compare with previous version

added 1 commit <ul><li>7cd97102 - Re-route ranking requests to make space for /beatmaps</li></ul> [Compare with previous version](/ripple/hanayo/-/merge_requests/3/diffs?diff_id=53&start_sha=ee1645e5969cfea10f34a4f8323605cb1ec76e44)
Lithium commented 2023-05-07 11:31:01 +00:00 (Migrated from zxq.co)
Copy link

Should work fully now

Should work fully now
Nyo commented 2023-05-08 12:28:30 +00:00 (Migrated from zxq.co)
Copy link

Those are the redirect from old links, they should be pretty much unused but the numbers should be kept as-is or it will break all the legacy redirects.

	37: "/beatmaps",
	38: "/register/verify",
	39: "/register/welcome",
	40: "/settings/discord",
	41: "/register", // elmo
Those are the redirect from old links, they should be pretty much unused but the numbers should be kept as-is or it will break all the legacy redirects. ```suggestion:-3+0 37: "/beatmaps", 38: "/register/verify", 39: "/register/welcome", 40: "/settings/discord", 41: "/register", // elmo ```
Nyo commented 2023-05-08 12:28:30 +00:00 (Migrated from zxq.co)
Copy link

I think this should be /rank_request instead of rank_request for simplePage.Handler

var simplePages = [...]simplePage{{"/", "homepage.html", "Home Page", "homepage2.jpg", 0}, {"/login", "login.html", "Log in", "login2.jpg", 0}, {"/settings/avatar", "settings/avatar.html", "Change avatar", "settings2.jpg", 2}, {"/dev/tokens", "dev/tokens.html", "Your API tokens", "dev.jpg", 2}, {"/rank_request", "rank_request.html", "Request beatmap ranking", "request_beatmap_ranking.jpg", 2}, {"/donate", "support.html", "Support Ripple", "donate2.png", 0}, {"/doc", "doc.html", "Documentation", "documentation.jpg", 0}, {"/doc/:id", "doc_content.html", "View document", "documentation.jpg", 0}, {"/help", "help.html", "Contact support", "help.jpg", 0}, {"/leaderboard", "leaderboard.html", "Leaderboard", "leaderboard2.jpg", 0}, {"/friends", "friends.html", "Friends", "", 2}, {"/changelog", "changelog.html", "Changelog", "changelog.jpg", 0}, {"/team", "team.html", "Team", "", 0}, {"/pwreset", "pwreset.html", "Reset password", "", 0}, {"/about", "about.html", "About", "", 0}}
I think this should be `/rank_request` instead of `rank_request` for simplePage.Handler ```suggestion:-0+0 var simplePages = [...]simplePage{{"/", "homepage.html", "Home Page", "homepage2.jpg", 0}, {"/login", "login.html", "Log in", "login2.jpg", 0}, {"/settings/avatar", "settings/avatar.html", "Change avatar", "settings2.jpg", 2}, {"/dev/tokens", "dev/tokens.html", "Your API tokens", "dev.jpg", 2}, {"/rank_request", "rank_request.html", "Request beatmap ranking", "request_beatmap_ranking.jpg", 2}, {"/donate", "support.html", "Support Ripple", "donate2.png", 0}, {"/doc", "doc.html", "Documentation", "documentation.jpg", 0}, {"/doc/:id", "doc_content.html", "View document", "documentation.jpg", 0}, {"/help", "help.html", "Contact support", "help.jpg", 0}, {"/leaderboard", "leaderboard.html", "Leaderboard", "leaderboard2.jpg", 0}, {"/friends", "friends.html", "Friends", "", 2}, {"/changelog", "changelog.html", "Changelog", "changelog.jpg", 0}, {"/team", "team.html", "Team", "", 0}, {"/pwreset", "pwreset.html", "Reset password", "", 0}, {"/about", "about.html", "About", "", 0}} ```
Nyo commented 2023-05-08 12:28:30 +00:00 (Migrated from zxq.co)
Copy link

Can this be simplified to something like:

sid, _ := strings.CutPrefix(c.Param("sid"), "#")
bid := c.Param("bid")

?

https://go.dev/play/p/_pXb_fswzBq

Can this be simplified to something like: ```go sid, _ := strings.CutPrefix(c.Param("sid"), "#") bid := c.Param("bid") ``` ? https://go.dev/play/p/_pXb_fswzBq
Nyo commented 2023-05-08 12:28:30 +00:00 (Migrated from zxq.co)
Copy link

Not sure and I can't remember what library is being used for the router, but shouldn't this be:

	r.GET("/beatmapsets/:sid/:bid", beatmapInfo)

instead? 🤔

Not sure and I can't remember what library is being used for the router, but shouldn't this be: ```go r.GET("/beatmapsets/:sid/:bid", beatmapInfo) ``` instead? :thinking:
Nyo commented 2023-05-08 12:28:30 +00:00 (Migrated from zxq.co)
Copy link

Great work! I've left a couple of comments, please test them before applying suggestions.

CI failure is normal as PRs do not have access to secrets and there are no tests either so it's not very helpful anyways.

Great work! I've left a couple of comments, please test them before applying suggestions. CI failure is normal as PRs do not have access to secrets and there are no tests either so it's not very helpful anyways.
Lithium commented 2023-05-08 13:26:01 +00:00 (Migrated from zxq.co)
Copy link

Gotcha

Gotcha
Lithium commented 2023-05-08 13:26:01 +00:00 (Migrated from zxq.co)
Copy link

ur right. Me Pepega

ur right. Me Pepega
Lithium commented 2023-05-08 13:26:01 +00:00 (Migrated from zxq.co)
Copy link

Seems to work, but has been added in a newer version of go (we were using 1.14, added in 1.20). Pulling the version wouldnt be too bad though, at least i'd assume so.

Seems to work, but has been added in a newer version of go (we were using 1.14, added in 1.20). Pulling the version wouldnt be too bad though, at least i'd assume so.
Lithium commented 2023-05-08 13:26:01 +00:00 (Migrated from zxq.co)
Copy link

its the default Gin router, and given that the /:bid part in the osu url is optional (e.g., some ingame buttons dont generate the link with the bid) /*bid is the syntax for an optional param named bid

its the default Gin router, and given that the `/:bid` part in the osu url is optional (e.g., some ingame buttons dont generate the link with the bid) `/*bid` is the syntax for an optional param named bid
Lithium commented 2023-05-08 13:32:39 +00:00 (Migrated from zxq.co)
Copy link

changed this line in version 5 of the diff

changed this line in [version 5 of the diff](/ripple/hanayo/-/merge_requests/3/diffs?diff_id=55&start_sha=7cd97102ca7a0eaefa085983a97d48e7e223ad72#dbdfe88648f2633396c9ba69b8bad523c73bdc06_55_52)
Lithium commented 2023-05-08 13:32:39 +00:00 (Migrated from zxq.co)
Copy link

changed this line in version 5 of the diff

changed this line in [version 5 of the diff](/ripple/hanayo/-/merge_requests/3/diffs?diff_id=55&start_sha=7cd97102ca7a0eaefa085983a97d48e7e223ad72#5700f0c9f73a2cc692b8a6d41937f04e54198da6_23_23)
Lithium commented 2023-05-08 13:32:40 +00:00 (Migrated from zxq.co)
Copy link

changed this line in version 5 of the diff

changed this line in [version 5 of the diff](/ripple/hanayo/-/merge_requests/3/diffs?diff_id=55&start_sha=7cd97102ca7a0eaefa085983a97d48e7e223ad72#05aa067a704b4a61de6942121dcdeed855dcf4fd_58_32)
Lithium commented 2023-05-08 13:32:40 +00:00 (Migrated from zxq.co)
Copy link

added 1 commit

Compare with previous version

added 1 commit <ul><li>d544e45d - Apply review fixes</li></ul> [Compare with previous version](/ripple/hanayo/-/merge_requests/3/diffs?diff_id=55&start_sha=7cd97102ca7a0eaefa085983a97d48e7e223ad72)
Nyo commented 2023-05-08 13:34:09 +00:00 (Migrated from zxq.co)
Copy link

My bad I think this should be

	parts := strings.SplitN(c.Param("sid"), "#", 2)
My bad I think this should be ```suggestion:-0+0 parts := strings.SplitN(c.Param("sid"), "#", 2) ```
Nyo commented 2023-05-08 13:34:09 +00:00 (Migrated from zxq.co)
Copy link

Thank you just one small think left I think. I will test it out later before merging.

Thank you just one small think left I think. I will test it out later before merging.
Nyo commented 2023-05-08 13:34:38 +00:00 (Migrated from zxq.co)
Copy link

https://go.dev/play/p/Zpuj6ktngLl

With 1 it would just return the whole string for me..

https://go.dev/play/p/Zpuj6ktngLl With `1` it would just return the whole string for me..
Lithium commented 2023-05-08 13:51:18 +00:00 (Migrated from zxq.co)
Copy link

changed this line in version 6 of the diff

changed this line in [version 6 of the diff](/ripple/hanayo/-/merge_requests/3/diffs?diff_id=57&start_sha=d544e45db81304f53c071e1eaa3ed7ae707b9824#0607f785dfa3c3861b3239f6723eb276d8056461_297_297)
Lithium commented 2023-05-08 13:51:18 +00:00 (Migrated from zxq.co)
Copy link

changed this line in version 6 of the diff

changed this line in [version 6 of the diff](/ripple/hanayo/-/merge_requests/3/diffs?diff_id=57&start_sha=d544e45db81304f53c071e1eaa3ed7ae707b9824#05aa067a704b4a61de6942121dcdeed855dcf4fd_29_28)
Lithium commented 2023-05-08 13:51:18 +00:00 (Migrated from zxq.co)
Copy link

added 2 commits

  • abd5d937 - Simplify bid and sid from context params
  • 4effc0d6 - remove unneeded route

Compare with previous version

added 2 commits <ul><li>abd5d937 - Simplify bid and sid from context params</li><li>4effc0d6 - remove unneeded route</li></ul> [Compare with previous version](/ripple/hanayo/-/merge_requests/3/diffs?diff_id=57&start_sha=d544e45db81304f53c071e1eaa3ed7ae707b9824)
Lithium commented 2023-05-08 13:51:19 +00:00 (Migrated from zxq.co)
Copy link

so... turns out the # functions as the windowlocaiton-hash in the router and everything behind it gets cut off anyway.

so... turns out the # functions as the windowlocaiton-hash in the router and everything behind it gets cut off anyway.
Lithium commented 2023-05-08 13:51:44 +00:00 (Migrated from zxq.co)
Copy link

Ready for your testing 👍

Ready for your testing :thumbsup:
Lithium commented 2023-05-08 13:52:01 +00:00 (Migrated from zxq.co)
Copy link

resolved all threads

resolved all threads
Nyo commented 2023-05-09 15:54:26 +00:00 (Migrated from zxq.co)
Copy link
r.GET("/beatmapsets/:sid/*bid", beatmapInfo)

is gone now? Is this wanted?

```go r.GET("/beatmapsets/:sid/*bid", beatmapInfo) ``` is gone now? Is this wanted?
Lithium commented 2023-05-09 15:57:37 +00:00 (Migrated from zxq.co)
Copy link

wanted since this was the catch-attempt for "beatmapsets/123#osu/id", where #osu/id is interpreted as the location-hash and gets entirely ignored by gin, see my last discord rant.

wanted since this was the catch-attempt for "beatmapsets/123#osu/id", where #osu/id is interpreted as the location-hash and gets entirely ignored by gin, see my last discord rant.
Lithium commented 2023-05-09 15:57:37 +00:00 (Migrated from zxq.co)
Copy link

resolved all threads

resolved all threads
Nyo commented 2023-05-09 16:03:33 +00:00 (Migrated from zxq.co)
Copy link

Oh the WHOLE thing gets interpreted as location hash?? :face_palm: okay then 😭

Oh the WHOLE thing gets interpreted as location hash?? :face_palm: okay then :sob:
Nyo commented 2023-05-09 16:03:35 +00:00 (Migrated from zxq.co)
Copy link

resolved all threads

resolved all threads
Nyo commented 2023-05-09 16:07:57 +00:00 (Migrated from zxq.co)
Copy link

added 2 commits

  • 0ca20a6c - Beatmaps: Fix panic when unknown beatmap set is used
  • c7f2b59e - Update gulp bundle

Compare with previous version

added 2 commits <ul><li>0ca20a6c - Beatmaps: Fix panic when unknown beatmap set is used</li><li>c7f2b59e - Update gulp bundle</li></ul> [Compare with previous version](/ripple/hanayo/-/merge_requests/3/diffs?diff_id=59&start_sha=4effc0d6f67c391e62804c84e81f24d9a062a24c)
Nyo commented 2023-05-09 16:08:42 +00:00 (Migrated from zxq.co)
Copy link

Fixed a possible index out of bound and LGTM. Thanks for this 🙏

Fixed a possible index out of bound and LGTM. Thanks for this :pray:
Nyo commented 2023-05-09 16:08:45 +00:00 (Migrated from zxq.co)
Copy link

approved this merge request

approved this merge request
Nyo commented 2023-05-09 16:08:52 +00:00 (Migrated from zxq.co)
Copy link

mentioned in commit 0210d24577

mentioned in commit 0210d2457792c085f5295f75ec0625c673adc320
Nyo (Migrated from zxq.co) merged commit 0210d24577 into master 2023-05-09 16:08:52 +00:00
Nyo (Migrated from zxq.co) approved these changes 2026-07-03 19:40:55 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Nyo
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ripple/hanayo!79
No description provided.