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

Add searching URL support #1283

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

FireController1847
Copy link

Replacing my old PR #806 after the change to a monorepo. See that PR for more details. Here's the description form it:


Currently, if you want to install an unlisted project to the launcher, you need to manually download the associated file and perform a manual installation. The major issue here is that you can't enable syncing (for modpacks), nor can you update it. This is due to the .mrpack format appearing to be completely independent from the Modrinth site; it doesn't store a ProjectID or a VersionID. This PR aims to resolve these issues.

The functionality I'm proposing is similar to what you'd experience on the Technic Launcher. There, you paste the URL into their search field, and the pack appears as the only search result. As such, this implementation should feel like a natural transition for those familiar with that methodology. Additionally, it addresses any complications that might arise from other solutions and would require the least amount of maintenance effort moving forward.

The only hiccup I ran into was a mismatch between the data structures of a Search Result Model and a Project Model. In the future, it might be useful to consider augmenting the search API to allow for non-indexed results when a specific slug is given, but that's a topic better suited for discussion outside of this PR.

Here's a breakdown of the current workflow:

  • Check if the search term is a URL. If it's not, skip the section entirely.
  • If it is a URL, but it does not match our required conditions (it must be on the modrinth.com domain and use the appropriate project type), then return zero results.
  • Parse the project slug out of the URL and make a request to the API endpoint /project/[slug]
    • To fix a missing 'author' field, make a request to /project/[slug]/members
  • Check if the results exist; if not, return zero results.
  • Check if the returned project matches the project type; if not, return zero results.
    • Prevents an edge case where the project type and url may be the same, but the project is not that type. For example, /mod/the-fv-pack on the mods tab would work, since it matches the second check (project type tab = mod, url = mod, so it requests /project/the-fv-pack). This check prevents that.
  • Convert the Project Model to a Search Result Model.
  • Return the results with the project as the only hit.

I recognize that the code might not be optimal, but it is functionally effective. I'm open to any constructive suggestions and willing to make changes as necessary. A video is attached below to demonstrate the new functionality, and a video showing the issue with the old functionality.

9utDqwCoLE.mp4
5mke59xYyq.mp4

Comment on lines +158 to +159
potentialUrl.hostname.indexOf('modrinth.com') != -1 &&
potentialUrl.href.indexOf(`/${projectType.value}/`) != -1
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
potentialUrl.hostname.indexOf('modrinth.com') != -1 &&
potentialUrl.href.indexOf(`/${projectType.value}/`) != -1
potentialUrl.hostname.includes('modrinth.com') &&
potentialUrl.href.includes(`/${projectType.value}/`)

Comment on lines +182 to +210
{
slug: rawResults.slug,
title: rawResults.title,
description: rawResults.description,
categories: rawResults.categories.concat(rawResults.additional_categories),
client_side: rawResults.client_side,
server_side: rawResults.server_side,
project_type: rawResults.project_type,
downloads: rawResults.downloads,
icon_url: rawResults.icon_url,
color: rawResults.color,
thread_id: rawResults.thread_id,
monetization_status: rawResults.monetization_status,
project_id: rawResults.id,
author: memberRawResults
.filter((mem) => mem.role === 'Owner')
.map((mem) => mem.user.username)[0],
display_categories: rawResults.categories,
versions: rawResults.game_versions,
follows: rawResults.followers,
date_created: new Date().toISOString(),
date_modified: rawResults.updated,
latest_version: rawResults.game_versions[rawResults.game_versions.length - 1],
license: rawResults.license.id,
gallery: rawResults.gallery.map((img) => img.url),
featured_gallery: rawResults.gallery
.filter((img) => img.featured === true)
.map((img) => img.url)[0],
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using update syntax

const modpacksIndex = potentialUrlSlashes.indexOf(projectType.value)
const packSlug = potentialUrlSlashes[modpacksIndex + 1]
if (packSlug) {
let url = 'project'
Copy link
Contributor

Choose a reason for hiding this comment

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

Make it const

@Geometrically
Copy link
Member

I don't like the way this is implemented. It just feels unintuitive and a little jank. Users right now can already install any unlisted pack or mod by opening this URL in their browser:

modrinth://mod/aged (this will install the aged modpack).

I think a better way would be a "From URL" option in the instance creation modal. Then, the user could input any URL and it would automagically get all the details (whether it was a CDN link, modrinth project link, modrinth version link, etc).

Since this will probably touch some part of the backend I recommend basing your PR off my launcher-db-overhual branch. It will be merged soon and that way you can ensure you don't have any duplicate work.

Feel free to ping me on discord if you have any questions!

Copy link
Member

@Geometrically Geometrically left a comment

Choose a reason for hiding this comment

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

See above

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

Successfully merging this pull request may close these issues.

3 participants