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

Create ScreenRayTracker.cs #129

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

JAQuent
Copy link

@JAQuent JAQuent commented May 9, 2022

No description provided.

I've added the possibility to provide trials using tab-separated-values (tsv) so people can use "," in strings.
@JAQuent
Copy link
Author

JAQuent commented May 10, 2022

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.

@jackbrookes
Copy link
Member

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.

@jackbrookes jackbrookes self-requested a review May 19, 2022 07:51
Comment on lines +76 to +106
/// <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;
}
Copy link
Member

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely agree :)

Copy link
Author

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.

Comment on lines +7 to +54
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);
}
}

}
Copy link
Member

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 {
Copy link
Member

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 {
Copy link
Member

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.";
Copy link
Member

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){
Copy link
Member

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
Copy link
Member

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment is not useful

Comment on lines 111 to 113
protected override UXFDataRow GetCurrentValues(){
return currentRow;
}
Copy link
Member

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?

Copy link
Author

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.

Copy link
Member

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?

@JAQuent
Copy link
Author

JAQuent commented Sep 6, 2022

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?

@jackbrookes jackbrookes reopened this Sep 7, 2022
@jackbrookes
Copy link
Member

a) Why are the rows recorded twice

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

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?

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

@JAQuent
Copy link
Author

JAQuent commented Sep 8, 2022

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.

@jackbrookes
Copy link
Member

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?

Yeah you would need to also restore the functionality to allow LateUpdate and FixedUpdate updateTypes to work

As to your other question, I used manual because I wanted to be able to record mutliple rows per frames. See here #91.

Yeah I see - I think this is confusing because now the updateType must be set to manual. We would need a nicer way for this to work for it to be a part of UXF. It may be better to override the RecordRow() method for example

@JAQuent
Copy link
Author

JAQuent commented Sep 13, 2022

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?

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

Successfully merging this pull request may close these issues.

None yet

2 participants