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

Use Mod Search Database #852

Draft
wants to merge 26 commits into
base: main
Choose a base branch
from
Draft

Use Mod Search Database #852

wants to merge 26 commits into from

Conversation

otobot1
Copy link
Member

@otobot1 otobot1 commented Aug 24, 2024

Closes #606.

otobot1 added 20 commits August 10, 2024 14:26
-updates the local on-disk copy of the mod search database
-only works in dev
-no authentication
-untested
-required info is provided during the existing mod api routes
-the useDownloadUrl and useModImageUrls hooks use the mod search database info if available, otherwise they use the existing logic to fetch the info from GameBanana
something about pino.transport not being a function
couldn't find 'fs/promises'. it's a server-only module.
avoid confusion with nextjs app router
making progress. builds finish now, but the `mod.getAll` tRPC endpoint is taking WAY too long and is causing issues. potentially including the build issues.
only store Maps, and only store the parameters that we use.
- batch the mod prefetches
- store a highly trimmed version of the mod search database instead of the full file
contains nextjs bundle analysis
5bdf9bb didn't quite fix the webpack error apparently
-ensure that the `logs` and `cache` directories exist
-ensure outdated mod search database isn't used for builds
@otobot1 otobot1 linked an issue Aug 24, 2024 that may be closed by this pull request
@otobot1
Copy link
Member Author

otobot1 commented Aug 25, 2024

@ShouvikGhosh2048 Please review when you have a chance. #766 is higher-priority, but do this whenever.
Please check that both production and dev mode work.

…axios-1.7.5

Bump axios from 1.7.2 to 1.7.5

const cachedDownloadUrl = contextOrUndefined?.state[gamebananaModId];

const [downloadUrl, setDownloadUrl] = useState<string>(cachedDownloadUrl ?? "");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this state? We update the context whenever we update the downloadUrl, so couldn't we just use the cachedDownloadUrl?

Copy link
Member Author

@otobot1 otobot1 Aug 27, 2024

Choose a reason for hiding this comment

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

Yeah I think we can. I'll take a stab at it once I fix the carousel

};


export const useModImageUrls = (
{
gamebananaModId,
screenshotsFromModSearchDatabase,
}: useModImageUrlsProps,
): ModImageUrls => {
const contextOrUndefined = useContext(modImageUrlsContext);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same thing here - we update state and context together, so couldn't we use the context directly and remove the imageUrls state?

@ShouvikGhosh2048
Copy link
Collaborator

  • I'm getting a Failed to write the Mod Search Database to the file system. when I refresh the mods page (in dev).
  • I think the GAMEBANANA_MOD_IMAGES_BASE_URL should be "https://images.gamebanana.com/img/ss/mods/"?

@otobot1
Copy link
Member Author

otobot1 commented Aug 26, 2024

  • I'm getting a Failed to write the Mod Search Database to the file system. when I refresh the mods page (in dev).
  • I think the GAMEBANANA_MOD_IMAGES_BASE_URL should be "https://images.gamebanana.com/img/ss/mods/"?
  • I bet that warning is because you don't have the local copy of the mod search database. If it's the warning I think it is, it will just download the latest copy and carry on. I just updated the warning so it's more clear that it's ok. Please let me know if it was a different error.
  • That was the original value for the images base url actually, but I had to change it to be more general as a couple of mods in the mod search database have their images elsewhere at images.gamebanana.com. We consume the whole url from the mod search database and just use this constant as a sanity check for the beginning of the url, so it's not a big deal.

@otobot1
Copy link
Member Author

otobot1 commented Aug 27, 2024

Looks like I broke the carousel at some point on this branch (the arrows don't do anything now). I'll fix that and then take a stab at those remaining comments.

@otobot1 otobot1 marked this pull request as draft September 16, 2024 22:24
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.

Use Maddie's Random Stuff
2 participants