GearVRf icon indicating copy to clipboard operation
GearVRf copied to clipboard

Handle input events and collision events with scripts

Open danke-sra opened this issue 9 years ago • 7 comments

Bring up this issue for discussion and some design / implementation efforts.

After the Input manager is in, we may need to work on application's handling of input events, in addition to the framework's handling of inputs.

What I have in mind is some steps below, but I am looking for inputs to revise them:

  1. Define input events in an interface like the IScriptEvents in the script patch. For example, add an interface called IInputEvents, and define signatures of handlers, like this
interface IInputEvents {
    void onKeyEvent(KeyEvent event);
    void onMotionEvent(MotionEvent event);
}

The actual event class may be changed to GVR equivalents (when we have them). We may also add a source parameter, and a return value.

  1. Possibly modify the input manager to allow adding these listeners to corresponding targets. We might need to consider both application-level listener, and fine-grained scene-object-level listeners.

  2. When invoking handling methods in the listeners, please use the GVREventManager. This allows for scripting support transparently.

After the above input event handling is in place, we can continue to add scripting support for input events. When input event support is done, we can extend it to other event groups, such as hit-test, visibility, collision, timers, etc.

@rahul27, can you please comment on this proposal? Do you have suggestions on its improvement / modification?

danke-sra avatar Jan 13 '16 22:01 danke-sra

@danke-sra If my understanding of the proposal is correct then I would like to address the issue at two levels:

  1. Application Level:

The current implementation of the input manager does not prevent the application from accessing the MotionEvents or KeyEvents generated by Android i.e. the app can do this by overriding the corresponding methods like below:

public class SampleActivity extends GVRActivity {

    private static final String TAG = SampleActivity.class.getSimpleName();

    @Override
    protected void onCreate(Bundle icicle) {
        super.onCreate(icicle);
        setScript(new SampleViewManager(), "gvr_note4.xml");
    }

    @Override
    public boolean dispatchGenericMotionEvent(MotionEvent event) {
        // TODO Auto-generated method stub
        return true;
    }

    @Override
    public boolean dispatchKeyEvent(KeyEvent event) {
        // TODO Auto-generated method stub
        return true;
    }

    @Override
    public boolean dispatchTouchEvent(MotionEvent event) {
        // TODO Auto-generated method stub
        return true;
    }
}

This allows the application to override the Frameworks handling of the events and implement its own handling mechanism like how vrf_controls_sample handles input:

https://github.com/gearvrf/GearVRf-Demos/blob/master/gvrf_controls_sample/src/org/gearvrf/controls/MainActivity.java

Since there is already a defined API defined by Android then the introduction of the IInputEvents in my opinion would add to the confusion in addition to duplicating the mechanism to receive these events.

Let me know if this makes sense, else correct me if wrong. Is there a bigger reason for exposing the Motion and Key Events at the application level differently?

Additionally, the application could very well define interfaces to distribute these events to the scene objects.

  1. Framework Level:

If we choose to add these interfaces for convenience at the Framework level and expose these events for use by the scripting framework then that should be fine as long as it is not exposed to the application (for reasons explained above). We could define private/package visible methods in the input manager to add and remove these interfaces and have the scripting or other interested internal classes use them.

rahul27 avatar Jan 13 '16 23:01 rahul27

@rahul27 Thanks for the input. As we discussed, I will try to add scripting for input / collision events. This may involve some refactoring. When it's ready, please kindly review it.

danke-sra avatar Jan 14 '16 00:01 danke-sra

@rahul27 Thanks for offering to refactor some of the code. I just looked at the SensorEventListener, and it seems the changes are straightforward:

  1. Add a base interface IEvents to SensorEventListener, so that it can be handled by GVREventManager
  2. Possibly use the new naming convention I*Events for SensorEventListener, like ISensorEvents, or IDetectorEvents, or IColliderEvents. Let's figure this out with Tom.
  3. In ListenerDelegate, use the GVREventManager to deliver the event. Instead of sensorEventListener.onSensorEvent(event);, we can do gvrEventManager.sendEvent(...). In this way, if the listener implements IScriptable, a handler function in the script will be automatically invoked.

These are some changes I can think of about the collision sensor. I think you are more experienced with input events, and you are more than welcome to contribute scripting support for that.

PS. If you have time, please help verify the script framework patch: https://github.com/Samsung/GearVRf/pull/371

danke-sra avatar Jan 15 '16 08:01 danke-sra

@danke-sra for now the proposed changes seem fine to me. I still have a few doubts about the script/event manager that hopefully should be cleared after I go through the patch.

rahul27 avatar Jan 16 '16 00:01 rahul27

@rahul27 Thanks. BTW, the script/event manager are in very primitive state. Comments are welcome. :)

danke-sra avatar Jan 16 '16 00:01 danke-sra

Conversion for collision events are ready for review: https://github.com/Samsung/GearVRf/pull/388

Motion events can be postponed after something is figured out with eye pointees. It is optional to support key events at the scene-object-level, because it is ambiguous which scene object a key event should be delivered to.

danke-sra avatar Jan 23 '16 02:01 danke-sra

@danke-sra here is a new pull request for review:

https://github.com/Samsung/GearVRf/pull/414

rahul27 avatar Feb 18 '16 00:02 rahul27