-
Notifications
You must be signed in to change notification settings - Fork 791
Metro_RP2350_Minesweeper: Refactor for USB mouse library and new core USB errors #3118
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
Conversation
mouse_tg.x = max(0, min(display.width - 1, mouse.x // sensitivity)) | ||
mouse_tg.y = max(0, min(display.height - 1, mouse.y // sensitivity)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the USB_Host_Mouse library will update the mouse tilegrid position internally when mouse.update()
is called. Is it changing the position here again in order to apply the sensitivity division?
I noticed the sensitivity value used to not be a variable and was just hardcoded to 2
, but in the new version it's broken out to a variable and set to 4
. Was changing the value from 2
to 4
intentional?
Does this value basically just slow the mouse down to make it easier to be more precise with aiming it on the small squares of the game?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the hard coded 2 was actually just placing the cursor in the center of the screen. I don't think the original program used a sensitivity setting but I added it to PyPaint when you noticed the mouse was erratic and carried it forward here. To be honest I didn't test without the setting but have played many hours now and the mouse control seems good to me 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, looking at the code now, it does seem like the original code was using the divide by 2 to slow down the mouse which is the same thing I'm using sensitivity for. I didn't pay close attention to that section of code because the library returns absolute coordinates and the old code was translating delta movements so I couldn't use much of the old logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be nice eventually to add sensitivity
as a concept to the mouse library. It could be passed in as an constructor argument or set as a property and then have it used inside of update()
to divide the movement deltas down to lower numbers like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I'm sort of using a backdoor hack to get the results I'm looking for. I'll keep it on my list of things to look at... So many moving parts at the moment though 😁
After taking a closer look at this I realize now it's not using the mouse TileGrid that gets created internally by adafruit_usb_host_mouse. I've also found that it raises this exception when it's run on a device without the rest of Fruit Jam OS
adafruit_usb_host_mouse uses the OS mouse bmp file by default, but does provide a way for the code using it to pass in an alternative. I'm going to try to see if it will be difficult to change this to use the TileGrid that is inside the usb_host_mouse library instead of making it's own and only using the coordinates from usb_host_mouse. If that turns out to be easy enough then I'll have some changes to suggest. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opened a PR over in usb_host_mouse library that adds the sensitivity concept to BootMouse class and was able to test it successfully with a few changes to the minesweeper code.
Here is a diff of the changes: diff.patch
Or full contents of updated code.py: https://gist.github.com/FoamyGuy/09efb3a9df2d13e630486f5302f260dd
This version is able to use the internal mouse tilegrid from usb_host_mouse library instead of making and managing a separate one.
Thanks! I've only scanned the library and minesweeper changes but they look good to me. I'll be able to give them a better workout tonight. I'll also update my Larsio_Paint_Music PR and start working on a PR for PyPaint. Do you want me to go ahead and close this or merge in your changes? |
@RetiredWizard you can either close this and make a new PR when ready. Or leave this one open and commit the changes linked above. Either way is fine IMO, if you prefer one approach go for that. Thanks again for working on all of these! |
We should wait until adafruit/Adafruit_CircuitPython_USB_Host_Mouse#6 is merged before merging this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. Thanks for working on this refactor @RetiredWizard!
I tested the updated version successfully on Fruit Jam 10.0.0-beta.3
After a little more experience with the Adafruit_usb_host_mouse library it turned out not to be too hard to update this to the higher level mouse approach.
This became more pressing when the core started raising errors when a device.manufacturer couldn't be retrieved as the loop we were using to wait for the mouse to become ready wasn't catching any errors but instead, simply checking for a non-None response. We could have just started testing for the error but using the higher level library seemed like a better route.
I still hope to update minesweeper with the right mouse button feature but that's behind a couple other projects I'm working on at the moment.