Board index FlightGear Development Canvas

Live change for setChecked/setCheckable checkbox

Canvas is FlightGear's new fully scriptable 2D drawing system that will allow you to easily create new instruments, HUDs and even GUI dialogs and custom GUI widgets, without having to write C++ code and without having to rebuild FlightGear.

Live change for setChecked/setCheckable checkbox

Postby F-JJTH » Thu Nov 20, 2014 2:44 pm

Hi,

I'm playing with Canvas GUI, and I'm missing 2 features about the canvas checkbox:

first case:
Create a canvas window and insert a checkbox, with "setCheckable()" to false. I want that this checkbox become checkable as soon as another checkbox is checked.
Unfortunately it's not possible because "setCheckable()" doesn't listen to my other checkbox.

second case:
Again, a canvas window with a checkbox and "setChecked()" to false. I want that this checkbox become checked without user action (the user don't click on the checkbox, instead he press a button on his joystick).
Unfortunately it's not possible because "setChecked()" doesn't listen to a property.


In order to fit my need I implemented this:
Code: Select all
diff --git a/Nasal/canvas/gui/widgets/Button.nas b/Nasal/canvas/gui/widgets/Button.nas
index e185323..3e594b4 100644
--- a/Nasal/canvas/gui/widgets/Button.nas
+++ b/Nasal/canvas/gui/widgets/Button.nas
@@ -24,6 +24,13 @@ gui.widgets.Button = {
     me._checkable = checkable;
     return me;
   },
+  setCheckableProp: func(prop)
+  {
+    setlistener(prop, func(v){
+      me._checkable = v.getBoolValue();
+    });
+    return me;
+  },
   setChecked: func(checked = 1)
   {
     if( !me._checkable or me._down == checked )
@@ -36,6 +43,14 @@ gui.widgets.Button = {
     me._onStateChange();
     return me;
   },
+  setCheckedProp: func(prop)
+  {
+    setlistener(prop, func(v){
+      me._down = v.getBoolValue();
+      me._onStateChange();
+    });
+    return me;
+  },
   setDown: func(down = 1)
   {
     if( me._checkable or me._down == down )


With this change I have everything working as I want.
Does it look ok ? can I commit it ?

Regards,
Clément
User avatar
F-JJTH
 
Posts: 697
Joined: Fri Sep 09, 2011 11:02 am

Re: Live change for setChecked/setCheckable checkbox

Postby Hooray » Thu Nov 20, 2014 3:36 pm

Actually, you would normally implement such things separately by wrapping your me reference in a closure so that it can be used by the callback registered for listener/timer.

Thus, the "conventional" method would be to register a listener that updates the other checkbox.

(unless I am missing something here obviously)
Please don't send support requests by PM, instead post your questions on the forum so that all users can contribute and benefit
Thanks & all the best,
Hooray
Help write next month's newsletter !
pui2canvas | MapStructure | Canvas Development | Programming resources
Hooray
 
Posts: 11317
Joined: Tue Mar 25, 2008 8:40 am

Re: Live change for setChecked/setCheckable checkbox

Postby Philosopher » Thu Nov 20, 2014 5:38 pm

You need to make sure that the setlistener gets destroyed on .del(), otherwise it will be leaked and not freed. I'm not sure if each element's .del() is called when the window is destroyed... if it isn't, then we should probably add a method to C++ to keep track of a listener (Element.setlistener or Widget.setlistener or something) so that the listener will be freed when the element or widget is deleted.
Thanks,
Philosopher
(inactive but lurking occasionally...)
Philosopher
 
Posts: 1590
Joined: Sun Aug 12, 2012 6:29 pm
Location: Stuck in my head...
Callsign: AFTI
Version: Git
OS: Mac OS X 10.7.5

Re: Live change for setChecked/setCheckable checkbox

Postby F-JJTH » Fri Nov 21, 2014 2:50 am

Philosopher wrote in Thu Nov 20, 2014 5:38 pm:You need to make sure that the setlistener gets destroyed on .del(), otherwise it will be leaked and not freed.


Yep it was the thing I mainly wondered, I would need to investigate, help is welcome because right now I have no idea how to delete my listener when I close my canvas window.
Example of code are welcome ;)
User avatar
F-JJTH
 
Posts: 697
Joined: Fri Sep 09, 2011 11:02 am

Re: Live change for setChecked/setCheckable checkbox

Postby Philosopher » Fri Nov 21, 2014 4:51 am

For setlistener I would do something like this:

Code: Select all
var <Class> = {
    new: func(...) {
        me.listeners = [];
    },
    setlistener: func(arg...) {
        var id = call(setlistener, arg);
        append(me.listeners, id);
    },
    del: func() {
        ...;
        foreach (var l; me.listeners)
            removelistener(l);
    },
    ...
};


(sorry if the code is too long, it's basically "id = setlistener(fn); removelistener(id)")
Thanks,
Philosopher
(inactive but lurking occasionally...)
Philosopher
 
Posts: 1590
Joined: Sun Aug 12, 2012 6:29 pm
Location: Stuck in my head...
Callsign: AFTI
Version: Git
OS: Mac OS X 10.7.5

Re: Live change for setChecked/setCheckable checkbox

Postby F-JJTH » Fri Nov 21, 2014 9:55 am

I almost did what you say yesterday without success. But I see that you use
Code: Select all
var id = call(setlistener, arg);


where I use
Code: Select all
var id = setlistener(prop, func(v){
  *some nasal here*
});


Anyway here is the current state of my patch:
Code: Select all
diff --git a/Nasal/canvas/gui/Widget.nas b/Nasal/canvas/gui/Widget.nas
index bcbd1c4..1812c7a 100644
--- a/Nasal/canvas/gui/Widget.nas
+++ b/Nasal/canvas/gui/Widget.nas
@@ -9,6 +9,7 @@ gui.Widget = {
       _hover: 0,
       _enabled: 1,
       _view: nil,
+      _listeners: [],
       _pos: [0, 0],
       _size: [32, 32]
     });
@@ -121,6 +122,13 @@ gui.Widget = {
       me._view = nil;
     }
 
+    print("onRemove()");
+    foreach(var l; me._listeners)
+    {
+      print("onRemove::removelistener");
+      removelistener(l);
+    }
+
     if( me._focused )
       me.getCanvas()._focused_widget = nil;
   },
@@ -173,6 +181,13 @@ gui.Widget = {
       );
     return me;
   },
+  _registerListener: func(id)
+  {
+    print("_registerListener()");
+    append(me._listeners, id);
+    print("nb of listener: "~size(me._listeners));
+    return me;
+  },
   _windowFocus: func
   {
     var canvas = me.getCanvas();
diff --git a/Nasal/canvas/gui/widgets/Button.nas b/Nasal/canvas/gui/widgets/Button.nas
index e185323..888ac69 100644
--- a/Nasal/canvas/gui/widgets/Button.nas
+++ b/Nasal/canvas/gui/widgets/Button.nas
@@ -24,6 +24,13 @@ gui.widgets.Button = {
     me._checkable = checkable;
     return me;
   },
+  setCheckableProp: func(prop)
+  {
+    setlistener(prop, func(v){
+      me._checkable = v.getBoolValue();
+    });
+    return me;
+  },
   setChecked: func(checked = 1)
   {
     if( !me._checkable or me._down == checked )
@@ -36,6 +43,16 @@ gui.widgets.Button = {
     me._onStateChange();
     return me;
   },
+  setCheckedProp: func(prop)
+  {
+    var id = setlistener(prop, func(v){
+      print(v.getBoolValue());
+      me._down = v.getBoolValue();
+      me._onStateChange();
+    });
+    me._registerListener(id);
+    return me;
+  },
   setDown: func(down = 1)
   {
     if( me._checkable or me._down == down )


and this is the result in the console:
Code: Select all
Enabling ATI viewport hack
Starting automatic scenery download/synchronization. Using built-in SVN support. Directory: '/home/clement/FlightGear/TerraSync'.
environment init
onRemove()     <-- here I open my canvas window
_registerListener()  <-- this is automaticalled at same time I opened my canvas window
nb of listener: 1
1  <-- here I press the button of my joystick, everything works as I want
0
1
0
onRemove()  <-- here I close my canvas window, I should see   "print("onRemove::removelistener");" also but it looks like the foreach loop doesn't work :/
onRemove()  <-- here I open again my canvas window
_registerListener()
nb of listener: 1
1  <-- here I press the button of my joystick
1  <-- and now 2 listeners are executed... that's what I have to fix
0
0
1
1
0
0


So I conclude that the listener is correctly created (because he works) and registered ( "_registerListener()") but when I want to delete my listener it doesn't work because it seems that me._listeners is empty that's why the foreach() is not executed.

[EDIT] I added a "size(me._listeners)" in the onRemove() method and it return 0. So I don't understand what append... in the _registerListener() method it works as expected (append(me._listeners, id); size(me._listeners, id); give me "1" as expected) but when I want to retrieve my me._listeners vector in the onRemove() method I discover that me._listeners is empty :shock:


Any suggestion ?
User avatar
F-JJTH
 
Posts: 697
Joined: Fri Sep 09, 2011 11:02 am

Re: Live change for setChecked/setCheckable checkbox

Postby TheTom » Fri Nov 21, 2014 10:54 am

I don't like the idea of extending the API to support properties directly. What if the next one wants to connect other methods to properties? I would like to see something more general, probably something like listener objects, which automatically destroys the associated listenere if it gets garbage collected. You would just have to add the listener object to the widget/object/etc. to whose lifetime it should be bound and would automatically get it cleaned up if the object is garbage collected. I already got some problems with methods called on destruction and want to let the garbage collector handle as much as possible.

Maybe we could add something similar to maketimer or qt connect:
Code: Select all
# Let the listener be destroyed once listener_ref is garbage collected
var listener_ref = makelistener(<prop>[, <self>], <func>);

# Wrapper around makelistener to tie the resulting object to the lifetime of an object
# (For hashes this would be trivial. I'm not yet sure what the best solution would be
# for ghosts. We would need space for an additional naRef per ghost instance
# somewhere, or an global lookup table...)
connect(<prop>, <obj>, <func>);
TheTom
 
Posts: 321
Joined: Sun Oct 09, 2011 10:20 am

Re: Live change for setChecked/setCheckable checkbox

Postby Hooray » Fri Nov 21, 2014 11:53 am

I agree completely, and having a dedicated OO API for listener management, analogous to maketimer(), would be in line with Zakalawe's proposal here:

Listener objects
zakalawe wrote:Dumb question of the day. Someone recently pointed out that the setlistener() / removelistener() API makes it easy to leak resources. So I wondered about making an alternate API where the return value from setlistener must be kept, or the listener is removed. I can imagine this with a helper object

var myL = setlistener2("some/prop", func { ... }} )
myL.addprop("some/other/prop");
myL.addprop("yet/another/prop");

Now you need to retain a ref to myL or the listeners on all the props are removed. I don't think we can retro-fit this to the existing API, because I suspect many places just ignore the return value and would break with this change.

(Or even pass a vec of property names to setlistener2, to avoid all the discrete calls to 'addprop', that's a seperate detail really)

It seems like this would work, and be easy enough to implement - question is if it gives enough benefit to be worth the confusion.
Please don't send support requests by PM, instead post your questions on the forum so that all users can contribute and benefit
Thanks & all the best,
Hooray
Help write next month's newsletter !
pui2canvas | MapStructure | Canvas Development | Programming resources
Hooray
 
Posts: 11317
Joined: Tue Mar 25, 2008 8:40 am

Re: Live change for setChecked/setCheckable checkbox

Postby F-JJTH » Fri Nov 21, 2014 4:00 pm

Ok, so my working solution is not welcome.
Do you plan to commit a working solution that you like soon ? I'm asking because I can't continue to work on my project until this issue is not addressed.
User avatar
F-JJTH
 
Posts: 697
Joined: Fri Sep 09, 2011 11:02 am

Re: Live change for setChecked/setCheckable checkbox

Postby Hooray » Fri Nov 21, 2014 4:03 pm

Given that you have commit access to $FG_SRC, you'd only need to expose SGPropertyChangeListener via cppbind, for which you can refer to the cppbind/maketimer() implementation wrapping SGTimer ($FG_SRC/Nasal / FGNasalSys) according to the proposal outlined above: http://wiki.flightgear.org/Nasal/CppBind
Please don't send support requests by PM, instead post your questions on the forum so that all users can contribute and benefit
Thanks & all the best,
Hooray
Help write next month's newsletter !
pui2canvas | MapStructure | Canvas Development | Programming resources
Hooray
 
Posts: 11317
Joined: Tue Mar 25, 2008 8:40 am

Re: Live change for setChecked/setCheckable checkbox

Postby F-JJTH » Fri Nov 21, 2014 4:11 pm

Well to be honest I'm not interested to dig into CppBind :/ I never looked at this part of the source code so I have no clue what happen in this part of SG but not really seduced to look at it.
TheTom is working on this source code since lot of month, it's his baby and I wouldn't touch his baby at least for respect.
User avatar
F-JJTH
 
Posts: 697
Joined: Fri Sep 09, 2011 11:02 am

Re: Live change for setChecked/setCheckable checkbox

Postby TheTom » Sat Nov 22, 2014 2:56 pm

I started to work on this, but it turned out to be a bit more complicated than I thought, due to issues with the garbage collector. But I think they should be solveable and I already got an idea (have a look at the mailing list if you are interested...).
TheTom
 
Posts: 321
Joined: Sun Oct 09, 2011 10:20 am


Return to Canvas

Who is online

Users browsing this forum: No registered users and 0 guests