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

Luke's Multiple Fixes #36

Merged
merged 13 commits into from
Nov 4, 2022
Merged

Luke's Multiple Fixes #36

merged 13 commits into from
Nov 4, 2022

Conversation

newcomb-luke
Copy link
Contributor

Changes:

  • Changed Dial row_id and col_id to be 32 bit integers 3100353
  • Renamed DialReaction to AlarmReaction 9e7bd62
  • Fixed AlarmReaction output to use alarm names e638e88
  • Renamed ErrorPopup to DialogPopup c06a23f
  • Added a bunch of comments
  • Bumped to newest version of egui and rodio 12a58d3
  • Added "Trial Complete!" end splash screen 2cb4974

@newcomb-luke
Copy link
Contributor Author

End splash screen preview

image

Copy link
Contributor

@TroyNeubauer TroyNeubauer left a comment

Choose a reason for hiding this comment

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

Sick dude! Just a few nits but the rest looks great!

src/app.rs Outdated Show resolved Hide resolved
src/app.rs Outdated Show resolved Hide resolved
src/dial.rs Show resolved Hide resolved
src/dial.rs Show resolved Hide resolved
src/output.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
@TroyNeubauer
Copy link
Contributor

End splash screen preview

Maybe have some other information here. I don't think there is anything that would actually be helpful to present here, but even like a total elapsed time or random shiny stats to make it feel more "complete"

src/lib.rs Outdated
.unwrap_or_else(|| String::from(DEFAULT_OUTPUT_PATH)),
);
} else {
panic!();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: use unreachable!("App always in the running state on startup")?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unreachable gets completely taken out in release builds, which I'm not sure if I'm comfortable with the program continuing in any case, even if we test it in debug mode. I'll change it to panic with a message though.

@newcomb-luke
Copy link
Contributor Author

Maybe have some other information here. I don't think there is anything that would actually be helpful to present here, but even like a total elapsed time or random shiny stats to make it feel more "complete"

I sort of agree, I'll add some text saying "Trial data saved to: ...", but besides that, in theory it would take pretty much the same time every run, unless the person has a response time of 3 seconds because they were eating a sandwich at the same time, I don't think anything like that would be helpful.

@newcomb-luke
Copy link
Contributor Author

Slightly better end splash screen:

image

I don't know what else could be added without it just being completely useless

@newcomb-luke newcomb-luke merged commit 7bd5e77 into develop Nov 4, 2022
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

3 participants