Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to make ABaseInlet usage clearer, as AInlet will be deprecated #13

Open
cboulay opened this issue Jan 3, 2017 · 15 comments
Open

How to make ABaseInlet usage clearer, as AInlet will be deprecated #13

cboulay opened this issue Jan 3, 2017 · 15 comments
Assignees
Labels

Comments

@cboulay
Copy link
Collaborator

cboulay commented Jan 3, 2017

Am I missing something or did you just not get around to it yet? I'm working on a project using LSL4Unity now so I can attempt to reduce repeated code if desired. I'm new to C# though so I can't promise it'll be good.

@xfleckx xfleckx self-assigned this Jan 3, 2017
@cboulay
Copy link
Collaborator Author

cboulay commented Jan 3, 2017

I see you're writing an answer now so I'll keep this short: I now see that ABaseInlet looks like it is intended to replace AInlet eventually. Is this true?

@xfleckx
Copy link
Collaborator

xfleckx commented Jan 3, 2017

Yes! :)

Your observation is valid, their is some duplicated code within the AInlet and the BaseInlet.
BUT I would like to remove the AInlet implementation is somewhat ugly regarding to the usage of the ContinuousResolver. I intend to remove this implementation completely and stick to the BaseInlet implementation for the following reason:

AInlet already uses the Start, Update and FixedUpdate Methods and you need to override them if you want to change them. Since inheritance could be a root for complexity and bugs I was thinking on an approach using compositions which is more Unity'sh. As you can see, the ABaseInlet provides no implementation for Unity Messages (Start, Update etc.) so you are free to use them, which is I guess a bit to more easy understand for beginners.

Another reason was the usage of only on ContinuousResolver Instance within a separated MonoBehaviour, using UnityEvents to propagate the appearance and loose of Streams among a UnityScene.

I wrote a documentation for that

@xfleckx
Copy link
Collaborator

xfleckx commented Jan 3, 2017

What do you think about this approach!? Maybe it's to complicated - over engineered - ?

@cboulay
Copy link
Collaborator Author

cboulay commented Jan 3, 2017

I think this approach is good, but I think it's a bit confusing at the moment because the other examples and docs. Can you remove the old examples and update the docs?

I also think the provided example in the new wiki page is a bit confusing at the bullet point about "Reference the AStreamIsFound". (It is clear before that). Why does the screenshot have different A and B OnStreamFound references?

I think a better example (for clarity, not necessarily for simplicity) would show how to subscribe to the Resolver's events via code.

@xfleckx
Copy link
Collaborator

xfleckx commented Jan 3, 2017

Ok, I will take care on that...

@cboulay cboulay changed the title Inlets in AInlet.cs should inherit from ABaseInlet How to make ABaseInlet usage clearer, as AInlet will be deprecated Jan 3, 2017
@cboulay
Copy link
Collaborator Author

cboulay commented Jan 3, 2017

(Edited title of issue to reflect its true topic)

I'll edit this post with other little things I notice.

  • isTheExpected needs to make use of StreamType as well, when available.
  • In 'DemoInletForFloatSamples.cs', the timing about when the samples are pulled vs when the next game tick update happens is unclear.
    • It appears as though pullSamples might be called very soon after WaitForFixedUpdate() returns. I assume WaitForFixedUpdate() does not return until the frame is sent off to the GPU. If I'm correct then the latest pulled sample will be Process()ed while the previous frame is being rendered and the new value won't be reflected on screen until the end of the next frame. In my case (Vive), this is > 11 msec difference. This is probably not the preferred way to do it. I see the code comment about simply calling pullSamples in Update, which is probably the better way. I may be wrong though as I still don't quite get the difference between FixedUpdate() and Update().
  • I am creating a prefab that inherits InletFloatSamples. All instances will use StreamType='Control' by default, but it should be possible to override this in the Editor. Is there a way to override parent class parameters with new default values? The standard way doesn't work. For now I'm adding two new default_ parameters.

@xfleckx
Copy link
Collaborator

xfleckx commented Jan 3, 2017

Thanks for the comment, I see the problem with WaitForFixedUpdate() - you're absolute right about that I will change that accordingly.

The difference between FixedUpdate and Update is important for physics simulation or - as I assume - stable sampling rates for the game state since the update frequency depends heavily on game logic.
However, this is something which needs more investigation due to experiments and data aquasition/analysis protocolls.

@cboulay
Copy link
Collaborator Author

cboulay commented Jan 3, 2017

This problem deserves its own comment:

What happens if the resolver is already running then a new gameObject is instantiated and it wants access to an already-found stream? onStreamFound won't trigger because it is already in knownStreams.

  • Is it possible to hook into the event-registration mechanism then fire off the onStreamFound events for each stream in knownStreams but only for that listener?

@xfleckx
Copy link
Collaborator

xfleckx commented Jan 3, 2017

Nice question... 😅 Actually, I never thought to this point since I assume that the applications using LSL are not that kind of dynamic but ok let's see how this could be achieved...

I would think the most simple solution to this might be a new Method within resolver which could get used for requesting a specific stream. Example:

// a component inheriting from InletFloatSamples implementing your logic
void Start(){
      var resolver = FindObjectOfType<Resolver>();

     LSLStreamInfoWrapper streamInfo;
     bool isAvailable = resolver.IsStreamAvailable("yourStreamName", out streamInfo);

     if(isAvailable){
          // use it immediately
          this.AStreamIsFound(streamInfo);
     }else
     {
         // register listener dynamically for the event which is expected later....
         resolver.onStreamFound.addListener(this.AStreamIsFound);
     }

}

To answer your question... As far as I know, there is no option to invoke just one callback from the listener collection.

@xfleckx
Copy link
Collaborator

xfleckx commented Jan 3, 2017

To answer this question:

I am creating a prefab that inherits InletFloatSamples. All instances will use StreamType='Control' by default, but it should be possible to override this in the Editor. Is there a way to override parent class parameters with new default values? The standard way doesn't work. For now I'm adding two new default_ parameters.

It is possible to override the property using the new keyword but this might cause strange behavior and bugs cause at runtime it is not possible to decide which version of a property should be used except using explizit type casts. Due to Unitys serialization logic it will provide both properties in the inspector which might become ambiguous. So this might not be an option.. 😞 And Unity also raises an error if you try that.. 👍

However, I'm not completely sure what the problem is... The script on your prefab inherits from InletFloatSamples so it get's the properties from this base class.... Whenever you instantiate the prefab (in your scene) you could change the content of the properties for this single instance (as long you did not click the Apply-Button in the inspector) or make a copy of the prefab with the expected default values.

@cboulay
Copy link
Collaborator Author

cboulay commented Jan 4, 2017

However, I'm not completely sure what the problem is...

Ah. I'm showing my Unity-unfamiliarity here. I'm working on some components that I'm going to package up and share across different projects and, after sufficient testing, release as open source (Asset Store - someday). There are some assumptions that the general experimental framework makes about inlet type, control value range, etc, that I want reflected in the different assets. I hadn't thought to releasing the prefab, I thought only of releasing code, but in this case it makes sense to release the prefab. Thanks for reminding me of that.

I would think the most simple solution to this might be a new Method within resolver which could get used for requesting a specific stream.

That should work but it puts more burden on the user to add probably unfamiliar code to their Start() implementation. It would be nice if this code existed in ABaseInlet. What's the preferred way of doing that? (1) Requiring users to call OnStart() (or base.Start()) at the beginning of the subclass Start(); or (2) requiring users to forego implementing Start() in their base class altogether and instead implement StartImpl() which is called by ABaseInlet.Start()?

@cboulay
Copy link
Collaborator Author

cboulay commented Jan 4, 2017

BTW, here are the changes that I've made so far.

It's incomplete. For example, I can never get out of the while ((lastTimeStamp = inlet.pull_sample(sample, 0.0f)) != 0) loop because I'm Processing the samples slower than they are coming in. At least that's what I think the problem is. My pullChunk implementation isn't done yet either.

I just thought it might be useful for this discussion.

@xfleckx
Copy link
Collaborator

xfleckx commented Jan 5, 2017

Sorry for the late response, for this problem:

For example, I can never get out of the while ((lastTimeStamp = inlet.pull_sample(sample, 0.0f)) != 0) loop because I'm Processing the samples slower than they are coming in.

We should open a new issue... (made this #14 ). I would like to discuss this problem in detail. I suppose that a thread based approach might be necessary.

Due to this:

There are some assumptions that the general experimental framework makes about inlet type, control value range, etc,

I would recommend not to make too much dependencies/assumptions among framework components. I seems to be better to provide smaller building blocks with proper documentation which can be reused within custom scripts written by the user, which is the more Unity'sh way to build applications. I also tried to write a comprehensive framework for building experiments you can find the approach here <- this is absolutely not usable, I hopefully got time this year to improve the implementation. The major thing I learned during this work is, putting too much effort into abstraction makes things neither simpler nor useful 😞 Anyway, if you are interested in building a framework we could stick heads together and searching for a better approach 🙏

@cboulay
Copy link
Collaborator Author

cboulay commented Jan 5, 2017

Anyway, if you are interested in building a framework we could stick heads together and searching for a better approach.

Yes, I would like that. This discussion is off-topic and should probably go somewhere else, but I'm not sure where.

We're working towards a couple specific experiments at the moment and building the pieces we need for those as we go. I doubt I'll get there, but it would be great to offer functionality similar to NBS Presentation (or PsychToolbox, or E-Prime, or Superlab, etc). Of course Unity takes care of most of this, we just need the glue to make it an Experiment.

There are a few things that I'm focused on for the current projects.

  • Experimenter control panel.
    • Simple main scene with main camera (VR: participant), secondary camera (monitor: experimenter), and a GUI on the secondary camera (using IMGUI).
    • Change experimenter camera position to view the game world from different prescribed angles.
    • Some objects in the game world have visual cues only visible to the experimenter (e.g. HMD pose)
    • Switch Tasks via unloading and loading TaskScenes additively.
    • start/pause/resume/stop experiment.
    • Indicate task performance
    • Contains the Resolver
    • Can set number of trials and any other parameters the TaskScene exposes (e.g., number of targets)
  • Task state machine
    • Maintains a markers outlet.
    • Maintains one or more inlets.
    • User defines states.
    • User defines what happens in the transition to each state
    • State machine automatically sends out Marker strings on each transition.
  • Example scripts that hook in to the state machine
    • Use inlet signal to change renderer colour
    • Use inlet signal to change text
    • Use inlet signal to change transform
    • Send collision events to marker outlet
  • Example Tasks scenes
    • Centre-out
    • Neurofeedback

@xfleckx
Copy link
Collaborator

xfleckx commented Jan 5, 2017

Just a suggestion, we could put this discussion to the VREF repository. because, all you requirements you described are mainly target by this project, however, nothing is really finished. Or you create a new repository for these components and if want, you could invite me as a contributor and I put the interesting components from the VREF repository to yours.

The reusable components I see in this collection are:

  • A control gui for the experiment.
  • the camera rig for the experimenter and it's preview to the observation window using render textures.
  • the state machine -> which might be already available through Unitys Animation System

I would not spend that much time in abstraction due to the (experimental) task dependent requirements. Rather, I would spend more time to investigate and reuse Unitys Feature and document them proper for future experiments. As an example, the Experimenter View could be achieved through different layers.
OFFTOPIC Start
Since, I got a new job for the next months which reduces the amount of time enabled for this project to a minimum, thats why another even temporary maintainer of such a project would be a nice. I could provide some suggestions and advanced knowledge on Unity even I won't be able to implement any detail by my self. But, as I said, just a suggestion...
OFFTOPIC End

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants