X-1.0rc3: Region.onChange(handler, Integer) should give an error, instead of silently ignoring the Integer

Bug #905435 reported by Josh
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SikuliX
Fix Released
Critical
RaiMan

Bug Description

***** problem

using onChange(handler, integer) seems to work, but it is a wrong usage according to the API docs. In fact the second parameter is simply ignored in this case.

This usage should give an error.

---------------------------------------

The other observations here are due to the fact, that in some cases, a Match object does not work like a Region, which is a known problem.

*** workaround
cast the Match to a Region:

matchRegion = Region(some_match)

------------------------------------------

Problem: The Match.onChange() method does not reflect the same behavior as Region.onChange()

--
from sikuli.Sikuli import Match

def test(event):
    print event

region = Match(100,100,200,200, 2.5)
region.onChange(test, 50)
region.observe()

####
Traceback (most recent call last):
  File "C:\work\killpy\test.py", line 7, in <module>
    region.onChange(test, 50)
TypeError: onChange(): 1st arg can't be coerced to int

---
Swapping, onChange arguments as follows:
region.onChange(50, test)

####
Traceback (most recent call last):
  File "C:\work\killpy\test.py", line 7, in <module>
    region.onChange(50, test)
TypeError: onChange(): 2nd arg can't be coerced to org.sikuli.script.SikuliEventObserver
--

However, the following code does work:
--
from sikuli.Sikuli import Region

def test(event):
    print event

region = Region(100,100,200,200)
region.onChange(test, 50)
region.observe()

####
ChangeEvent on Region[100,100 200x200]@Screen(0) E:Y, T:3.0 | 1 changes
ChangeEvent on Region[100,100 200x200]@Screen(0) E:Y, T:3.0 | 1 changes
---

However this does not reflect the documentation which states the minChangedSize as the 1st argument and the handler as the 2nd argument. In reality it is flipped around.

http://sikuli.org/docx/region.html?highlight=onchange#Region.onChange

Tags: fkt-observe
Josh (sammysnake)
description: updated
description: updated
description: updated
description: updated
description: updated
description: updated
Revision history for this message
RaiMan (raimund-hocke) wrote :

--- from sikuli.Sikuli import Match/Region
does not make sense, because all classes needed by Sikuli are internally imported, when running a Sikuli script with the means of Sikuli

--- region = Match(100,100,200,200, 2.5)
does not make sense, because it is simply a region and setting a similarity value for a region does not make sense either. So in this case it would be better to say:
region = Region(100,100,200,200)

The only thing that might make sense, if you want to duplicate an existing match, because you want to have different target offsets:
m1 = find("some-image.png")
m2 = Match(m1)

now you have two different match objects.

--- region.onChange(test, 50) works
yes it works (and this is a bug), but it simply ignores the 50 in this case (or whatever the second parameter contains). So it is the same as:
region.onChange(test)

--- documentation bug - no!
the documentation reflects exactly how it should be:
if there are two parameters, then the first one has to be an integer and the second one is taken as the handler. All other combinations of two parameters are wrong and should lead to an error.

--- Match does not behave like a Region
Yes, there are still some problems with the Region behavior of Match objects in some cases (observe is one of them). This is a known problem (a Region class exists on both levels Jython and Java, which mixes things up in some cases) with an easy workaround: cast the Match to a Region and it works as expected:

m1 = Region(find("some-image.png"))
m1.onChange(50, handler)

--- so what's left
I will reduce your bug report to the fact, that onChange(test, 50) should give an error instead of silently ignoring the second parameter.

summary: - Match.onChange does not reflect Region.onChange method
+ Region.onChange(handler, Integer) should give an error, instead of
+ silently ignoring the Integer
RaiMan (raimund-hocke)
description: updated
summary: - Region.onChange(handler, Integer) should give an error, instead of
- silently ignoring the Integer
+ X-1.0rc3: Region.onChange(handler, Integer) should give an error,
+ instead of silently ignoring the Integer
RaiMan (raimund-hocke)
Changed in sikuli:
status: New → Fix Committed
assignee: nobody → RaiMan (raimund-hocke)
milestone: none → x1.0
RaiMan (raimund-hocke)
tags: added: fkt-observe
Revision history for this message
RS (robsk) wrote :

When trying the duplication of an existing match, as mentioned above:

m1 = find("some-image.png")
m2 = Match(m1)

It gives:

sikuli.script.Match(): expected 2 or 6-7 args; got 1

When I tried a few things as second argument, it gave:

2nd arg can't be coerced to org.sikuli.script.IScreen

What can be specified as the second argument to make the above example work?

Revision history for this message
RaiMan (raimund-hocke) wrote :

@RS
If you ask questions in questions, that are not yours, you should subscribe (did it for you this time) to the question or make your own question (which is always better).

in the current version you might use:
m2 = Match(m1, None)

hoping it does what you want ;-)

This will be fixed in the new version, where
m2 = Match(m1)

would do what you expect.

Revision history for this message
RS (robsk) wrote :

Thanks so much Raiman.

I do use m2 = Match(m1, None) in some places. However when I tried the same thing in another place, it gave me NullPointerException, which was why I tried other things and got the "2nd arg can't be coerced to org.sikuli.script.IScreen" message.

Glad to hear that it will fixed in the new version!

RS

RaiMan (raimund-hocke)
Changed in sikuli:
importance: Undecided → Critical
RaiMan (raimund-hocke)
Changed in sikuli:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.