-
Notifications
You must be signed in to change notification settings - Fork 46
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
Create ScreenRayTracker.cs #129
base: master
Are you sure you want to change the base?
Conversation
I've added the possibility to provide trials using tab-separated-values (tsv) so people can use "," in strings.
In additition to the screen tracker I also quickly edited the CSV session builder and created a TSV session builder with it because people might want to use "," in their strings. |
Hi, I don't have time to review right now but I can see several problems immediately, that would need to be addressed before merging. I will add comments. |
/// <summary> | ||
/// Build a table from lines of TSV text. | ||
/// </summary> | ||
/// <param name="tsvLines"></param> | ||
/// <returns></returns> | ||
public static UXFDataTable FromTSV(string[] tsvLines) | ||
{ | ||
string[] headers = tsvLines[0].Split('\t'); | ||
var table = new UXFDataTable(tsvLines.Length - 1, headers); | ||
|
||
// traverse down rows | ||
for (int i = 1; i < tsvLines.Length; i++) | ||
{ | ||
string[] values = tsvLines[i].Split('\t'); | ||
|
||
// if last line, just 1 item in the row, and it is blank, then ignore it | ||
if (i == tsvLines.Length - 1 && values.Length == 1 && values[0].Trim() == string.Empty ) break; | ||
|
||
// check if number of columns is correct | ||
if (values.Length != headers.Length) throw new Exception($"TSV line {i} has {values.Length} columns, but expected {headers.Length}"); | ||
|
||
// build across the row | ||
var row = new UXFDataRow(); | ||
for (int j = 0; j < values.Length; j++) | ||
row.Add((headers[j], values[j].Trim('\"'))); | ||
|
||
table.AddCompleteRow(row); | ||
} | ||
|
||
return table; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should probably be a separate Pull Request, and made generic with the CSV version to accept a delimiter, rather than copying the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Completely agree :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, just changing the experimenter builder is not enough because then what might happen is that a variable such as a string that is to be displayed to the participant contains and a comma, which UXF then tries to save, which doesn't work if the the output file is an .csv. I currently have work-around where I just replace a comma with an underscore before it is saved but tihis prevents me to copy the data from the input file to the result file.
namespace UXF | ||
{ | ||
public class TSVExperimentBuilder : MonoBehaviour, IExperimentBuilder | ||
{ | ||
|
||
[Tooltip("The name key in the settings that contains the name of the trial specification file.")] | ||
[SerializeField] private string tsvFileKey = "trial_specification_name"; | ||
[Tooltip("Enable to copy all settings from each trial in the TSV file to the the trial results output.")] | ||
[SerializeField] private bool copyToResults = true; | ||
|
||
/// <summary> | ||
/// Reads a TSV from filepath as specified in tsvFileKey in the settings. | ||
/// The TSV file is used to generate trials row-by-row, assigning a setting per column. | ||
/// </summary> | ||
/// <param name="session"></param> | ||
public void BuildExperiment(Session session) | ||
{ | ||
// check if settings contains the tsv file name | ||
if (!session.settings.ContainsKey(tsvFileKey)) | ||
{ | ||
throw new Exception($"TSV file name not specified in settings. Please specify a TSV file name in the settings with key \"{tsvFileKey}\"."); | ||
} | ||
|
||
// get the tsv file name | ||
string tsvName = session.settings.GetString(tsvFileKey); | ||
|
||
// check if the file exists | ||
string tsvPath = Path.GetFullPath(Path.Combine(Application.streamingAssetsPath, tsvName)); | ||
if (!File.Exists(tsvPath)) | ||
{ | ||
throw new Exception($"TSV file at \"{tsvPath}\" does not exist!"); | ||
} | ||
|
||
// read the tsv file | ||
string[] tsvLines = File.ReadAllLines(tsvPath); | ||
|
||
// parse as table | ||
var table = UXFDataTable.FromTSV(tsvLines); | ||
|
||
// build the experiment. | ||
// this adds a new trial to the session for each row in the table | ||
// the trial will be created with the settings from the values from the table | ||
// if "block_num" is specified in the table, the trial will be added to the block with that number | ||
session.BuildFromTable(table, copyToResults); | ||
} | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same with this, it should be generic with specified delimiter
/// Attach this component to any gameobject (e.g. an empty one) and assign it in the trackedObjects field in an ExperimentSession to record | ||
/// if ray casted from camera is hitting anything. NOTE: Update Type must be set to MANUAL. | ||
/// </summary> | ||
public class ScreenRayTracker : Tracker { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be in UXF namespace
/// Attach this component to any gameobject (e.g. an empty one) and assign it in the trackedObjects field in an ExperimentSession to record | ||
/// if ray casted from camera is hitting anything. NOTE: Update Type must be set to MANUAL. | ||
/// </summary> | ||
public class ScreenRayTracker : Tracker { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should use consistent brace style (opening brace {
on new line)
|
||
[Header("Ray coordinates")] | ||
[TextArea(10, 10)] | ||
public string HowToUse = "Please provide coordinates in form of lists named ray_x & ray_y in your .json file. For example \n\"ray_x\": [0.5],\n\"ray_y\": [0.5]\nif you want to have one ray in the middle of the screen. Multiple rays can be provide with this method."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should not be here, better in the wiki
/// <summary> | ||
/// Function to detect objects on screen by rays | ||
/// </summary> | ||
List<string> ray2detectObjects(List<float> x, List<float> y, Camera cam){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Methods should be PascalCase
} | ||
|
||
/// <summary> | ||
/// Function to detect objects on screen by rays |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment is not useful
public override string MeasurementDescriptor => "ObjectsOnScreenTracker"; | ||
public override IEnumerable<string> CustomHeader => new string[] { "rayIndex", "x", "y", "objectDetected"}; | ||
|
||
// Get values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment is not useful
protected override UXFDataRow GetCurrentValues(){ | ||
return currentRow; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this means we will end up recording the values twice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mhmm, strange in the result files each row is unique as far as I can see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if you use updateType
that is not manual?
Hey Jack, I've finally got around addressing your suggestions about the ray tracker because I never got notification of those until you closed the issue. Is there still point working on this? I am actually starting data collection for new project soon so I wanted to tidy all the code up including this. I've two questions: a) Why are the rows recorded twice. I think I might misunderstand how this might work. b) The reason I add the stop method On Trial Begin/End is because I think it only works if the update type is set to manual and I was under the impression that I have to do this like this then. Is that not true? |
The trackers have an updateType which can be LateUpdate, FixedUpdate, Manual. If we don't select manual, there will be multiple times RecordRow is called per frame
Regardless of UpdateType (which should not be needed to be set to manual) the UXF trial automatically calls the StopRecording method at the end of a trial - no need to hook into events |
Hey, so when I don't attach the start and stop methods to the On Trial Begin/End then a .csv tracking file is saved but it is empty. Could that be because I use version 2.4.2? As to your other question, I used manual because I wanted to be able to record mutliple rows per frames. See here #91. |
Yeah you would need to also restore the functionality to allow
Yeah I see - I think this is confusing because now the |
Do you have an idea how to do this? I happy to link into this. Not sure how this will effect other things but would it be possible to simply take away the option fo the other update types and force manual here? |
No description provided.