Junk-Store: Merge PR's and minor updates #782
Conversation
This mergest PR's from Beebles, GloriousEggroll and Ugzuzg.
Beebles: changed scripts to work on nixos
GloriousEggroll: Added changes to make Umu fixes work correctly.
Ugzuzg: Changed sorting to be case insensitive on the game grid screen.
Fix for Garden Story where ${} was in the name the $ needed escaping.
Added args that needs to be passed to games that was over looked in the initial builds (very few if any games might be affected by this).
|
Please properly fill out the template and comment when done.
|
|
Clicking Install Dependencies button made my deck hang on this Is this supposed to take more than 10 minutes? Logs: Nothing on FE logs. |
|
That's a race condition that I've been chasing for months now. I think you stumbled on a thread (pun intended) that I might be able to pull on here! |
|
@GedasFX once this is deployed to the testing store could you verify that it's now actually working? |
|
Yeah will do but will have to wait for tomorrow ^^ you can probably see the time from the steam deck 😄 |
|
All good, I have a deck that's untouched by any dev tools, so I'll check it on that too, but race conditions are VERY hard to test reliably. |
|
I've tested on my "clean" deck. It appears to work, but it's a race condition so take that for what it's worth. |
ebenbruyns
had a problem deploying
to
testing_env
GitHub Actions
Failure
Plugin Testing ReportInstalled Plugins
Specifications
IssuesHas the following major blocking issue(s): [3] After logging in to Epic Games, no games show up. Took me a while to figure out that I need to click "refresh games" in a settings menu. According to this video this feature used to work and broke at some point. Has the following minor non-blocking issue(s): [4] I tried launching a game "Cat Quest" to try out if integration works, and, it opens, but it requested OAuth from Epic Games, which, while worked, it didn't end up closing. I needed to manually close the window afterwards. Unsure if anything can be done about this. [5] README shows 2 tutorial videos, but it is actually, the same video. You should clean that up :D SummaryAfter downloading the updated plugin and retrying the install dependencies, I was greeted with issue [1], I am not sure if it is worth putting it to a "major" issue but it is for sure regression failure. I was able to at least continue. Trying to log in with Epic had me stumped a little bit with issue [2], but it was my skill issue for the most part. That being said, I found that if you use OAuth to log in to Epic Games, it will not work, and I couldn't find that documented as a known issue. After login I see the second regression failure, of [3], where I can see games should auto load, but they haven't. Was able to bypass by clicking the first settings button of the two and selecting reload. Installation and launching works okay, launching from Steam works okay too. If epic games requests OAuth, the window does not close, however. Testing of documented changes:
Testing Limitations: |
|
@GedasFX I need some more information around #3 (also I do not believe this is a blocking issue as the user can still continue to use the application). The auto refresh of games is actually rather prone to refresh loops. There's code specifically checking if you have text in the search box or if you have the installed flag toggled (pushing x). Could you verify that this is actually a regression vs the deployed version on the stable store? I have a vague recollection that I might actually have disabled this due to refresh loops when a users logs in and they have no games in their account. Due to the extensibility of this plugin it comes with a unique set of challenges because everything is async and stateless. |
|
@ebenbruyns Checked on 1.1.8, seems to work there okay. I feel like this is an issue as, as a new user, it for sure confused me why I didn't have any games. |
|
I don't disagree that it's an issue, but it's not a show stopper. Did you clear the databases for the 1.1.8 test? If so this is curious because I haven't gone anywhere near that code. I wonder if there's something in the decky frontend lib that's shifted... I'll have a closer look at this one. Thanks for spotting it. |
|
yeah i did the same test for both (on the test process "install plugin" i install either 118 or 119) |
|
So it looks like something has shifted in decky/ui |
|
looking into #1, I suspect this has to do with the child process not flushing it's buffers and it's also using \r instea of \n for stuff. it could also be related to python having blocking reads on streams by default and it's a royal pain to have nonblocking streams work in python. This is not really something users engage with a lot so it's not worth trying to solve the none blocking stream read for this one thing. It's a tough one because it's tempting to just skill std err or std out, but that's a half solution either way you look at it. |
|
so 2 and 4 I can't do anything about, that's out of my control. Oauth is a know issue in game mode, you can how ever do this in desktop mode if you put steam in big picture mode. 5 is a typo, out of scope, but thanks for the heads up, it's sorted now! I've made some updates, can you verify that they align with your testing now? |
ebenbruyns
had a problem deploying
to
testing_env
GitHub Actions
Failure
|
Currently waiting for new build, but I went to check the code you changed, and I only see README was updated? Was 1 or 3 touched on? As for non-blocking and blocking streams, I do not know what your implementation is, but I engineered something cool before, which might come in useful: just create a fire and forget async task and have its completion be an event, and just wrap it all in a promise. This way it does not block python nor JS render threads. First, create a task and run it Emit completion: And then wrap all together in FE: This basically sets up listener, then fires the start job, which only completes after event is done. |
|
2 and 4 are fine, they are minor issues anyway, just wanted to verify if they are known issues. I remember having exact same issues with google drive. |
|
And another unrelated issue, but it appears that by sheer coincidence, this plugin broke testing of another one: #785 (comment) This might come as a new issue on github eventually, as if that plugin loaded before this one, then functionality of this plugin would have been affected instead. Right now I am not too sure what to do about this, personally will ignore it's existence, but someone from loader team should come in with support. |
it wont be a problem on this side as I state here #785 (comment) |
|
v1.1.9 resolves this issue ebenbruyns/junkstore#84 Issue now closed as fix is confirmed by author. |
|
The new version waiting to be merged to testing seems to also resolve this issue: ebenbruyns/junkstore#37 Leaving it open until we can confirm from new testing release. |
|
All issues appear to be resolved, LGTM! |
|
Just waiting on my own testers to complete their test report before thinking about moving to stable. |
|
Plugin Testing Report Installed Plugins Specifications Issues Summary
|
|
@EMERALD0874 when you get a chance can you deploy a new build with the last commit so we can verify that it works. I think this is probably very close to being ready. The last commit is a low impact change, just escaping spaces in a shell script essentially. |
|
@ebenbruyns deployed, please have a tester confirm then we should be good from there. |
Plugin Testing ReportInstalled Plugins: Specifications: Issues: Summary: New commit tested muptiple times on non-dev Steam Deck. Changed exe names for games to have a space. Behaves as expected, no longer breaks exe files into separate buttons if there is a space in the name. |
|
Plugin Testing Report Installed Plugins Specifications Issues Summary Tested naming of games that have a space in their name. If the name of an exe in the game folder has a space or is changed to have a space, the listing in JunkStore when displaying .exe's in directory display correctly and will properly change if I manually change the file name. Multiple attempts with different exe's in different game folders tested (changing/changing back/native) all respond properly |
github-project-automation
bot
moved this from 🔖 Ready for review
to 👀 In review
in SDH Tracker
Junk-Store
This merges PR's from PartyWumpus, GloriousEggroll and Ugzuzg.
PartyWumpus: changed scripts to work on nixos
GloriousEggroll: Added changes to make Umu fixes work correctly.
Ugzuzg: Changed sorting to be case insensitive on the game grid screen.
Fix for Garden Story where ${} was in the name the $ needed escaping.
Added args that needs to be passed to games that was over looked in the initial builds (very few if any games might be affected by this).
Task Checklist
Developer
Plugin
Backend
Community
Testing