Reimplement detect red clicks

[gabrielfoo]

I have done what was recommended in the deprecated message about red-click detection and have thoroughly tested it.

Red click detection + "contains=[]" made my bot many times more robust than ever before, so I believe this feature is worth a look at least.

[gabrielfoo]

Just a random thought, I wonder if it will be breaking if I put is_red_click() inside click(), between the mouse up and mouse down as part of the delay?

[kelltom]

Just a random thought, I wonder if it will be breaking if I put is_red_click() inside click(), between the mouse up and mouse down as part of the delay?

Honestly, I was under the impression that the screenshotting was happening too quickly, prior to the red-x sprite even displaying on the screen. So perhaps it's best to keep it outside of the mouse_up.

I will look at this today I believe.

[gabrielfoo]

Oh yeah, I forgot about the time it takes for the red sprite to appear.

Anyway, this enhancement only worked with game_view, as a cursor_view requires both the mouse and the character to not move, which severely limits its uses. So, I'm not sure about allowing the function to accept a Rectangle, as it may create a wrong impression that any Rect size is fine.

These are my thoughts, let me know yours if you have any :)

[kelltom]

Just doing a bit of testing now.

From what I can see, it takes anywhere from 20-50 ms to perform the search for the red click, which is pretty good.

I see what you're saying about the cursor_view, but I think it's still a better solution than using the game_view. It's a bit unfortunate that the mouse utility doesn't have access to the game_view directly, which then relies on the developer to add the game_view as an argument every time they call click_with_check(), which is not ideal really.

I have another idea which eliminates the need to modify the image_search function and passing around the game view.

What if, as soon as the click registers, it takes 3-4 screenshots of the cursor view in succession? Maybe we can make the cursor_view a little larger to compensate. Then, we can perform an image search for the click sprites across all screenshots. This way, we are not adding niche functionality to image search, and it's likely a lot faster than scanning an entire game view.

What do you think? I might try it real quick if I have time.

[kelltom]

I believe I've managed to make it work in about 3 ms using a version of the above method! The trick was to just increase the size of the cursor box

Edit: Actually that's not true, yet. I've still been using the modified image search. I see why you did it now... otherwise, it's take a new screenshot for each image search... hmmm

[kelltom]

Ok, I've debunked my previous theory of the sprite not appearing before the screenshot happens. It always appears in the screenshot, from what I can tell, which means we probably don't need a while loop... This would need to be tested on other machines though.
I'm doing this

    def click_with_check(self) -> bool:
        """
        Clicks on the current mouse position and checks if the click was red.
        Returns:
            True if the click was red, False if the click was yellow.
        """
        self.click()
        return self.__is_red_click()
     
    def __is_red_click(self) -> bool:
        """
        Checks if a click was red, must be called directly after clicking.
        Returns:
            True if the click was red, False if the click was yellow.
        """
        mouse_x, mouse_y = pag.position()
        mouse_rect = Rectangle.from_points(Point(mouse_x - 20, mouse_y - 20), Point(mouse_x + 20, mouse_y + 20))
        # Make sure the rectangle is within the screen. This may not cover all cases.
        mouse_rect.top = max(mouse_rect.top, 0)
        mouse_rect.left = max(mouse_rect.left, 0)
        cursor_sct = mouse_rect.screenshot()
        for click_sprite in ["red_1.png", "red_3.png", "red_2.png", "red_4.png"]:
            try:
                if imsearch.search_img_in_rect(imsearch.BOT_IMAGES.joinpath("mouse_clicks", click_sprite), cursor_sct):
                    return True
            except mss.ScreenShotError:
                print("Failed to take screenshot of mouse cursor. Please report this error to the developer.")
                continue
        return False

It's quite fast since it's not searching the entire game view -- about 3 - 8 ms on my rig.

There's an ultimatum: the image search function needs to either accept a list of images, or it must accept an image in place of a rectangle (which you've already implemented). I feel like the former has more use cases, but it sounds like a lot of work so I might just stick with this.

So, I've managed to eliminate the need for passing around the game view, but the image search modification is inevitable. Looks good though. I won't push changes unless you agree -- or I can make a branch off of your branch.

[gabrielfoo]

Sure, you can use my branch. I'm using your changes in my bot with no complaints. Thanks for your feedback.

I had a test case where I moved the mouse with my hand after a red_X(which failed, of course), so I came to the conclusion that game_view was needed. But this does not happen async in OSBC, so if I were to dismiss that test case, your changes work perfectly for me and there isn't a need for game_view.

For any other edge cases, I believe the right way to use click_with_check() is to put it in a loop that tries again if it fails. I'm sure that any rare scenarios like sprites not appearing in the screenshot can be fixed by "trying again". For that, I'm confident that my implementation plus your changes are fine. I'm happy to work on it further if there are bug reports, which we will only get if we push it out.

I'm not too sure how it would work with async mouse movements though (similar to my hand-movement test case), but I'm sure we could cross the bridge when we get there :D

[kelltom]

Hey, I came up with a good solution. When the click() function happens, it now records the cursor location when the mouseDown event triggers, meaning we now can draw a Rectangle around the exact area where the click happened. It's quite accurate, which should render looping unnecessary.

BTW, I got rid of the extra "click with check" function. the click() function now has a param that tells it to check for red clicks, so it'll change its return value.

Let me know what you think

[gabrielfoo]

Thanks, your new solution gave me the idea of also doing a Rectangle drawing after clicks. Then, the two rectangles will combine into a bigger one. This clears all test cases with great accuracy, including any async mouse movements, with a much smaller footprint than our previous solutions.

I think this will likely be a future-proof solution. What do you think?

Edit: The picture above is just a sample of failed cases, clicking while my hand is moving the mouse, which without combining, would have returned False.

[kelltom]

Neat, I'll check it out

[kelltom]

Sick! Awesome addition. This thing is pretty near bulletproof. Thanks for the contribution!

My last commit was just some formatting to comply with the style guide the project is following.

Thanks again!