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

Fixes for reticle preview and allowReticleChange #404

Merged
merged 2 commits into from Jan 6, 2023

Conversation

bboudaoud-nv
Copy link
Collaborator

This branch fixes a buggy interaction between specifying reticle configuration parameters at the experiment/session/trial level and using allowReticleChange = true to enable the user menu reticle configuration GUI.

The app now treats specifying any reticle configuration at the experiment/session/trial level as equivalent to setting the corresponding field's allowReticle[X]Change parameter to false. Thus the only reticle parameters users are free to configure are those not specified as part of experiment configuration.

Merging this PR closes #361

@bboudaoud-nv bboudaoud-nv added bug Something isn't working enhancement New feature or request labels Dec 13, 2022
@bboudaoud-nv bboudaoud-nv self-assigned this Dec 13, 2022
@bboudaoud-nv bboudaoud-nv changed the title Fixes for reticle preview and allowReticleChange Fixes for reticle preview and allowReticleChange Dec 13, 2022
@bboudaoud-nv
Copy link
Collaborator Author

A fuller list of changes in this branch include:

  • Automatically update the user menu at the beginning of each trial
  • Update the reticle preview (to what is actually drawn) on any change to reticle parameters (not just index)
  • Implement a ReticleConfig != operator to support the item above
  • Conditionally show user menu reticle options based on what is specified in the experiment config
  • Fix a bug keeping reticle change time from showing when color change was disabled

@bboudaoud-nv bboudaoud-nv marked this pull request as ready for review December 13, 2022 18:08
Copy link
Contributor

@jspjutNV jspjutNV left a comment

Choose a reason for hiding this comment

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

Looks like a good fix and passes all tests. Approved.

@jspjutNV jspjutNV merged commit 71fd9e8 into master Jan 6, 2023
@jspjutNV jspjutNV deleted the UserMenuReticleFix branch January 6, 2023 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Interaction between User configured reticle and experiment configure reticle
2 participants