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

Videogames nolonger searchable #159

Open
3 tasks done
android272 opened this issue Aug 16, 2024 · 15 comments · May be fixed by #161
Open
3 tasks done

Videogames nolonger searchable #159

android272 opened this issue Aug 16, 2024 · 15 comments · May be fixed by #161
Labels
bug Something isn't working

Comments

@android272
Copy link

  • The Plugin is up to date - 0.7.2
  • Obsidian is up to date - V1.6.7

Describe the bug
When I activate the Media DB Video Game search, enter a title and press ok the modals go away and no search results are seen.

To Reproduce
Steps to reproduce the behavior:

  1. Open Command Palette and search for game to select Media DB: Create Media DB entry (Game)
  2. Search for a title: Fable
  3. Press enter or hit OK
  4. See no search results

Expected behavior
A clear and concise description of what you expected to happen.

I expected to see search results for various titles the API thinks I am looking for like when I search for books, music, or TV shows (all of which work as expected).

Screenshots
If applicable, add screenshots to help explain your problem.

Music:
image

Book:
image

TV Series:
image

Occurs on

  • Linux

Plugin version
0.7.2

@android272 android272 added the bug Something isn't working label Aug 16, 2024
@ltctceplrm
Copy link
Contributor

Strange, I have no issues adding games on 0.7.2. Could you add the console debug (Ctrl + Shift + i) to see if there's any errors?

@ltctceplrm
Copy link
Contributor

Ah I've found why, I tested this with a fresh install and it seems that if you don't have both an omdbAPI and a mobygamesAPI key then you can't search for games. This is strange though because I thought #147 was supposed to fix that

If this is the case for you then you can fix it by creating an omdbAPI and/or mobygamesAPI key, whichever one you're missing.

@mProjectsCode I know you're busy so please take your time but maybe this would be a good moment to re-enable to "selective API disabling" I added in #133? I'm currently using it on my fork without any issues, even with the latest changes (0.7.2).

If you want to I can create a new PR for it or if you prefer I can try to fix #147

@mProjectsCode
Copy link
Owner

I didn't really like how #133 handled the settings, so that is why I disabled it. Tbh I forgot that I wanted to fix that, maybe I can find some time soon.

#147 throws errors when the key is not present, which is why the search fails. There should be some sort of error message to the user, but the search should not fail if one API fails. I guess this is easily fixed through some better error handling.

@ltctceplrm
Copy link
Contributor

ltctceplrm commented Aug 21, 2024

Tbh I forgot that I wanted to fix that, maybe I can find some time soon.

That would be great yes. I remember you wanted to change/fix the settings but I'm not sure how to improve it myself

There should be some sort of error message to the user, but the search should not fail if one API fails. I guess this is easily fixed through some better error handling.

Currently the search does fail if one of them has no API key and the the errors only show up in the console, most users won't notice them.

If instead of throwing an error it just returns an empty array then the search still works for the other videogame APIs like steam which requires no api key and it'll just skip omdbAPI or mobygamesAPI if those don't have a key.

		if (!this.plugin.settings.MobyGamesKey) {
			console.error(Error(`MDB | API key for ${this.apiName} missing.`));
			return [];
		}

rather than

		if (!this.plugin.settings.MobyGamesKey) {
			throw Error(`MDB | API key for ${this.apiName} missing.`);
		}

The error still only shows up in the console but at least the search won't fail anymore. I made a PR for this fix

@ltctceplrm ltctceplrm linked a pull request Aug 21, 2024 that will close this issue
@android272
Copy link
Author

Sorry I had not looked at my email in a while. It looks like your right @ltctceplrm I am missing the MobyGamesAPI.

plugin:obsidian-media-db-plugin:47 Error: MDB | API key for MobyGamesAPI missing.
at Ni.searchByTitle (plugin:obsidian-media-db-plugin:48:21585)
at eval (plugin:obsidian-media-db-plugin:47:571)
at Array.map ()
at Di.query (plugin:obsidian-media-db-plugin:47:538)
at eval (plugin:obsidian-media-db-plugin:50:10247)
at Gi.openSearchModal (plugin:obsidian-media-db-plugin:50:3826)
at async zi.createEntryWithSearchModal (plugin:obsidian-media-db-plugin:50:10045)
eval @ plugin:obsidian-media-db-plugin:47
await in eval (async)
query @ plugin:obsidian-media-db-plugin:47
eval @ plugin:obsidian-media-db-plugin:50
openSearchModal @ plugin:obsidian-media-db-plugin:50
await in openSearchModal (async)
createEntryWithSearchModal @ plugin:obsidian-media-db-plugin:50
callback @ plugin:obsidian-media-db-plugin:50
BK @ app.js:1
t.onChooseItem @ app.js:1
t.onChooseSuggestion @ app.js:1
t.selectSuggestion @ app.js:1
e.useSelectedItem @ app.js:1
(anonymous) @ app.js:1
e.handleKey @ app.js:1
e.onKeyEvent @ app.js:1
plugin:obsidian-media-db-plugin:50 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'type')
at eval (plugin:obsidian-media-db-plugin:50:10342)
at Array.filter ()
at zi.createEntryWithSearchModal (plugin:obsidian-media-db-plugin:50:10319)

@android272
Copy link
Author

Adding the API key fixed the issue. I don't remember having to add the API Key before.

@ltctceplrm
Copy link
Contributor

Adding the API key fixed the issue. I don't remember having to add the API Key before.

This happened after Mobygames was added but there's a bug fix coming where if you're missing an API key then it'll just skip that API provider and fall back on the other ones.

@android272
Copy link
Author

android272 commented Aug 24, 2024

Could you add a warning saying that "Missing mobygames api key, falling back to [other] api"? I just thought that the plugin was broken all together.

@ltctceplrm
Copy link
Contributor

ltctceplrm commented Aug 25, 2024

It will say MediaDB | API key for ${this.apiName} missing. which hopefully will be clear enough for users

image

I just thought that the plugin was broken all together.

Yeah that was unintentional since it was supposed to give a clearer warning but the warning only showed up in the console

@android272
Copy link
Author

Yep that will work for me

@gusper
Copy link

gusper commented Sep 4, 2024

Mostly just FYI as mine was a slightly different case. I had both keys in the plugin's settings, but hadn't activated my OMDb API key and it was failing to authenticate. I went back to the mail I got from OMDb API, noticed the activate link, and it started working. I also thought the plugin was just failing but saw the auth error in the console and then I ran across this issue here. In addition to a warning about missing keys, it would be good to also let users know when there's an auth error. Thanks!

@albertoloscerritos
Copy link

Hi. One question, is there any way to bypass this issue. Since right now you need to pay in order get a mobygames api key
image

Or is there any way to access games from steam at least? At the moment I can't search for any games at all.
Thanks for your help :)

@ltctceplrm
Copy link
Contributor

@albertoloscerritos Since this is a breaking bug I made a temporary release here but please note that this is not a release by the creator of this plugin (@mProjectsCode). It should work fine though and I've tested it.

@mProjectsCode
Copy link
Owner

I should fix this. Since I am not active using this plugin anymore, I kind of loose track what needs to be done...

@albertoloscerritos
Copy link

Thanks for the help guys, I really appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants