search

wizzomafizzo / MiSTer_Favorites

Add and manage favorites in your MiSTer menu.
MIT License
66 stars 2 forks source link

added support for optionally using LLAPI cores instead of main cores #7

Closed kerobaros closed 1 year ago

kerobaros commented 1 year ago

Hello! I pointed a friend on Discord to your script, and they loved it, except they wanted to use the LLAPI cores that update_all had installed for them. A couple hours of back and forth later, and here is my work. Seems to work on their MiSTer, but I haven't tested it on my own, as I have no LLAPI hardware.

EDIT: I didn't see any kind of Options screen inside the script, so the choice to use the LLAPI cores is a series of boolean variables in the script. Don't know if it's worth doing anything more complicated than that, since I imagine this is a thing you'd touch once, before creating any favorites, and never touch again.

Thoughts?

wizzomafizzo commented 1 year ago

Thank you for taking the time to put this together. I only had a quick glance so far but it seems like a perfectly good solution to me. I'll pull it soon! Maybe it's also time for this script to get a config file

I am in progress writing a search script and a shared library associated with it which I'd like to eventually use for this script. So it's good timing you have brought it up. It would be useful to add support for alternate cores wouldn't it

Can you please ask your friend for me, as someone who uses the llapi cores, would they ever/often need to switch back to the original cores? I think it would always need to be configurable, but perhaps it's also safe to default to the llapi cores if they exist?

kerobaros commented 1 year ago

Said friend also believes it would be safe to default to LLAPI cores if they exist. Thought about how I would to that while I was at work tonight, and I'll try to knock something up real quick.

kerobaros commented 1 year ago

Okay, here's a first go at it.

I tried to get a little clever to minimize code reuse and shorten line lengths on lines 97-101, but I'm not super satisfied with it; I feel like that whole section could be replaced with some kind of iteration through a list, but this is a rough draft.

wizzomafizzo commented 1 year ago

Happy with this one now?

kerobaros commented 1 year ago

Sorry, no, not yet; while it worked on a test I set up on my laptop, it doesn't yet work on MiSTer; turns out sorgelig ships an older version of Python 3 that is missing some features in the glob library. I'm going to do some work on it tonight and will hopefully have something working by the morning, local time.

wizzomafizzo commented 1 year ago

No worries, just checking. No rush!

kerobaros commented 1 year ago

Okay. Copied the script at the tip of my branch to my MiSTer and did the following: -make some favorites with regular cores, all work fine -installed some LLAPI cores, and then made some more favorites; all MGLs had the correct, LLAPI core specified -disabled the LLAPI support in the script with the flag I added, and made some more favorites; all used the regular core instead

I am satisfied that this works. I hope it's not just Works On My System (tm) though.

I can see some more that can be done (maybe adding which core an MGL specifies in the Favorites Manager?) but I will hold off on anything else; I think maybe you and I should discuss what your future plans are, and how I may or may not be able to contribute to them. Great work so far, I'm definitely going to start using this script on my end.

wizzomafizzo commented 1 year ago

Awesome. Thank you! Let's give it a shot. Don't worry, nobody is gonna die if it breaks

Regarding my future plans. I've been working on a new repo here to eventually rewrite all my scripts published so far in Go. A few reasons: it's much faster than Python (browsing zips is super slow in this script), all my scripts share functions which would be better as a library, I can split the files up but publish a single binary (2000 line pythons files suck), I don't need to rely on what's shipped with with mister image (like for example finding out it ships with python 3.9 when you're using 3.10). Also just wanted an excuse to learn go for some work stuff. I've got a bit of a vision here of a central well maintained library to interact with and enhance mister

So yeah, it probably won't happen for a while since this script is quite stable already, but please don't invest too much time into this script unless you're ok with it being rewritten in the future. I am happy to chat more if you're interested in any of that, but I appreciate what you did already if that's as far as you want to go :)