return of find() unlike Subregion().inside().find()

[RaiMan]

find() returns the first/best Match. If you want to work with the Matches, you have to use find.regions.

If you use Subregion().inside().find(), you get Matches and have to say

Subregion().inside().find().region or Subregion().inside().find()[0] to refrence the same match object.

This is confusing and not straight forward.

find() and Subregion().inside().find() should deliver the same result, so that for performance reasons, a find() searching the whole screen can just be exchanged by a Subregion().inside().find(). This is even clearer, when using the possibilities of Pattern to limit the search with .similar(1).firstN(1)

[RaiMan (raimund-hocke)]

# --------------------------------- a script to test it
img =( ) # its the apple logo ;-)

# standard find whole screen
tf = find(img)
print tf
print ">>> with find: x = "+str(tf.getX())

# standard find upper left corner only, match from find.region
ts = Subregion(0, 0, 200, 200).inside().find(img)
ts = find.region
print ts
print ">>> with find_inside_region using find.region: x = "+str(ts.getX())

# standard find upper left corner only, using mach directly
ts = Subregion(0, 0, 200, 200).inside().find(img)
print ts
print ">>> with find_inside_region using find.region: x = "+str(ts.getX())

# --------------------------------- the result
capture: java.awt.Rectangle[x=0,y=0,width=1920,height=1200]
1 matches found
Match[470,122-25x24 1,00]@/var/folders/C9/C9+A8gl7F9GRQtCP6jc+VU+++TI/-Tmp-/sikuli8773201938289071842.png
>>> with find: x = 470
[sikuli] SubregionJ: 0,0,200,200 edu.mit.csail.uid.SikuliScript@10b755d
capture: java.awt.Rectangle[x=0,y=0,width=200,height=200]
1 matches found
Match[470,122-25x24 1,00]@/var/folders/C9/C9+A8gl7F9GRQtCP6jc+VU+++TI/-Tmp-/sikuli8773201938289071842.png
>>> with find_inside_region using find.region: x = 470
[sikuli] SubregionJ: 0,0,200,200 edu.mit.csail.uid.SikuliScript@10b755d
capture: java.awt.Rectangle[x=0,y=0,width=200,height=200]
1 matches found
[Match[16,1-25x24 1,00]@/var/folders/C9/C9+A8gl7F9GRQtCP6jc+VU+++TI/-Tmp-/sikuli2547078643922503913.png]
[sikuli] [Error] source lineNo: 17
[sikuli] [Error] Traceback (innermost last):
  File "/var/folders/C9/C9+A8gl7F9GRQtCP6jc+VU+++TI/-Tmp-/sikuli-tmp3351939762081648958.py", line 17, in ?
AttributeError: getX

[Tsung-Hsiang Chang (vgod)]

We will review all the APIs and revise them to be more consistent. Thanks!

[Tsung-Hsiang Chang (vgod)]

I just draft a proposal of new API design. Let's discuss this at https://blueprints.launchpad.net/sikuli/+spec/revise-api.
Comments and ideas are welcome.

[RaiMan (raimund-hocke)]

looks good and really straightforward.

what I miss:

- Subregion should accept Match and capture() as input for construction e.g. s = Subregion(Match), s = Subregion(capture())

- as a convenience Subregion should have setX() .... (now: sub = sub(sub.getX()+z, sub.getY(), ...) then: sub.setX(sub.getX()+z)

- it should be possible to click a Subregion (= calculated click point)

- spatial operators with Subregion (not written down)

- that find() AND findAll() (and all the action functions) work with these spatial ops: match/subregion.inside().find()/findAll() e.g. (in the moment Subregion(x, y, w, h).inside().findAll() gives attribute error on findAll (find works))

- all objects that have a pos/dim should understand getX(), getY(), getW(), getH(), getRect() (not really clear, with Subregion now it is Subregion.x, ....)

- as a convenience all functions, that accept a Pattern as first parm (except dragDrop: possible to use it for the drag if given, use it for both wouldn't make sense normally) should accept the same methods as Pattern does: e.g. find(img).similar(1).firstN(1) should be possible. If not an image (= string) they should be silently ignored or may be a warning when run inside IDE.

additional comments:

hoverAll doesn't make sense without an additional timing parm

clickAll/repeatClickAll: whats really the difference? if I now (new API) would say: clickAll(Matches) every contained match would be clicked, I understand. Whats then the add on of repeatClickAll? In the new API I would expect a repeatWhatever to do the following: the first time, it is called it sets up itself by using findAll and makes action on the first Match and returns some ID. then if you call it again by saying repeatWhatever(ID) it acts on the next Match and so on. If no more Match is there it returns None.

dragDropAll should accept (single, multiple) what means, that single is dropped to every of multiple. if given (Matches, Matches) you have a problem, since they are not sorted in a way a user would expect (e.g. based on pos from top left to bottom right). How to match drag position an drop position? So I think, in the current implementation, where there its not possible to define the order within Matches (e.g. by saying Matches.topLeftBottomRight() or Matches.topBottomLeftRight()) dragDropAll only makes sense as
dragDropAll( Pattern/String/Match, Matches )

it should read:
sleep( seconds ) #deprecated - use wait( seconds ) instead

untilNotExist ---> waitVanish

PathToAnImage? capture( [Subregion/Match/Matches] ), capture( x, y, w, h ) should read
String capture( [Subregion/Match/Matches] ), capture( x, y, w, h ) # returns path to image

find.region and find.regions should be mentioned together with: after all actions, where a find/findAll is executed implicitly (e.g. click actions) find.region and find.regions are defined and filled accordingly

[RaiMan (raimund-hocke)]

One more thing:

Subregion().inside().wait() should work (same problem as with Subregion().inside().findAll() now) since it boosts performance
same goes for waitVanish(), type() and paste()

[Tsung-Hsiang Chang (vgod)]

@RaiMan:
Thanks for your great comments!
I have incorporated your comments into the draft. Please let me know if you have further thought.

[RaiMan (raimund-hocke)]

Since I really like the approach of Sikuli, its a pleasure for me to give some input for further development. So you are welcome.

on the draft itself:

- You should give every function its own line. Its clearer and easier to scan.

- dragdrop/dragdropAll should read
dragDrop( Pattern/String/Match/Matches, Pattern/String/Match/Matches/List or Tuple of 2 absolute coordinates [x,y], [delay1], [delay2] )
dragDropAll( Pattern/String/Match/Matches, Pattern/String/Match/Matches/List or Tuple of 2 absolute coordinates [x,y], [delay1], [delay2], [delay between each dragDrop] ) # only dragDropAll(multiples, single) or dragDropAll(single, multiples) are accepted
to avoid the question: Whats special with the second form?

- In the intro, you should state once, that every timing parm (timeout, delay) have to be specified as seconds and can be a decimal. so you can omit this info at the function itself.
This in mind: wait, untilNotExist, waitVanish should read: ... ( [Pattern/String/Match/Matches], timeout )

- with ...clickAll/hoverAll it should read [delay between the single actions]

comments on the content itself:

- with dragDrop/dragDropAll: what do delay1/delay2 do? I guess internally, first the finds are processed. If for source and target at least one match is found, the action click-hold, mouse-move, click-release is processed accordingly. So I guess: click-hold, delay1, mouse-move, delay2, click-release???

- with dragDropAll: the delay parms should be in this order: [delay between each dragDrop], [delay1], [delay2] because I think the between-delay is used more often

- Actions on Subregions directly (actions: single-clicks, type, paste):
idea: Subregion should be a valid parm for these actions e.g. click(Subregion or list/tuple with 4 (region) or 2 (point) values). why?: convenience and performance (without this: click(capture(x,y,w,h)) which has to go thru find and produces an image)

Here I have a problem with the API doc:
Intro now:
Subregion limits the region that find() works on. It will support all action commands as well as spatial operators.

Class Subregion now:
all the actions are listed, what should be understood as Subregion.click() being possible. But with the given parms as Pattern/String/Match/Matches, this makes no sense, since it is already possible with Subregion.inside().click(). I think this is straightforward, because it implies, that with the spatials always something has to be found.

Conclusion:
So I think its clearer to allow action(Pattern/String/Match/Matches/Subregion, ...)
In the Intro you could say:
Subregion limits the region that find() works on. It will support all action commands via the spatial operators. As a convenience Subregion can be a parameter for all actions that accept Pattern/String/Match/Matches and can always be substituted by a list or tuple with 4 values (a region) or 2 values (a point).
Class Subregion: don't list the actions

- as a consequence Subregion/Match/Matches.find/findAll() should not be allowed, since its possible with x.inside().find/findAll() already.

- NEW class Subregions: since Match differs from Subregion only in knowing its content plus similarity, with this overall r...

[RaiMan (raimund-hocke)]

version 0.10: its a fun now!